Merge lp:~axwalk/juju-ci-tools/juju-ci-tools into lp:juju-ci-tools

[Andrew Wilkins]

Use qualified model name in grant/revoke test

If you want to refer to another user's model,
you need to qualify the model with the model
owner's name. e.g. if I'm billie, and I want
to see cath's model foo, then I need to refer
to it as "cath/foo". Plain old "foo" is short
for "billie/foo".

Use qualified model name in grant/revoke test

If you want to refer to another user's model,
you need to qualify the model with the model
owner's name. e.g. if I'm billie, and I want
to see cath's model foo, then I need to refer
to it as "cath/foo". Plain old "foo" is short
for "billie/foo".

[Andrew Wilkins (axwalk)]

This is so we can land axw/cli-model-owner into juju again, fixing https://bugs.launchpad.net/juju-core/+bug/1605710.

[Martin Packman (gz)]

Thanks for doing this Andrew. Poked Nicholas to have a look yesterday, but he's in sprint-mode I guess. He was sure that there was another issue as well beyond just the model naming, but I didn't get the details out of him.

The basic change here seems right, but seems like this really needs to be in jujupy. That can probably happen in a later branch. Does the change work on master at present? I expect it should.

review: Needs Information

[Andrew Wilkins (axwalk)]

> Thanks for doing this Andrew. Poked Nicholas to have a look yesterday, but
> he's in sprint-mode I guess. He was sure that there was another issue as well
> beyond just the model naming, but I didn't get the details out of him.
>
> The basic change here seems right, but seems like this really needs to be in
> jujupy. That can probably happen in a later branch. Does the change work on
> master at present? I expect it should.

Urgh, actually no it won't. Can we break CI temporarily, while the juju PR is pending? I'm not sure how easy it'll be to detect/keep compat. It's just for a beta as well...

[Andrew Wilkins (axwalk)]

> Thanks for doing this Andrew. Poked Nicholas to have a look yesterday, but
> he's in sprint-mode I guess. He was sure that there was another issue as well
> beyond just the model naming, but I didn't get the details out of him.

Also: I ran the test that was failing, and it passes with this patch and my juju PR back in place.

[Nicholas Skaggs (nskaggs)]

As suggested, please move this to jujupy and base against https://code.launchpad.net/~nealpzhang/juju-ci-tools/assess_multi_users/+merge/299463 which is landing today. You'll need to make a new EnvJujuClientB14 to support the change.

review: Needs Fixing

[Nicholas Skaggs (nskaggs)]

For those watching this MP, the real bug here is this; juju register didn't work.

----------------
balloons: if you added a user with access to exactly one model, register would try to switch to it but failed to qualify its name

In addition, Andrew noted the test passed with Leo's new branch, making this branch not needed. We need to ensure the regression is still noted.

-------------
balloons: so to answer your question about why it wasn't failing: the test wasn't checking the exit status of the "juju register" pexpect process. register is failing right at the end, when it goes to set the current model. so the controller is set up, and further commands succeed because they're specifying a model with "-m"

[Andrew Wilkins (axwalk)]

> For those watching this MP, the real bug here is this; juju register didn't
> work.
>
> ----------------
> balloons: if you added a user with access to exactly one model, register would
> try to switch to it but failed to qualify its name
>
>
> In addition, Andrew noted the test passed with Leo's new branch, making this
> branch not needed. We need to ensure the regression is still noted.

I no longer know up from down, I ran the test on Leo's branch again and it failed with my juju PR in place. I'm working on a fix still.

> -------------
> balloons: so to answer your question about why it wasn't failing: the test
> wasn't checking the exit status of the "juju register" pexpect process.
> register is failing right at the end, when it goes to set the current model.
> so the controller is set up, and further commands succeed because they're
> specifying a model with "-m"

And I'll update the test code to check that the register process returns with success.

[Martin Packman (gz)]

This has now landed via a later change by Andrew and Chris.

Unmerged revisions

1517. By Andrew Wilkins on 2016-07-25

Use qualified model name in grant/revoke test

If you want to refer to another user's model,
you need to qualify the model with the model
owner's name. e.g. if I'm billie, and I want
to see cath's model foo, then I need to refer
to it as "cath/foo". Plain old "foo" is short
for "billie/foo".

=== modified file 'assess_user_grant_revoke.py'
--- assess_user_grant_revoke.py	2016-07-06 17:55:43 +0000
+++ assess_user_grant_revoke.py	2016-07-25 12:37:16 +0000
@@ -75,6 +75,15 @@
     user_client.env.juju_home = cloned_juju_home
     # New user names the controller.
     user_client.env.controller = Controller(controller_name)
+    # The client's environment name needs to be made absolute for the
+    # cloned client to be able to refer to it. i.e. default -> admin/default.
+    for m in client.get_models()['models']:
+        name = m['name']
+        owner = m['owner']
+        if name == client.env.environment:
+            model_name = "{}/{}".format(owner, name)
+            user_client.env.environment = model_name
+            break
     user_client_env = user_client._shell_environ()
     return user_client, user_client_env