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 region check #10949

Merged
merged 1 commit into from Nov 25, 2019
Merged

add region check #10949

merged 1 commit into from Nov 25, 2019

Conversation

nammn
Copy link

@nammn nammn commented Nov 25, 2019

Checklist

  • Checked if it requires a pylibjuju change?
  • Added integration tests for the PR?
  • Added or updated doc.go related to packages changed?
  • Do comments answer the question of why design decisions were made?

Description of change

See the code comment on the place for further reference.
One PR added that we always have at least an empty region in the clouds.yaml. The code, in the past, checked for each region invalid endpoints.
Those endpoints do not get called for clouds not "supporting" them.

QA steps

~/golang/src/juiu/juju/acceptancetests/assess_add_cloud.py ~/cloud-city/example-clouds.yaml ~/golang/bin/juju

e.g. output

Expected fail (bug #1641970): bogus-auth-canonistack, long-endpoint-canonistack, long-endpoint-canonistack-lcy01, long-endpoint-canonistack-lcy02, long-endpoint-finfolk-vmaas
Expected fail (bug #1641981): numeral-prefix-canonistack, numeral-prefix-finfolk-vmaas, slash-in-name-canonistack, slash-in-name-finfolk-vmaas
Failed: none

Copy link
Member

@SimonRichardson SimonRichardson left a comment

Choose a reason for hiding this comment

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

I'm approving this test, but we should work on improving this test, so that it interacts with the CLI to gain the knowledge it needs. Using the clouds.yml is shortsighted, although you can check that it's actually stored in the yaml, the point of this test is to check that juju knows about regions. This should be exposed via the CLI and then uses the CLI as a normal user would. Skipping the latter shows that there could be a gap for operators.

I think we should use juju regions <cloud> --format=json to get what we want!

@nammn
Copy link
Author

nammn commented Nov 25, 2019

@SimonRichardson I agree on using the CLI. I would suggest postponing it to another PR though, as this would touch the client library and therefore touch more than we eventually want to check for now.

$$merge$$

@nammn
Copy link
Author

nammn commented Nov 25, 2019

$$merge$$

@jujubot jujubot merged commit 41f6d85 into juju:develop Nov 25, 2019
@nammn nammn mentioned this pull request Nov 25, 2019
@nammn nammn deleted the fix-clouds-many-region branch November 25, 2019 16:30
jujubot added a commit that referenced this pull request Nov 25, 2019
#10952

### Checklist
(Not really appropriate for a CI test fix backport.)

----

## Description of change

fixed this test in develop (#10949), cherry-picking the fix back to the 2.7 branch so the test passes there too.

## QA steps

Ran the updated test.

## Documentation changes

None

## Bug reference

None
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