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

connect fixes #461

Merged
merged 5 commits into from
Apr 15, 2024
Merged

connect fixes #461

merged 5 commits into from
Apr 15, 2024

Conversation

avalanche123
Copy link
Contributor

@avalanche123 avalanche123 commented Apr 12, 2024

Description of your changes

  • upgrade connect agent version and helm values
  • always reinstall the agent after attaching
  • refactor attach/detach flows

Fixes upbound/connect#179

Additional flows that are enabled by this PR:

Detach space when no connect configmap exists in the space cluster but space name is specified.

This would previously exit without doing anything, instead the flow is now:

% up alpha space detach space-bulat-test
 INFO  ConfigMap "upbound-system/space-token" not found

 WARNING  We're unable to confirm Space "upbound-papper-cutters/space-bulat-test" is currently connected to Upbound Console. Would you like to delete it anyway?

            If the other Space cluster still exists, the Upbound agent will be left running and you will need to delete it manually.


 ▀ Continue? (Y/n) (12s)

 INFO  Looking for Robot "upbound-papper-cutters/space-bulat-test"
 INFO  Deleting Robot "upbound-papper-cutters/space-bulat-test"
 INFO  Robot "upbound-papper-cutters/space-bulat-test" deleted
 INFO  Deleting Space "upbound-papper-cutters/space-bulat-test"
 INFO  Uninstalling Chart "upbound-system/agent"
 INFO  Chart "upbound-system/agent" uninstalled
 INFO  Deleting Secret "upbound-system/space-token"
 INFO  Secret "upbound-system/space-token" deleted
  ✓   Space "upbound-papper-cutters/space-bulat-test" has been successfully disconnected from Upbound Console

Attach a space that is currently attached under a different name.

Previously this flow would create a new space in Upbound console and leave the old Space quietly broken.
The new behavior is as follows:

% up alpha space attach space-bulat-test-2
 INFO  ConfigMap "upbound-system/space-connect" found, resuming...

 WARNING  Space "upbound-papper-cutters/space-bulat-test" is currently connected to Upbound Console, would you like to disconnect it first?

            This will remove it and re-connect the Upbound agent with Space "upbound-papper-cutters/space-bulat-test-2" instead.


▄  Continue? (Y/n) (11s)

 INFO  Looking for Robot "upbound-papper-cutters/space-bulat-test"
 INFO  Deleting Robot "upbound-papper-cutters/space-bulat-test"
 INFO  Robot "upbound-papper-cutters/space-bulat-test" deleted
 INFO  Deleting Space "upbound-papper-cutters/space-bulat-test"
 INFO  Creating Space "upbound-papper-cutters/space-bulat-test-2"
 INFO  Space "upbound-papper-cutters/space-bulat-test-2" created
 INFO  Creating Robot "upbound-papper-cutters/space-bulat-test-2"
 INFO  Robot "upbound-papper-cutters/space-bulat-test-2" created
 INFO  Creating a new Token for Robot "upbound-papper-cutters/space-bulat-test-2"
 INFO  Token "319b6cf2-2bca-46de-890f-8cde496aae19" created
 INFO  Creating Secret "upbound-system/space-token"
 INFO  Secret "upbound-system/space-token" exists, updating...
 INFO  Secret "upbound-system/space-token" updated
 INFO  Reinstalling Chart "upbound-system/agent" 0.0.0-309.gd29fc13
  ✓   Space "space-bulat-test-2" is connected to Upbound Console

I have:

  • Read and followed Upbound's contribution process.
  • Run make reviewable to ensure this PR is ready for review.
  • Added backport release-x.y labels to auto-backport this PR, as appropriate.

How has this code been tested

Ran the attach/detach multiple times, see description.

Copy link
Contributor

@cbuto cbuto left a comment

Choose a reason for hiding this comment

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

lgtm, a few minor wording suggestions that are non-blocking

if (space.Name != "" && space.Name != name) || space.Namespace != ns {
attachSpinner.UpdateText("Continue? (Y/n)")
if err := warnAndConfirm(
`Space "%s/%s" is currently connected to Upbound Console, would you like to disconnect it first?`+"\n\n"+
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe something like:

Space "<space name>" is currently connected to the Upbound Console, by continuing the current Space will be removed and this Space will be attached as "<space name>". Would you like to continue?

cmd/up/space/detach.go Outdated Show resolved Hide resolved
@avalanche123 avalanche123 merged commit d5bf1ac into main Apr 15, 2024
6 checks passed
@avalanche123 avalanche123 deleted the connect-fixes branch April 15, 2024 19:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants