-
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
Vetting / Formatting / Cleanup #3078
Conversation
- fixed any of the vettting / formatting issues that i'm came across on the update - removed the commented out lines from the componentconfig as it make its increasingly difficult to find what is supported, what is not and the difference between them. - added SerializeImagePulls, RegisterSchedulable to kubelet (by default they are ignored) - added FeatureGates to the kube-proxy Out of interest can someone point me to where these multi-versioned componentconfig are being used?
Hi @gambol99. Thanks for your PR. I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with I understand the commands that are listed here. 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/test-infra repository. I understand the commands that are listed here. |
// hostnameOverride is the hostname used to identify the kubelet instead | ||
// of the actual hostname. | ||
// Note: We recognize some additional values: | ||
// @aws uses the hostname from the AWS metadata service |
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.
This note about @aws
is important to keep!
|
||
// hostnameOverride, if non-empty, will be used as the identity instead of the actual hostname. | ||
// Note: We recognize some additional values: | ||
// @aws uses the hostname from the AWS metadata service |
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.
Probably we want to keep the @aws
note here also
/ok-to-test |
/lgtm |
It's a whopper of a PR, but thank you for cleaning it all up. Normally I'd ask for it to be split up into its component ideas, but as you took on the thankless task of cleaning up govet I've marked it as LGTM (subject to passing tests!) |
/approve no-issue |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: gambol99, justinsb Associated issue requirement bypassed by: justinsb The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these OWNERS Files:
You can indicate your approval by writing |
/test all [submit-queue is verifying that this PR is safe to merge] |
/retest |
/test all [submit-queue is verifying that this PR is safe to merge] |
Automatic merge from submit-queue |
Automatic merge from submit-queue Kube Proxy Feature Gates - fixing the [kubeproxy feature gates](#3078), this should have been a [map](https://github.com/kubernetes/kops/blob/master/pkg/apis/kops/v1alpha2/componentconfig.go#L134) not an array ... apologizes!! from
Out of interest can someone point me to where these multi-versioned componentconfig are being used?