Skip to content
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

Merged
merged 1 commit into from Mar 31, 2015

Conversation

@nikhiljindal
Copy link
Contributor

@nikhiljindal nikhiljindal commented Mar 27, 2015

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

@googlebot
Copy link

@googlebot googlebot commented Mar 27, 2015

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.

@nikhiljindal
Copy link
Contributor Author

@nikhiljindal nikhiljindal commented Mar 27, 2015

@@ -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"]
Copy link
Member

@bgrant0607 bgrant0607 Mar 27, 2015

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

Copy link
Contributor Author

@nikhiljindal nikhiljindal Mar 30, 2015

Done.

@bgrant0607
Copy link
Member

@bgrant0607 bgrant0607 commented Mar 27, 2015

FYI: We need to wait for the last couple breaking changes to be merged before we merge this PR. Hopefully ~Monday.

@deads2k
Copy link
Contributor

@deads2k deads2k commented Mar 31, 2015

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.

@bgrant0607
Copy link
Member

@bgrant0607 bgrant0607 commented Mar 31, 2015

Waiting on #6182 and #6100

@bgrant0607
Copy link
Member

@bgrant0607 bgrant0607 commented Mar 31, 2015

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,
Copy link
Member

@bgrant0607 bgrant0607 Mar 31, 2015

Copy link
Contributor

@smarterclayton smarterclayton Mar 31, 2015

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

@bgrant0607
Copy link
Member

@bgrant0607 bgrant0607 commented Mar 31, 2015

Ok, merging.

bgrant0607 added a commit that referenced this issue Mar 31, 2015
Enabling v1beta3 api version by default in master
@bgrant0607 bgrant0607 merged commit af858c9 into kubernetes:master Mar 31, 2015
2 of 3 checks passed
@bgrant0607 bgrant0607 mentioned this pull request Apr 6, 2015
16 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants