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

✨ Add re-creation of bastion host on change #1303

Merged

Conversation

tobiasgiese
Copy link
Member

@tobiasgiese tobiasgiese commented Jul 19, 2022

Signed-off-by: Tobias Giese tobias.giese@mercedes-benz.com

Thanks to @chrischdi for the discussion and hash annotation idea! 🙂

TODO:

  • test bastion re-creation
  • add docs
  • e2e test? (we will add envtests after Envtest mocks #1236 is merged)

What this PR does / why we need it:

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #1298

Special notes for your reviewer:

  1. Please confirm that if this PR changes any image versions, then that's the sole change this PR makes.

TODOs:

  • squashed commits
  • if necessary:
    • includes documentation
    • adds unit tests

/hold

Tobias Giese tobias.giese@mercedes-benz.com, Mercedes-Benz Tech Innovation GmbH, legal info/Impressum

@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. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. labels Jul 19, 2022
@netlify
Copy link

netlify bot commented Jul 19, 2022

Deploy Preview for kubernetes-sigs-cluster-api-openstack ready!

Name Link
🔨 Latest commit 9c9365e
🔍 Latest deploy log https://app.netlify.com/sites/kubernetes-sigs-cluster-api-openstack/deploys/62d911295a3f970008c98b3e
😎 Deploy Preview https://deploy-preview-1303--kubernetes-sigs-cluster-api-openstack.netlify.app/print
📱 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 settings.

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jul 19, 2022
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: tobiasgiese

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 requested review from jichenjc and mdbooth July 19, 2022 10:23
@k8s-ci-robot k8s-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jul 19, 2022
@tobiasgiese tobiasgiese force-pushed the tobiasgiese/bastion-recreation branch 7 times, most recently from 5a5dcd3 to 132a2dc Compare July 19, 2022 15:18
@tobiasgiese tobiasgiese changed the title [WIP] ✨ Add re-creation of bastion host on change ✨ Add re-creation of bastion host on change Jul 19, 2022
@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 Jul 19, 2022
@tobiasgiese tobiasgiese force-pushed the tobiasgiese/bastion-recreation branch 4 times, most recently from 30b9f71 to c82503e Compare July 19, 2022 16:04
@tobiasgiese
Copy link
Member Author

/retest

Copy link
Contributor

@seanschneeweiss seanschneeweiss left a comment

Choose a reason for hiding this comment

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

Lovely implementation. Thanks.
/lgtm

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

@jichenjc jichenjc left a comment

Choose a reason for hiding this comment

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

/lgtm

controllers/openstackcluster_controller.go Show resolved Hide resolved
The hash package computes the hash of a InstanceSpec using the spew library.

Signed-off-by: Tobias Giese <tobias.giese@mercedes-benz.com>
Signed-off-by: Tobias Giese <tobias.giese@mercedes-benz.com>
Signed-off-by: Tobias Giese <tobias.giese@mercedes-benz.com>
@tobiasgiese tobiasgiese force-pushed the tobiasgiese/bastion-recreation branch from c82503e to 9c9365e Compare July 21, 2022 08:41
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 21, 2022
@tobiasgiese
Copy link
Member Author

had to rebase @seanschneeweiss / @jichenjc

@jichenjc
Copy link
Contributor

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 21, 2022
@tobiasgiese
Copy link
Member Author

/unhold

@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 Jul 21, 2022
@k8s-ci-robot k8s-ci-robot merged commit f040fc4 into kubernetes-sigs:main Jul 21, 2022
@tobiasgiese tobiasgiese deleted the tobiasgiese/bastion-recreation branch July 24, 2022 09:25
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/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

✨ bastion should be mutable
4 participants