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

Sleep wake cli #1983

Merged
merged 1 commit into from
Jul 29, 2024
Merged

Sleep wake cli #1983

merged 1 commit into from
Jul 29, 2024

Conversation

zerbitx
Copy link
Contributor

@zerbitx zerbitx commented Jul 24, 2024

What issue type does this pull request address? (keep at least one, remove the others)
/kind enhancement

What does this pull request do? Which issues does it resolve? (use resolves #<issue_number> if possible)
resolves #ENG-4157
resolves #ENG-4158
resolves #ENG-4159

Please provide a short message that should be published in the vcluster release notes
Sleep and wake features used across drivers (platform/helm) are more intelligent when the --driver flag is omitted, still providing useful information for failures if the wrong driver is explicitly used from the command.

Copy link

netlify bot commented Jul 24, 2024

Deploy Preview for vcluster-docs ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit d72f269
🔍 Latest deploy log https://app.netlify.com/sites/vcluster-docs/deploys/66a7d43234940200085a0458
😎 Deploy Preview https://deploy-preview-1983--vcluster-docs.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.

@zerbitx zerbitx requested a review from lizardruss July 24, 2024 15:57
@zerbitx zerbitx marked this pull request as draft July 24, 2024 16:51
@zerbitx zerbitx marked this pull request as ready for review July 24, 2024 16:52
Copy link
Contributor

@lizardruss lizardruss left a comment

Choose a reason for hiding this comment

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

Some nits, and a question about erroring when the user choosed not to wake up the vcluster during add.

pkg/cli/add_vcluster_helm.go Outdated Show resolved Hide resolved
pkg/cli/add_vcluster_helm.go Outdated Show resolved Hide resolved
pkg/cli/add_vcluster_helm.go Outdated Show resolved Hide resolved
@zerbitx zerbitx requested a review from lizardruss July 25, 2024 20:38
pkg/platform/helper.go Outdated Show resolved Hide resolved
pkg/cli/find/find.go Outdated Show resolved Hide resolved
@zerbitx zerbitx requested a review from facchettos July 29, 2024 15:22
Copy link
Contributor

@lizardruss lizardruss left a comment

Choose a reason for hiding this comment

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

One suggestion for re-wording. If the wording has already been reviewed / approved then we can disregard.

pkg/cli/pause_helm.go Outdated Show resolved Hide resolved
…rrect commands to wake it up

Warns allows waking and adding a sleeping helm vcluster in a single command.
Adds the ability to set chart dir for local dev when adding external cluster.
Only errors if helm driver is specified, otherwise it falls back to a platform resume.
Remove old check due to platform's auto conversion of helm slept clusters.
Continue with adding the secret in either case and only add if they choose to wake the cluster.

Co-authored-by: Russell Centanni <russell.centanni@gmail.com>
Copy link
Contributor

@lizardruss lizardruss left a comment

Choose a reason for hiding this comment

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

lgtm

@zerbitx zerbitx merged commit 786879c into loft-sh:main Jul 29, 2024
60 checks passed
@zerbitx zerbitx added the backport-to-v0.20 backport this PR to v0.20 branch label Jul 29, 2024
@loft-bot
Copy link
Collaborator

💔 All backports failed

Status Branch Result
v0.20 Backport failed because of merge conflicts

Manual backport

To create the backport manually run:

backport --pr 1983

Questions ?

Please refer to the Backport tool documentation and see the Github Action logs for details

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-to-v0.20 backport this PR to v0.20 branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants