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

apps/v1beta1 is not valid #3814

Merged
merged 5 commits into from
May 26, 2017
Merged

apps/v1beta1 is not valid #3814

merged 5 commits into from
May 26, 2017

Conversation

campoy
Copy link
Contributor

@campoy campoy commented May 16, 2017

Replacing it with extensions/v1beta1 which actually works

Found this file from this document:
https://kubernetes.io/docs/concepts/workloads/controllers/deployment/

^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
For 1.7 Features: set Milestone to 1.7 and Base Branch to release-1.7
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

NOTE: Please check the “Allow edits from maintainers” box below to allow
reviewers fix problems on your patch and speed up the review process.
Please delete this note before submitting the pull request.


This change is Reviewable

Replacing it with extensions/v1beta1 which actually works
@k8s-ci-robot
Copy link
Contributor

Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please follow instructions at https://github.com/kubernetes/kubernetes/wiki/CLA-FAQ to sign the CLA.

It may take a couple minutes for the CLA signature to be fully registered; after that, please reply here with a new comment and we'll verify. Thanks.


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.

@k8s-ci-robot k8s-ci-robot added the cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. label May 16, 2017
@foxish
Copy link
Contributor

foxish commented May 17, 2017

@chenopis
Copy link
Contributor

@foxish So can I close this PR?

@campoy
Copy link
Contributor Author

campoy commented May 22, 2017

When was this changed?

It still fails for me with this kubectl version:

Client Version: version.Info{Major:"1", Minor:"6", GitVersion:"v1.6.2", GitCommit:"477efc3cbe6a7effca06bd1452fa356e2201e1ee", GitTreeState:"clean", BuildDate:"2017-04-19T20:33:11Z", GoVersion:"go1.7.5", Compiler:"gc", Platform:"darwin/amd64"}
Server Version: version.Info{Major:"1", Minor:"5", GitVersion:"v1.5.7", GitCommit:"8eb75a5810cba92ccad845ca360cf924f2385881", GitTreeState:"clean", BuildDate:"2017-04-27T09:42:05Z", GoVersion:"go1.7.5", Compiler:"gc", Platform:"linux/amd64"}

Trying to create a deployment using apps/v1beta1 gives:

error: error validating "kubernetes.yaml": error validating data: couldn't find type: v1beta1.Deployment; if you choose to ignore these errors, turn validation off with --validate=false

@foxish
Copy link
Contributor

foxish commented May 22, 2017

The server version needs to be 1.6 or higher for it to work. kubernetes/kubernetes#39683 added the new apps/v1beta1.Deployment which will become the default eventually and we'll deprecate extensions/v1beta1.Deployment by the year end. See kubernetes/kubernetes#43214

@foxish
Copy link
Contributor

foxish commented May 22, 2017

In the absence of versioned docs (as opposed to docs that reflect the latest version only), we can't make this change. cc @janetkuo @kubernetes/sig-apps-misc

@0xmichalis
Copy link
Contributor

Yeah, we really need versioned docs.

@campoy
Copy link
Contributor Author

campoy commented May 22, 2017

Agreed, versioned docs are essential here.

In the meanwhile, could we add a note about how in previous versions the value is different?
This was very confusing for me

@foxish
Copy link
Contributor

foxish commented May 22, 2017

I think a note makes total sense! @campoy, can you modify your PR to add that? Also I think you'll need to sign the CLA. It can be similar to how the node controller specifies pre-1.5 behavior. Thanks!
@chenopis, In the long term, perhaps we should come up with a standard way to call out such changes in docs.

@0xmichalis
Copy link
Contributor

@kubernetes/sig-docs-maintainers is there any discussion somewhere re versioned docs?

@campoy
Copy link
Contributor Author

campoy commented May 23, 2017

PTAL

@campoy
Copy link
Contributor Author

campoy commented May 23, 2017

I am authorized to contribute, as a Google employee

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. and removed cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. labels May 23, 2017
@foxish
Copy link
Contributor

foxish commented May 23, 2017

I was thinking more along specifying the constraint in the text description surrounding the YAML. @chenopis WDYT of comments inline in the YAML itself?

@campoy
Copy link
Contributor Author

campoy commented May 23, 2017

Well, I don't know from how many places this specific file is embedded.
If it's only one I could easily add a note there

@chenopis
Copy link
Contributor

chenopis commented May 24, 2017

Yeah, I like this solution. YAML comments works for me! If it's in the YAML, that note should show up if anyone copy/pastes it or if another doc uses it. If it was just in the surrounding text, it might not get noticed. Will that work for you @foxish ?

@campoy
Copy link
Contributor Author

campoy commented May 25, 2017

ping

@janetkuo
Copy link
Member

Agree with @chenopis, lgtm

@janetkuo
Copy link
Member

I saw a similar fix (closed) for this earlier, which proved that this is confusing enough for a fix

@foxish
Copy link
Contributor

foxish commented May 25, 2017

/lgtm

@chenopis chenopis merged commit 35e959d into kubernetes:master May 26, 2017
chenopis added a commit that referenced this pull request May 26, 2017
…hub.io into release-1.7

* 'master' of https://github.com/kubernetes/kubernetes.github.io:
  Fix sentence
  Minor grammatical correction
  Remove beta as K8s support on Bluemix Container Service is live
  Fix some output
  replace REASON with STATUS
  column IP is missed
  replace kubectl.sh with kubectl
  replace KUBECTL with kubectl
  fix the command output
  fix the command output
  fix the command output
  Correcting the typo in init-container's name
  fix typo
  fix typo
  fix typo
  fix typo
  fix typo
  Update pod-overview.md (#3881)
  apps/v1beta1 is not valid (#3814)
  Get access to search console. (#3901)
@jonpulsifer jonpulsifer mentioned this pull request May 28, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants