-
Notifications
You must be signed in to change notification settings - Fork 630
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
Set priority class for the critical Flux components #3802
Conversation
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.
Why did you choose to leave out IRC and IAC?
Otherwise lgtm.
Please see my comment here #3800 (comment) |
@@ -4,3 +4,6 @@ | |||
- op: add | |||
path: /spec/template/spec/serviceAccountName | |||
value: helm-controller | |||
- op: add | |||
path: /spec/template/spec/priorityClassName | |||
value: system-cluster-critical |
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've been trying to reason about how is this different from setting security context which is set on each of the individual component's deployments.
Priority class seems to be a common thing and not have any dependency on bootstrap related configurations like the service account and events-address that are set above.
I don't see anything wrong, but just wondering how to decide what should be added as a bootstrap patch and what should go to each of the individual component's deployment manifests.
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.
wondering how to decide what should be added as a bootstrap patch and what should go to each of the individual component's deployment manifests.
I think the deployment manifests in each repo should contain the minimum needed to run the e2e tests. In the future, I really hope to create a dedicated repo for Flux distribution written in cuelang and drop all the kustomize overlays & templating done in the CLI.
Mark source-controller, kustomize-controller and helm-controller as system-cluster-critical. This will reduce the chances of Flux controllers being evicted before other non-critical workloads. Signed-off-by: Stefan Prodan <[email protected]>
eeec3c9
to
a122ceb
Compare
This PR marks source-controller, kustomize-controller and helm-controller as system-cluster-critical. This priority class will reduce the chances of Flux controllers being evicted before other non-critical workloads and prevents the pods from being permanently unavailable.
Note that this does not prevent evictions, if a node is under pressure, the Flux pods will be evicted before pods marked as
system-node-critical
which is the highest available priority.Fixes #3800