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

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

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

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@bgrant0607
Copy link
Member

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 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

Waiting on #6182 and #6100

@bgrant0607
Copy link
Member

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

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

@bgrant0607
Copy link
Member

Ok, merging.

bgrant0607 added a commit that referenced this pull request Mar 31, 2015
Enabling v1beta3 api version by default in master
@bgrant0607 bgrant0607 merged commit af858c9 into kubernetes:master Mar 31, 2015
@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
Development

Successfully merging this pull request may close these issues.

None yet

5 participants