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

Quote grep patterns in docs/rotate-secrets.md #10656

Merged
merged 1 commit into from
May 6, 2021

Conversation

keithlayne
Copy link
Contributor

This should work more reliably independent of shell config.

This should work more reliably independent of shell config.
@k8s-ci-robot k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Jan 25, 2021
@k8s-ci-robot
Copy link
Contributor

Welcome @keithlayne!

It looks like this is your first PR to kubernetes/kops 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes/kops has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

@k8s-ci-robot
Copy link
Contributor

Hi @keithlayne. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

@k8s-ci-robot k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Jan 25, 2021
@k8s-ci-robot k8s-ci-robot added area/documentation size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jan 25, 2021
@keithlayne
Copy link
Contributor Author

keithlayne commented Jan 25, 2021

Wow, thought I would validate the instructions there...and BOOM thought I had destroyed my whole cluster. Not an awesome experience. When in doubt, add moar --force and --yes flags, and everything will be okay.

@hakman
Copy link
Member

hakman commented Jan 30, 2021

Hey @keithlayne. Any conclusion? I see the PR is still set as draft.

@keithlayne
Copy link
Contributor Author

Yeah, sorry. Got distracted. I think this tiny change is potentially valuable since the carets in the greps lines made my shell unhappy.

Let me try to give some feedback on my experience after that.

  • When deleting keypairs/secrets, there was a duplicate keypair called master - not sure if this was a peculaiarity of my cluster or not, but they AFAICT needed to be deleted individually like kops delete secret keypair master 6921496088630691630953591119
  • Exporting kubecfg might benefit from a note or link about the 1.19 changes - that would probably be helpful all over the docs, ideally something unintrusive.
  • Maybe format the block under recreating service accounts to not be chopped off - would like to be able to see in one go what I need to copy & paste, possibly:
# Delete all service account tokens in all namespaces
NS=`kubectl get namespaces -o 'jsonpath={.items[*].metadata.name}'`
for i in ${NS}
do 
  kubectl get secrets --namespace=${i} --no-headers \
    | grep "kubernetes.io/service-account-token" \
    | awk '{print $1}' \
    | xargs -I {} kubectl delete secret --namespace=$i {}
done

# Allow for new secrets to be created
sleep 60

# Bounce all pods to make use of the new service tokens
pkill -f kube-controller-manager
kubectl delete pods --all --all-namespaces

It's really a simple one-liner, but much easier (for me) to see exacly what's going on when I don't have to scroll.

As far as execution goes, that block it where it all went bad for me:

  • the pkill line needed sudo for me
  • deleting all the pods never actually terminated for me

Then I had a rough time from there. I tried a whole bunch of manual interventions but my cluster was down for hours until I sucked it up and did another kops rolling-update cluster --cloudonly --force --yes, which eventually brought the cluster back to life.

I don't know how much of any of this experience is peculiar to me, and it's not something I'm excited to try again real soon.

@hakman If this is TMI, lemme know, i'll just leave this PR tiny. Otherwise if you could respond to my feedback and let me know what you would like to incorporate in this PR I'd appreciate it, and we can go from there.

@cten
Copy link

cten commented Feb 4, 2021

I experienced much of the same issues that @keithlayne describes above. Including having to run the rolling-update a second time because after the first none of the worker nodes even showed up in my cluster. Also depending on the version of kops the export kubecfg might be different, this actually took me the longest to figure out bc I could use kubectl from the master but got Unauthorized from my laptop.

@fejta-bot
Copy link

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-contributor-experience at kubernetes/community.
/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label May 5, 2021
@johngmyers
Copy link
Member

@keithlayne I believe this PR is good as-is, but first you would need to take it out of draft state.

@keithlayne
Copy link
Contributor Author

@johngmyers Was hoping for the other items in this comment might get more feedback and be addressed. But I will mark it ready.

@keithlayne keithlayne marked this pull request as ready for review May 6, 2021 18:23
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 6, 2021
@johngmyers
Copy link
Member

@keithlayne we are working on more direct support for rotating keys, targeting kops 1.22. #10516 and #11252 are early attempts. So this particular document might not be relevant for much longer.

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

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: johngmyers

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 May 6, 2021
@k8s-ci-robot k8s-ci-robot merged commit 8409917 into kubernetes:master May 6, 2021
@k8s-ci-robot k8s-ci-robot added this to the v1.21 milestone May 6, 2021
@keithlayne keithlayne deleted the patch-1 branch May 7, 2021 18:30
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. area/documentation 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. lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. 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.

None yet

6 participants