Allow direct communication to Openshift Quota API #205
Conversation
It was discovered that Openshift integration did not function because the Openshift allocation would use the same url to make calls to both the account manager and the Openshift API. This resulted in the Openshift API never actually being called. Due to poor error handling by the integration code (more details below), this bug went undetected by the functional tests. Appropriate fixes have been added to tests, and the Openshift resource type now requires two URLs, one for the account manager, and one for the Openshift API. Regarding the poor error handling, none of the functional Openshift test cases actually checked if the call to `get_federated_user()` returned the expected output. The `get_federated_user()` function itself only emits a log message if the user is not found. This meant that even though `get_federated_user()` never called the Openshift API and would therefore never find the user, the test cases still passed. Additionally, while `_openshift_user_exists()`, the function which was supposed to call the Openshift API, does catch the `kexc.NotFoundError`, this error is not specific enough, as it could be caused by a 404 response made by ANY server (in the case of our tests, the account manager). It was also found that the `RESOURCE_IDENTITY_NAME` attribute, which identifies the idp, was referenced in integration code but never defined, leading to `_openshift_identity_exists` always failing, since `self.id_provider` would always be `None`
QuanMPhm
force-pushed
the
187/acct_quota
branch
from
b91248f to
6a4b4d1
Compare
|
|
QuanMPhm
force-pushed
the
187/acct_quota
branch
from
6a4b4d1 to
ca050e9
Compare
|
The way the Openshift quota is fetched has been simplified. |
|
What are the circumstances under which this direct communication is needed? The description is insufficient. |
|
@hpdempsey Currently, this Coldfront plugin interacts with the Openshift API through a piece of middleware, @knikolla @naved001 I'll defer to you guys for more clarification |
src/coldfront_plugin_cloud/management/commands/add_openshift_resource.py
Show resolved
Hide resolved
@naved001 The unit test |
The Openshift allocator will now only make the minimal `resourcequota` object for each namespace, with no support for scopes. Most of the integration code and test cases have been adapted from `openshift-acct-mgt`. Notable exclusions were any code related to the `quota.json`[1], `limits.json`[2], and quota scopes[3]. [1] https://github.com/CCI-MOC/openshift-acct-mgt/blob/master/k8s/base/quotas.json [2] https://github.com/CCI-MOC/openshift-acct-mgt/blob/master/k8s/base/limits.json [3] https://github.com/CCI-MOC/openshift-acct-mgt/blob/42db8f80962fd355eac1bc80a1894dc6bb824f12/acct_mgt/moc_openshift.py#L418-L431
QuanMPhm
force-pushed
the
187/acct_quota
branch
from
ca050e9 to
87ad16a
Compare
Closes #187, this PR consists of two commits. The first one fixes an integration bug and will now require Openshift resources to specify two URLs, one to the account manager, and one to the Openshift API. The second implements the integration to the Openshift API.
More details in the commit messages.