-
Notifications
You must be signed in to change notification settings - Fork 14.6k
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
Fix wrong guide for egress selection. #34839
Conversation
|
Welcome @lshgdut! |
✅ Pull request preview available for checkingBuilt without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site settings. |
Hey @lshgdut , thank you for your contribution! I'd rather we have this change technically approved & signed off in one language i.e. English & then replicated via PRs in different languages. |
OK! I have reverted files changed except English. |
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.
/approve
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: divya-mohan0209 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 |
This PR fix the wrong guide according to the commit bellow: [apiserver: support egress selection name 'controlplane' and deprecate 'master'](kubernetes/kubernetes@a0aebf9)
I'm sorry. I have force pushed again to correct the commit message. |
@@ -2,7 +2,7 @@ apiVersion: apiserver.k8s.io/v1beta1 | |||
kind: EgressSelectorConfiguration | |||
egressSelections: | |||
# Since we want to control the egress traffic to the cluster, we use the | |||
# "cluster" as the name. Other supported values are "etcd", and "master". | |||
# "cluster" as the name. Other supported values are "etcd", and "controlplane". |
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 @lshgdut . Thanks for contributing.
Do you know if controlplane
is an accepted value? I am trying to locate information about this change.
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.
The ref commit is here kubernetes/kubernetes@a0aebf9
# file: staging/src/k8s.io/apiserver/pkg/server/egressselector/egress_selector.go
func lookupServiceName(name string) (EgressType, error) {
switch strings.ToLower(name) {
// 'master' is deprecated, interpret "master" as controlplane internally until removed in v1.22.
case "master":
klog.Warning("EgressSelection name 'master' is deprecated, use 'controlplane' instead")
return ControlPlane, nil
case "controlplane":
return ControlPlane, nil
case "etcd":
return Etcd, nil
case "cluster":
return Cluster, nil
}
return -1, fmt.Errorf("unrecognized service name %s", name)
}
BTW, the "master" value is also accepted but deprecated, it will automatically convert to "controlplane".
Please take a look at staging/src/k8s.io/apiserver/pkg/apis/apiserver/v1alpha1/conversion.go
func Convert_v1alpha1_EgressSelection_To_apiserver_EgressSelection(in *EgressSelection, out *apiserver.EgressSelection, s conversion.Scope) error {
if err := autoConvert_v1alpha1_EgressSelection_To_apiserver_EgressSelection(in, out, s); err != nil {
return err
}
if out.Name == "master" {
out.Name = "controlplane"
}
return nil
}
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.
Thanks for the information!
Thanks @lshgdut /lgtm |
LGTM label has been added. Git tree hash: 74f066cdb93eb6acec8b3b6ec78008aaeb2665b5
|
This PR fix the wrong guide according to the commit bellow:
apiserver: support egress selection name 'controlplane' and deprecate 'master'