-
Notifications
You must be signed in to change notification settings - Fork 40k
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
Enabling v1beta3 api version by default in master #6098
Conversation
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project, in which case you'll need to sign a Contributor License Agreement (CLA) at https://cla.developers.google.com/. If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check the information on your CLA or see this help article on setting the email on your git commits. Once you've done that, please reply here to let us know. If you signed the CLA as a corporation, please let us know the company's name. |
cc @bgrant0607 |
@@ -199,7 +199,7 @@ func (s *APIServer) Run(_ []string) error { | |||
glog.Fatalf("Failure to start kubelet client: %v", err) | |||
} | |||
|
|||
_, v1beta3 := s.RuntimeConfig["api/v1beta3"] | |||
_, disableV1beta3 := s.RuntimeConfig["noapi/v1beta3"] |
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.
Just use api/v1beta3. Flags are an API to cluster turnup tools/scripts. We basically can't change them, and there's no good reason to here. ConfigurationMap supports values. We should just parse out true/false.
https://github.com/GoogleCloudPlatform/kubernetes/blob/aaa4a354d40fccd3c4ed1f3d6cb9ecea1e2f407f/pkg/util/configuration_map.go#L30
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.
Done.
FYI: We need to wait for the last couple breaking changes to be merged before we merge this PR. Hopefully ~Monday. |
…e v1beta3 by default
3cc1053
to
478b7d5
Compare
Is there a list of issues (or a tag to search on) that need resolution before this merges? The list in #5475 looks pretty happy. |
Both of those PRs are in. Rerunning tests. |
@@ -274,7 +278,7 @@ func (s *APIServer) Run(_ []string) error { | |||
Authenticator: authenticator, | |||
Authorizer: authorizer, | |||
AdmissionControl: admissionController, | |||
EnableV1Beta3: v1beta3, | |||
DisableV1Beta3: disableV1beta3, |
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.
@smarterclayton How much of a problem are these types of breaking changes?
https://github.com/openshift/origin/blob/master/pkg/cmd/server/kubernetes/master.go#L65
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.
Not bad - it's an easy compile error during rebase. The harder is when it isn't a compile error (no one notices).
----- Original Message -----
@@ -274,7 +278,7 @@ func (s *APIServer) Run(_ []string) error {
Authenticator: authenticator,
Authorizer: authorizer,
AdmissionControl: admissionController,
EnableV1Beta3: v1beta3,
DisableV1Beta3: disableV1beta3,
@smarterclayton How much of a problem are these types of breaking changes?
https://github.com/openshift/origin/blob/master/pkg/cmd/server/kubernetes/master.go#L65
Reply to this email directly or view it on GitHub:
https://github.com/GoogleCloudPlatform/kubernetes/pull/6098/files#r27528113
Ok, merging. |
Enabling v1beta3 api version by default in master
Repurposing enableV1beta3 to disableV1beta3 in master config to enable v1beta3 by default.
v1beta3 can be turned off by using "--runtime_config=noapi/v1beta3".
For #5475