Make process number configurable in KeystoneAPI by Deydra71 · Pull Request #500 · openstack-k8s-operators/keystone-operator

Deydra71 · 2024-11-13T13:49:11Z

Adds new section httpdCustomization with the option processNumber to configure the number of processes used by KeystoneAPI. The maximum number is not set, though this may be defined in the future

Jira: OSPRH-10363

vakwetu · 2024-11-13T14:50:02Z

+                description: Processumber - Number of processes running in keystone
+                  API
+                format: int32
+                maximum: 10


not sure about setting a max here. @d34dh0r53 @dmendiza @xek - is there a reasonable setting here or should we just leave it blank?

I deleted the maximum limit for now for testing.

I also don't think a maximum is needed, it's easy enough to add should this become a problem.

stuggi · 2024-11-18T15:52:32Z

This is probably something we want to be able to do across multiple service operators api deployments, but I am not sure if adding it as a top parameter for the deployment is good. if there will be more parameters we need to be able to customize related to httpd its probably better to have them in a sub struct. And this needs to be the same for all operators. @dprince @olliewalsh @abays thoughts?

maybe we should have a generic crd for httpd which allows us to extend for future customization.

vakwetu · 2024-11-18T16:00:16Z

@d34dh0r53 ^^ just for awareness of the above. @d34dh0r53 is working on changes needed for federation.

olliewalsh · 2024-11-18T16:11:49Z

This is probably something we want to be able to do across multiple service operators api deployments, but I am not sure if adding it as a top parameter for the deployment is good. if there will be more parameters we need to be able to customize related to httpd its probably better to have them in a sub struct. And this needs to be the same for all operators. @dprince @olliewalsh @abays thoughts?

maybe we should have a generic crd for httpd which allows us to extend for future customization.

+1 for a sub struct. Also could potentially switch from apache/wsgi to $something_else in future

vakwetu · 2024-11-18T16:54:24Z

I'm not opposed to a substruct of some sort. That said, there are types of apache config which are much more extensive - see #479 Does the approach there change at all?

stuggi · 2024-11-19T07:27:15Z

This is probably something we want to be able to do across multiple service operators api deployments, but I am not sure if adding it as a top parameter for the deployment is good. if there will be more parameters we need to be able to customize related to httpd its probably better to have them in a sub struct. And this needs to be the same for all operators. @dprince @olliewalsh @abays thoughts?
maybe we should have a generic crd for httpd which allows us to extend for future customization.

+1 for a sub struct. Also could potentially switch from apache/wsgi to $something_else in future

I was thinking about adding some subsection for httpdTunings or httpdCustomization. if there are more settings, we could just add them there, instead of having multiple top level ones.

spec:
  httpdCustomization:
    processNumber:

don't think it affects #479 as it is a separate feature which are not a generic httpd settings.

Deydra71 · 2024-11-19T11:27:16Z

@stuggi Please take a look again on the updated version, ty!

stuggi

looks good to me, @olliewalsh @dprince @abays for you too?

stuggi · 2024-11-21T09:10:11Z

+type HttpdCustomization struct {
+	// +kubebuilder:validation:Optional
+	// +kubebuilder:default=3
+	// +kubebuilder:validation:Minimum=1


as discussed on slack, this needs to be tested for updating the controller/CRD on an already deployed env

olliewalsh · 2024-11-21T09:24:08Z

looks good to me, if updating works

Signed-off-by: Veronika Fisarova <vfisarov@redhat.com>

stuggi

/lgtm

openshift-ci · 2024-11-22T08:10:27Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Deydra71, stuggi, vakwetu, xek

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~~ [stuggi]

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

Deydra71 · 2024-11-22T08:16:43Z

/cherry-pick 18.0-fr1

openshift-cherrypick-robot · 2024-11-22T08:17:22Z

@Deydra71: new pull request created: #504

Details

In response to this:

/cherry-pick 18.0-fr1

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

Includes openstack-k8s-operators/keystone-operator#500

Deydra71 added the do-not-merge/work-in-progress label Nov 13, 2024

Deydra71 requested review from vakwetu and xek November 13, 2024 13:49

openshift-ci Bot requested review from lewisdenny and viroel November 13, 2024 13:49

vakwetu reviewed Nov 13, 2024

View reviewed changes

Comment thread controllers/keystoneapi_controller.go Outdated

vakwetu reviewed Nov 13, 2024

View reviewed changes

Deydra71 force-pushed the customize-process-number branch 2 times, most recently from 93ab936 to 0bcde09 Compare November 14, 2024 10:13

Deydra71 changed the title ~~[WIP] Make process number configurable in KeystoneAPI~~ Make process number configurable in KeystoneAPI Nov 18, 2024

openshift-ci Bot removed the do-not-merge/work-in-progress label Nov 18, 2024

vakwetu approved these changes Nov 18, 2024

View reviewed changes

openshift-ci Bot assigned vakwetu Nov 18, 2024

openshift-ci Bot added the lgtm label Nov 18, 2024

Deydra71 requested a review from stuggi November 18, 2024 15:23

dprince reviewed Nov 18, 2024

View reviewed changes

Comment thread api/bases/keystone.openstack.org_keystoneapis.yaml Outdated

Deydra71 force-pushed the customize-process-number branch from 0bcde09 to b2c718c Compare November 19, 2024 08:40

openshift-ci Bot removed the lgtm label Nov 19, 2024

Deydra71 force-pushed the customize-process-number branch from 56e7c8d to 6e628c6 Compare November 19, 2024 08:51

stuggi reviewed Nov 19, 2024

View reviewed changes

xek approved these changes Nov 20, 2024

View reviewed changes

openshift-ci Bot assigned xek Nov 20, 2024

openshift-ci Bot added the lgtm label Nov 20, 2024

stuggi reviewed Nov 21, 2024

View reviewed changes

Make process number configurable

b52ef01

Signed-off-by: Veronika Fisarova <vfisarov@redhat.com>

Deydra71 force-pushed the customize-process-number branch from 6e628c6 to b52ef01 Compare November 21, 2024 09:49

openshift-ci Bot removed the lgtm label Nov 21, 2024

stuggi approved these changes Nov 22, 2024

View reviewed changes

openshift-ci Bot assigned stuggi Nov 22, 2024

openshift-ci Bot added the lgtm label Nov 22, 2024

openshift-ci Bot added the approved label Nov 22, 2024

openshift-merge-bot Bot merged commit 322aab6 into openstack-k8s-operators:main Nov 22, 2024

openshift-cherrypick-robot mentioned this pull request Nov 22, 2024

[18.0-fr1] Make process number configurable in KeystoneAPI #504

Merged

olliewalsh added a commit to olliewalsh/openstack-operator that referenced this pull request Nov 22, 2024

Inherit top-level nodeSelector in keystone template

4058189

Includes openstack-k8s-operators/keystone-operator#500

Uh oh!

Conversation

Deydra71 commented Nov 13, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

vakwetu Nov 13, 2024

Choose a reason for hiding this comment

Uh oh!

Deydra71 Nov 14, 2024

Choose a reason for hiding this comment

Uh oh!

d34dh0r53 Nov 18, 2024

Choose a reason for hiding this comment

Uh oh!

stuggi commented Nov 18, 2024

Uh oh!

vakwetu commented Nov 18, 2024

Uh oh!

olliewalsh commented Nov 18, 2024

Uh oh!

vakwetu commented Nov 18, 2024

Uh oh!

Uh oh!

stuggi commented Nov 19, 2024

Uh oh!

Deydra71 commented Nov 19, 2024

Uh oh!

stuggi left a comment

Choose a reason for hiding this comment

Uh oh!

stuggi Nov 21, 2024

Choose a reason for hiding this comment

Uh oh!

olliewalsh commented Nov 21, 2024

Uh oh!

stuggi left a comment

Choose a reason for hiding this comment

Uh oh!

openshift-ci Bot commented Nov 22, 2024

Uh oh!

Deydra71 commented Nov 22, 2024

Uh oh!

openshift-cherrypick-robot commented Nov 22, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

Deydra71 commented Nov 13, 2024 •

edited

Loading