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

Removing beta references on RunAsUserName page #19016

Merged
merged 1 commit into from
Mar 12, 2020

Conversation

marosset
Copy link
Contributor

@marosset marosset commented Feb 6, 2020

This is the doc PR for moving RunAsUserName feature to beta.

Code PR: kubernetes/kubernetes#87790

KEP: kubernetes/enhancements#1043

@k8sio-netlify-preview-bot
Copy link
Collaborator

k8sio-netlify-preview-bot commented Feb 6, 2020

Deploy preview for kubernetes-io-vnext-staging processing.

Building with commit 7ede9b6

https://app.netlify.com/sites/kubernetes-io-vnext-staging/deploys/5e6690651dc4d80008198316

@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 Feb 6, 2020
@marosset
Copy link
Contributor Author

marosset commented Feb 6, 2020

/hold
code PR is still WIP

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 6, 2020
@k8s-ci-robot k8s-ci-robot added language/en Issues or PRs related to English language sig/docs Categorizes an issue or PR as relevant to SIG Docs. labels Feb 6, 2020
@makoscafee
Copy link
Contributor

/milestone 1.18

@k8s-ci-robot k8s-ci-robot added this to the 1.18 milestone Feb 19, 2020
@marosset
Copy link
Contributor Author

/hold cancel
code PR has merged

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 25, 2020
@marosset
Copy link
Contributor Author

/cc @michmike @PatrickLang for review

@k8s-ci-robot
Copy link
Contributor

@marosset: GitHub didn't allow me to request PR reviews from the following users: for, review.

Note that only kubernetes members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

/cc @michmike @PatrickLang for review

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.

Copy link
Contributor

@michmike michmike left a comment

Choose a reason for hiding this comment

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

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 25, 2020
@marosset
Copy link
Contributor Author

/assign @tengqm

@tengqm
Copy link
Contributor

tengqm commented Feb 26, 2020

/lgtm

@zacharysarah
Copy link
Contributor

/assign @VineethReddy02

Copy link
Contributor

@michmike michmike left a comment

Choose a reason for hiding this comment

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

/lgtm
/approve

Copy link
Contributor

@sftim sftim left a comment

Choose a reason for hiding this comment

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

@@ -6,14 +6,8 @@ weight: 20

{{% capture overview %}}

{{< feature-state for_k8s_version="v1.17" state="beta" >}}

This page shows how to enable and use the `RunAsUserName` feature for pods and containers that will run on Windows nodes. This feature is meant to be the Windows equivalent of the Linux-specific `runAsUser` feature, allowing users to run the container entrypoints with a different username that their default ones.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
This page shows how to enable and use the `RunAsUserName` feature for pods and containers that will run on Windows nodes. This feature is meant to be the Windows equivalent of the Linux-specific `runAsUser` feature, allowing users to run the container entrypoints with a different username that their default ones.
This page shows how to use the `RunAsUserName` feature for pods and containers that will run on Windows nodes. This feature is meant to be the Windows equivalent of the Linux-specific `runAsUser` feature, allowing users to run the container entrypoints with a different username that their default ones.

@sftim
Copy link
Contributor

sftim commented Mar 4, 2020

/lgtm cancel

I'd expect to see an update to include the feature and its history in the feature gates table.

@k8s-ci-robot k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Mar 5, 2020
@marosset marosset force-pushed the run-as-username-ga branch 2 times, most recently from a80a7e8 to c119b8a Compare March 5, 2020 03:22
@marosset
Copy link
Contributor Author

marosset commented Mar 5, 2020

@sftim Please take another look and thanks for the feedback!

Copy link
Contributor

@michmike michmike left a comment

Choose a reason for hiding this comment

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

just one nit change

Copy link
Contributor

@michmike michmike left a comment

Choose a reason for hiding this comment

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

/lgtm
/approve

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

@sftim sftim left a comment

Choose a reason for hiding this comment

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

/lgtm

(see comments)

{{< note >}}
This feature is in beta. The overall functionality for `RunAsUserName` will not change, but there may be some changes regarding the username validation.
{{< /note >}}
This page shows how to use the `RunAsUserName` feature for pods and containers that will run on Windows nodes. This feature is meant to be the Windows equivalent of the Linux-specific `runAsUser` feature, allowing users to run the container entrypoints with a different username that their default ones.
Copy link
Contributor

Choose a reason for hiding this comment

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

An observation. With the documentation style guide in mind, I'd write something like:

Suggested change
This page shows how to use the `RunAsUserName` feature for pods and containers that will run on Windows nodes. This feature is meant to be the Windows equivalent of the Linux-specific `runAsUser` feature, allowing users to run the container entrypoints with a different username that their default ones.
This page shows how to use the `runAsUserName` setting for Pods and containers that will run on Windows nodes. This is roughly equivalent to the Linux-specific `runAsUser` setting, allowing you to run applications in a container as a different username than the default.

(technical detail, also in the original: is runAsUser Linux-specific? What about other POSIX-like platforms?)

This text was in the previous revision so I'm fine leaving it in. It'd be nice to see this tidied up for GA though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated the working here and in feature-gates.md.
(note: I left in the Linux-specific comment for now - as I'm not positive this works for all posix platforms)

@VineethReddy02
Copy link
Contributor

👋 Please rebase this PR on dev-1.18 in order to avoid merge conflicts.

Feel free to /hold cancel after you've rebased.

/hold

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 7, 2020
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 9, 2020
@marosset
Copy link
Contributor Author

marosset commented Mar 9, 2020

rebased on dev-1.18 and addressed some comments left by @sftim

/hold cancel

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 9, 2020
Copy link
Contributor

@sftim sftim left a comment

Choose a reason for hiding this comment

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

/lgtm

Might be worth fixing a nit I saw. It was wrong in the original so this PR isn't introducing an actual regression.

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 9, 2020
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 9, 2020
@marosset marosset added this to In Review (v1.18) in SIG-Windows Mar 10, 2020
@michmike
Copy link
Contributor

/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 Mar 12, 2020
@michmike
Copy link
Contributor

thanks for all the changes @marosset !

@VineethReddy02
Copy link
Contributor

/lgtm
/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: michmike, VineethReddy02

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 Mar 12, 2020
@k8s-ci-robot k8s-ci-robot merged commit 99052a4 into kubernetes:dev-1.18 Mar 12, 2020
SIG-Windows automation moved this from In Review (v1.18) to Done (v1.18) Mar 12, 2020
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. language/en Issues or PRs related to English language lgtm "Looks good to me", indicates that a PR is ready to be merged. sig/docs Categorizes an issue or PR as relevant to SIG Docs. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants