-
Notifications
You must be signed in to change notification settings - Fork 33
(fr-6) Use dedicated startup probe with higher failure threshold #604
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
Deydra71
wants to merge
1
commit into
openstack-k8s-operators:18.0-fr6
Choose a base branch
from
Deydra71:fix-startup-probe
base: 18.0-fr6
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+16
−1
Open
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Deydra71 o/
We recently introduced a new module [1] to provide an interface for probes.
Using cinder here as just a simple example [2] that shows how to use the interface, you can basically create a
Probesetstruct with something like:or, for more advanced usage, like mariadb does [3], it is possible to also pass a command and the handler type.
In general I think we could improve this code in main and add the probe interface as well, so we can take advantage of the override in case we need to tune this statefulset in production.
Let me know if that aligns with the goal of this patch, otherwise we can create a dedicated follow up that enhances the interface in main and then we backport to fr6 as well.
[1] github.com/openstack-k8s-operators/lib-common/modules/common/probes
[2] https://github.com/openstack-k8s-operators/cinder-operator/blob/main/internal/cinderapi/statefuleset.go#L56C2-L62C3
[3] https://github.com/openstack-k8s-operators/mariadb-operator/blob/main/internal/mariadb/statefulset.go#L138
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see, I overlooked the lib-common module :/
Looking at the examples I think it's pretty straightforward to update it in main, and then create backport only from the new one (and close this one).
I can work on it this week. @fmount Is there any Jira ttracking the implementation of
probes.OverrideSpecacross controlplane? So far I could find only support in storage operators and mariadbThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
but keep in mind that this will be a CRD change. we can not just backport it and expect it to show up without an openstack-operator change. we need to plan for when it has to be released. otherwise we have to do an short term update and a longer term transition to this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi,
Because the interface has grown over the time, the idea is to add overrides only where we need, and I assume we can create stories under https://redhat.atlassian.net/browse/OSPRH-2490 to track the work.
I agree that we need to coordinate to make sure it will be available on a specific maintenance release and bump the openstack-operator (fr6) to get the CRD change as well.
For cinder we have an associated bug that will go out soon, not sure we want to take a similar approach, or we just close 2490 w/ FR6 and work on dedicated items (e.g. a new horizon bug that creates the bug-epic and the associated stream).