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

Fixes lp#1830949: add-k8s validation for user supplied cloud/region #10523

Merged
merged 2 commits into from Aug 15, 2019

Conversation

anastasiamac
Copy link
Contributor

@anastasiamac anastasiamac commented Aug 14, 2019

Description of change

'add-k8s' tries to detect cloud/region. However, for the cases when the command cannot detect the cloud, it relies on user input.

Previously, if the user supplied cloud/region but the command successfully detected what cloud/region to use, user input was ignored and not even validated.

This PR checks user input when cloud/region is detected and if there is a mismatch, the user is prompted.

As a drive-by:

  • Ensure that the command no longer ignores --local
  • When cloud region validation fails, we list available regions. However, some clouds may not have regions (see lp#1819409) and displaying empty list is not helpful. This PR displays a better message.

Bug reference

https://bugs.launchpad.net/juju/+bug/1830949
https://bugs.launchpad.net/juju/+bug/1839898

@anastasiamac
Copy link
Contributor Author

add-k8s UX alignment

Copy link
Member

@wallyworld wallyworld left a comment

Choose a reason for hiding this comment

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

Thanks for fixing this wart

if givenRegion != "" || givenCloud != detectedRegion {
// If givenRegion is empty, then givenCloud may be a region.
// Check that it is not a region.
return errors.Errorf("given cloud %q was different to the detected cloud %q: re-run the command without specifying the cloud", givenCloud, detectedCloud)
Copy link
Member

Choose a reason for hiding this comment

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

s/given/specified I think

@anastasiamac
Copy link
Contributor Author

$$merge$$

@jujubot jujubot merged commit efb9d09 into juju:develop Aug 15, 2019
@anastasiamac anastasiamac deleted the addk8s-input-validation branch August 15, 2019 00:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants