-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
Node mode controllers #5867
Node mode controllers #5867
Conversation
// @check if the node authorization is enabled and if so enable the tokencleaner controller (disabled by default) | ||
// This is responsible for cleaning up bootstrap tokens which have expired | ||
if b.Context.IsKubernetesGTE("1.10") { | ||
if fi.BoolValue(clusterSpec.KubeAPIServer.EnableBootstrapAuthToken) && len(kcm.Controllers) <= 0 { |
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.
If a user has enabled bootstrap tokens for their cluster and provides a list of KCM controllers (excluding the tokencleaner), should we still append to the list here, or just take their literal set of controllers to apply?
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.
A list of controllers to enable. '*' enables all on-by-default controllers, 'foo' enables the controller named 'foo', '-foo' disables the controller named 'foo'.
All controllers: attachdetach, bootstrapsigner, clusterrole-aggregation, cronjob, csrapproving, csrcleaner, csrsigning, daemonset, deployment, disruption, endpoint, garbagecollector, horizontalpodautoscaling, job, namespace, nodeipam, nodelifecycle, persistentvolume-binder, persistentvolume-expander, podgc, pv-protection, pvc-protection, replicaset, replicationcontroller, resourcequota, route, service, serviceaccount, serviceaccount-token, statefulset, tokencleaner, ttl, ttl-after-finished
Disabled-by-default controllers: bootstrapsigner, tokencleaner
So only two controllers are actually disabled by default .. I'm going on the basis that if they are setting the controllers
flag they know what they are doing, or at the very least taking responsibility for it ..
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.
If we do that: documentation should be very clear on what and how this is going to affect the results. Also: needs to be backwards compatible to not break things as “know what you are doing” is good, but so many people implement it in pipelines and scripts - especially if they get this deep into the parameters.
Ah sorry I missed this in the last PR! Looks good, just one minor comment and will wait for tests to finish. :) |
Once the NodeAuthorizer / Bootstrap tokens work is moved to a more stable release within kops, will need to make sure enabling the KCM tokencleaner controller is highlighted in the docs. |
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: gambol99, KashifSaadat The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
controllers
flag to a[]string
, not the last PR wouldn't have worked as theflagbuilder
does not handle pointers to a string slice.