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

Remove PersistentVolumeLabel from default admission controllers. #7929

Merged
merged 1 commit into from
Apr 1, 2018

Conversation

gyliu513
Copy link
Contributor

@gyliu513 gyliu513 commented Apr 1, 2018

PersistentVolumeLabel is now deprecated, we should remove it from
default list.

^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
For 1.10 Features: set Milestone to 1.10 and Base Branch to release-1.10
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
NOTE: After opening the PR, please un-check and re-check the "Allow edits from maintainers" box so that maintainers can work on your patch and speed up the review process. This is a temporary workaround to address a known issue with GitHub.>

Please delete this note before submitting the pull request.

Allow edits from maintainers checkbox

PersistentVolumeLabel is now deprecated, we should remove it from
default list.
@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Apr 1, 2018
@k8sio-netlify-preview-bot
Copy link
Collaborator

Deploy preview for kubernetes-io-master-staging ready!

Built with commit 9867b0f

https://deploy-preview-7929--kubernetes-io-master-staging.netlify.com

@bradtopol bradtopol self-assigned this Apr 1, 2018
@bradtopol
Copy link
Contributor

Hi @gyliu513 Thank you for your doc contribution. On a change like this I would typically ask for a tech review. But you would be the person I would ask for a tech review on this type of change. So great job!
/approve
/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 1, 2018
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bradtopol

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 1, 2018
@k8s-ci-robot k8s-ci-robot merged commit 2a7f1d3 into kubernetes:master Apr 1, 2018
@gyliu513 gyliu513 deleted the admission branch April 2, 2018 00:02
@tengqm
Copy link
Contributor

tengqm commented Apr 2, 2018

No. This is incorrect/inaccurate. If we are document the behavior for 1.10 when the admission-control flag has been deprecated, we should instead document enable-admission-controls and disable-admission-controls.
If we are fixing the document for 1.9, the PersistentVolumeLabel has not been deprecated.

@gyliu513
Copy link
Contributor Author

gyliu513 commented Apr 2, 2018

@tengqm
Copy link
Contributor

tengqm commented Apr 2, 2018

@gyliu513 Yes, I know you are fixing master. However, in the master branch (1.10), we should say enable/disable-admission-controls. The admission-control flag is gone now.

@gyliu513
Copy link
Contributor Author

gyliu513 commented Apr 2, 2018

@tengqm can you show more detail for enable/disable-admission-controls? Where are they defined and how to use them?

@tengqm
Copy link
Contributor

tengqm commented Apr 2, 2018

@gyliu513

Upstream PR is here: kubernetes/kubernetes#58123 kubernetes/kubernetes#58567

Website issue: #7235

Website fixes: #7449

@gyliu513
Copy link
Contributor Author

gyliu513 commented Apr 2, 2018

I see, then I think that we should create another PR to address this issue. The document should be updated as follows:

  1. >1.10
  2. >=1.9.0 <1.10.0
  3. >=1.6.0 <1.9.0

Comments? @tengqm

@tengqm
Copy link
Contributor

tengqm commented Apr 2, 2018

@gyliu513 Agreed. Please keep in mind that the website is not actively maintaining contents for deprecated versions. This means you may want to only submit a PR to master branch. In that PR, you can describe the differences between versions.

zacharysarah pushed a commit that referenced this pull request Apr 16, 2018
PersistentVolumeLabel is now deprecated, we should remove it from
default list.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants