Customize the number of glanceAPI workers

[fmount]

No description provided.

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: fmount

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [fmount]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[konan-abhi]

should we limit the max cap for workers same as the number of CPUs?

[fmount]

we can yes, but we might need more logic in case we don't expose the parameter and allow customers to override it via customServiceConfig

[stuggi]

why should we add a parameter for this and not use the config customization if we need it? just wondering as we not add parameters for each of the settings in the service config, while this can be customized using the customServiceConfig.

[fmount]

why should we add a parameter for this and not use the config customization if we need it? just wondering as we not add parameters for each of the settings in the service config, while this can be customized using the customServiceConfig.

Mostly to keep consistency with other storage operators, but we can use customServiceConfig in this case.

[fmount]

@konan-abhi @stuggi should we simply document how to do override that value?
We might need more logic to control overrides on this parameter and control how GlanceAPI can scale in combination with replicas and workers.
We can check what is passed via customServiceConfig and set boundaries for that variable. @stuggi is this a good/better approach?

[stuggi]

I think we should not have added this parameter to any operator

[stuggi]

manila and cinder PRs are different. they customize the httpd processes. this should align with what was already done in keystone PR openstack-k8s-operators/keystone-operator#500 . in my opinion, if glance is different and does not use httpd process to control this, we should not replicate this here for the glance config. a user should use the default interface for this

[fmount]

manila and cinder PRs are different. they customize the httpd processes. this should align with what was already done in keystone PR openstack-k8s-operators/keystone-operator#500 . in my opinion, if glance is different and does not use httpd process to control this, we should not replicate this here for the glance config. a user should use the default interface for this

I'm ok to remove this parameter for Glance, and parse customServiceConfig to make sure we stay under certain limits. I will update this patch accordingly! Thanks @stuggi for the input.
BTW I can align Manila and Cinder with the keystone patch: I simply don't like the naming and we didn't agree on something we can use and spread across the operators. I think we should have periodic API reviews to keep consistency and agree on the parameters in advance.

[stuggi]

manila and cinder PRs are different. they customize the httpd processes. this should align with what was already done in keystone PR openstack-k8s-operators/keystone-operator#500 . in my opinion, if glance is different and does not use httpd process to control this, we should not replicate this here for the glance config. a user should use the default interface for this

I'm ok to remove this parameter for Glance, and parse customServiceConfig to make sure we stay under certain limits. I will update this patch accordingly! Thanks @stuggi for the input. BTW I can align Manila and Cinder with the keystone patch: I simply don't like the naming and we didn't agree on something we can use and spread across the operators. I think we should have periodic API reviews to keep consistency and agree on the parameters in advance.

agreed we nee to do better on that + also to get those things across all operators. Its not efficient that this is done by each group

[fmount]

manila and cinder PRs are different. they customize the httpd processes. this should align with what was already done in keystone PR openstack-k8s-operators/keystone-operator#500 . in my opinion, if glance is different and does not use httpd process to control this, we should not replicate this here for the glance config. a user should use the default interface for this

I'm ok to remove this parameter for Glance, and parse customServiceConfig to make sure we stay under certain limits. I will update this patch accordingly! Thanks @stuggi for the input. BTW I can align Manila and Cinder with the keystone patch: I simply don't like the naming and we didn't agree on something we can use and spread across the operators. I think we should have periodic API reviews to keep consistency and agree on the parameters in advance.

agreed we nee to do better on that + also to get those things across all operators. Its not efficient that this is done by each group

For the record: this is a conversation about scalability we had yesterday within the storage group. The result about the control around scalability, placement, and setting upper limits involved this workers parameter for Glance, processes for Manila and Cinder API. I didn't plan to put these set of patches up but considering how simple is adding this parameter in the picture I started it. Being that said I'm ok to keep consistency across the operators, I simply think we should plan to either periodically review or centralize the API for k8s-operators, and don't risk to do to different directions because I simply thought about a different name for a struct.

[fmount]

@stuggi I'm closing this patch for now, I will reopen in case we need some validation around workers parameter.