Get gophercloud system wide client#284
Merged
stuggi merged 1 commit intoopenstack-k8s-operators:mainfrom Jun 13, 2023
Merged
Conversation
fmount
added a commit
to fmount/glance-operator
that referenced
this pull request
Jun 10, 2023
This patch exposes a boolean that can be used to enforce per-tenant quotas in the GlanceAPI. If true, the related glance-api section is enabled and templateParameters are populated. This patch assumes the quota resources are already registered in keystone. Depends-On: openstack-k8s-operators/lib-common/pull/262 Depends-On: gophercloud/gophercloud/pull/2616 Depends-On: openstack-k8s-operators/lib-common/pull/284 Signed-off-by: Francesco Pantano <fpantano@redhat.com>
fmount
added a commit
to fmount/glance-operator
that referenced
this pull request
Jun 10, 2023
This patch exposes the data structure and implement the related logic that can be used to enforce per-tenant quotas in the GlanceAPI. When limits are defined, the related glance-api config section is enabled and templateParameters are populated. Depends-On: openstack-k8s-operators/lib-common/pull/262 Depends-On: gophercloud/gophercloud/pull/2616 Depends-On: openstack-k8s-operators/lib-common/pull/284 Signed-off-by: Francesco Pantano <fpantano@redhat.com>
konan-abhi
reviewed
Jun 12, 2023
modules/openstack/openstack.go
Outdated
| TenantName: cfg.TenantName, | ||
| DomainName: cfg.DomainName, | ||
| Scope: &gophercloud.AuthScope{ | ||
| System: true, |
Contributor
There was a problem hiding this comment.
Shouldn't this be based on some condition or do we always want this client with system scope?
Contributor
There was a problem hiding this comment.
Something like this;
if os.Getenv("OS_SYSTEM_SCOPE") != "all"
System: false
Contributor
Author
There was a problem hiding this comment.
not sure, maybe it's a good idea putting the Scope under a condition: I added it like this for two reasons:
- if we add a condition we then need to patch
keystone-operatorto get theOpenStackcli passing the scope; - we only use gophercloud to create resources from the operators (endpoints, services, etc) against the
ControlPlanebeing deployed: any other client can rely on the resultingclouds.yamlproduced by keystone in aConfigMapand get the appropriate client.
So you have strong opinions on it, or, do we have examples where having System: true can cause troubles?
I'm not against having it as a variable, it makes sense in lib-common, but I suspect we need to patch keystone-operator accordingly.
@stuggi any thoughts on this?
Contributor
There was a problem hiding this comment.
As you said, I am not sure if we need it, but you could add a Scope pointer to the above AuthOpts and if it is not nil you can set it in NewOpenStack. with this there would be no need to the keystone-operator
Contributor
Author
There was a problem hiding this comment.
Makes sense, thanks @stuggi, I'll follow up on this!
fmount
added a commit
to fmount/glance-operator
that referenced
this pull request
Jun 12, 2023
This patch exposes the data structure and implement the related logic that can be used to enforce per-tenant quotas in the GlanceAPI. When limits are defined, the related glance-api config section is enabled and templateParameters are populated. Depends-On: openstack-k8s-operators/lib-common/pull/262 Depends-On: gophercloud/gophercloud/pull/2616 Depends-On: openstack-k8s-operators/lib-common/pull/284 Signed-off-by: Francesco Pantano <fpantano@redhat.com>
fmount
added a commit
to fmount/glance-operator
that referenced
this pull request
Jun 12, 2023
This patch exposes the data structure and implement the related logic that can be used to enforce per-tenant quotas in the GlanceAPI. When limits are defined, the related glance-api config section is enabled and templateParameters are populated. Depends-On: openstack-k8s-operators/lib-common/pull/262 Depends-On: gophercloud/gophercloud/pull/2616 Depends-On: openstack-k8s-operators/lib-common/pull/284 Signed-off-by: Francesco Pantano <fpantano@redhat.com>
fmount
added a commit
to fmount/glance-operator
that referenced
this pull request
Jun 12, 2023
This patch exposes the data structure and implement the related logic that can be used to enforce per-tenant quotas in the GlanceAPI. When limits are defined, the related glance-api config section is enabled and templateParameters are populated. Depends-On: openstack-k8s-operators/lib-common/pull/262 Depends-On: gophercloud/gophercloud/pull/2616 Depends-On: openstack-k8s-operators/lib-common/pull/284 Signed-off-by: Francesco Pantano <fpantano@redhat.com>
fmount
added a commit
to fmount/glance-operator
that referenced
this pull request
Jun 12, 2023
This patch exposes the data structure and implement the related logic that can be used to enforce per-tenant quotas in the GlanceAPI. When limits are defined, the related glance-api config section is enabled and templateParameters are populated. Depends-On: openstack-k8s-operators/lib-common/pull/262 Depends-On: gophercloud/gophercloud/pull/2616 Depends-On: openstack-k8s-operators/lib-common/pull/284 Signed-off-by: Francesco Pantano <fpantano@redhat.com>
The 'NewOpenStack' function is used to get a an admin, token that can be used by operators through keystone service to register the associated resources. However, without "scope:system" Glance is not able to register the keystone global limits defined in its main CR and it will fail. This patch ensures we have a system wide admin client that is able to act globally against the existing ctlplane. Signed-off-by: Francesco Pantano <fpantano@redhat.com>
konan-abhi
approved these changes
Jun 12, 2023
Contributor
konan-abhi
left a comment
There was a problem hiding this comment.
Looks good, Thank you!
fmount
added a commit
to fmount/glance-operator
that referenced
this pull request
Jun 12, 2023
When the "quotas" struct is passed to the main Glance CR, the operator needs a system scoped token to make sure the request is run successfully. This change allows the glance-operator to not rely on the keystone operator and build the required OSClient based on the new AuthOps "System" parameter. Depends-On: openstack-k8s-operators/lib-common#284 Signed-off-by: Francesco Pantano <fpantano@redhat.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The
NewOpenStackfunction is only used bykeystoneto get a an admin, system wide token that can be used by other operators through keystone service to register the associated resources. However, withoutscope:systemGlanceis not able to register the global limits defined in its main CR and it will fail configuring limits [1].This patch ensures we have a system wide admin client that is able to act globally against the existing ctlplane.
[1] https://docs.openstack.org/glance/latest/admin/quotas.html#configuring-glance-for-per-tenant-quotas