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

Use sudo for elevated permissions while upgrading clusters using kubeadm #44832

Merged
merged 1 commit into from
Feb 15, 2024

Conversation

adityasamant25
Copy link
Contributor

The pages for Upgrading kubeadm clusters and Upgrading Linux nodes have reference to apt, apt-cache, apt-mark, apt-get, yum and kubeadm upgrade apply commands. These commands need elevated permissions through sudo (if executed without root).

The PR adds the usage of sudo for these commands to ease the experience for the reader.

Regards,
Aditya

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jan 21, 2024
@k8s-ci-robot k8s-ci-robot requested a review from onlydole January 21, 2024 09:08
@k8s-ci-robot k8s-ci-robot added the language/en Issues or PRs related to English language label Jan 21, 2024
@k8s-ci-robot k8s-ci-robot requested a review from tengqm January 21, 2024 09:08
@k8s-ci-robot k8s-ci-robot added sig/docs Categorizes an issue or PR as relevant to SIG Docs. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jan 21, 2024
Copy link

netlify bot commented Jan 21, 2024

Pull request preview available for checking

Built without sensitive environment variables

Name Link
🔨 Latest commit c6e210f
🔍 Latest deploy log https://app.netlify.com/sites/kubernetes-io-main-staging/deploys/65b0e0692a53c3000883ccd6
😎 Deploy Preview https://deploy-preview-44832--kubernetes-io-main-staging.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@T-Lakshmi
Copy link
Contributor

I think its worth to mention sudo.
Looking good to me

Preview page:
Upgrading Linux nodes | Upgrading kubeadm clusters

Copy link
Member

@neolit123 neolit123 left a comment

Choose a reason for hiding this comment

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

are these all the commands that need sudo on the pages?
/lgtm

but leaving to SIG docs what needs to be done here.
we had a discussion before that the sudo requirement can be just a note on top of the doc instead of having sudo on every command.

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

LGTM label has been added.

Git tree hash: b9a5565de5441af669e9fc62f1b04b05918d16af

@neolit123
Copy link
Member

/sig cluster-lifecycle

@k8s-ci-robot k8s-ci-robot added the sig/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. label Jan 29, 2024
@adityasamant25
Copy link
Contributor Author

are these all the commands that need sudo on the pages? /lgtm

I have executed the steps on this page several times. I am pretty confident that all commands that need sudo have been taken care of in this PR.

@T-Lakshmi
Copy link
Contributor

we had a discussion before that the sudo requirement can be just a note on top of the doc instead of having sudo on every command.

Good suggestion,
I believe this approach is an efficient way of documenting.

@adityasamant25
Copy link
Contributor Author

I believe most readers would copy paste the commands. And it is an inconvenience to re-execute with sudo each time a command fails with permissions. Some commands on the page already had the sudo prefixed. This PR adds it to the missing ones.

@@ -43,7 +43,7 @@ The upgrade workflow at high level is the following:
they could be running CoreDNS Pods or other critical workloads. For more information see
[Draining nodes](/docs/tasks/administer-cluster/safely-drain-node/).
- The Kubernetes project recommends that you match your kubelet and kubeadm versions.
You can instead use an a version of kubelet that is older than kubeadm, provided it is within the
You can instead use a version of kubelet that is older than kubeadm, provided it is within the
Copy link
Contributor

Choose a reason for hiding this comment

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

(nit)
I'd have put this fixup in a separate commit.

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.

Consider mentioning sudo in the prerequisites, and ideally tell people that alternative tools work fine.

(I like using PolicyKit locally, and sometimes use alternatives to sudo on servers).

Anyway:
/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: sftim

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 Feb 15, 2024
@k8s-ci-robot k8s-ci-robot merged commit 217f882 into kubernetes:main Feb 15, 2024
6 checks passed
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/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. sig/docs Categorizes an issue or PR as relevant to SIG Docs. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants