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

Use different registered app for azure credentials; allow use of custom resource groups #11821

Merged
merged 1 commit into from Jul 15, 2020

Conversation

wallyworld
Copy link
Member

Description of change

There's some issues bootstrapping to Azure.
The first is that the "Juju CLI" app in the canonical.com AD appears to have been setup as single tenant only. This means that depending on your subscription tenant, you could not bootstrap since it was not possible to create a service principal off this app. In my case, I could bootstrap but autoload-credentials failed.

The fix is to create a new "Juju" app in the canonical.com AD and have it be multi-tenant. The Juju change is to update the app id which is used to create the user's service principal. A drive by fix autoload-credentials was done to replace " " in detected credential names with "_".

The second issue is that for some accounts, resource groups cannot be created by policy. So we add a new Azure specific model config "resource-group-name". This will use an existing resource group. To make it work, Juju cannot be allowed to create a "default" model at bootstrap or else the same resource group will be used for both. So a new --no-default-model arg was added to bootstrap. We want to get rid of the default model eventually anyway.

There's a problem though - since we cannot tag these "read only" resource groups, destroy-controller cannot see them, so any models which use a custom group name will not have resources cleaned up. This is not easily fixed. It will be a known issue if this type of deployment is used. The user will need to destroy all models and then the controller.

QA steps

install the az cli tool (this will be there anyway if azure used previously)
az logout
az login
juju autoload-credentials
(choose interactive method)

There should be a credential created allowing you to juju bootstrap to azure.

To test the resource group, set up a resource group "juju-test" via the azure dashboard, then

juju bootstrap azure -config resource-group-name=juju-test --no-default-model

The juju-test group should have the controller machine etc in it.
Create a new group juju-test2. Add a model

juju add-model test --config resource-group-name=juju-test2
juju add-machine

The juju-test2 group should have machine and some network items.

juju destroy-model test

The juju-test2 group will have everything deleted but the group will remain behind

Bug reference

https://bugs.launchpad.net/bugs/1885557
https://bugs.launchpad.net/bugs/1869939

Copy link
Member

@ycliuhw ycliuhw left a comment

Choose a reason for hiding this comment

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

lgtm, just one question

@@ -749,9 +749,6 @@ func (cfg *BootstrapConfig) VerifyConfig() (err error) {
if cfg.BootstrapMachineInstanceId == "" {
return errors.New("missing bootstrap machine instance ID")
}
if len(cfg.HostedModelConfig) == 0 {
Copy link
Member

Choose a reason for hiding this comment

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

would be good to have a comment

@@ -1743,6 +1761,41 @@ func (env *azureEnviron) deleteResourceGroup(ctx context.ProviderCallContext, sd
return nil
}

func (env *azureEnviron) deleteResourcesInGroup(ctx context.ProviderCallContext, sdkCtx stdcontext.Context, resourceGroup string) error {
Copy link
Member

@ycliuhw ycliuhw Jul 15, 2020

Choose a reason for hiding this comment

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

should we only delete the resources with model/controller UUID tag?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is the same as the regular behaviour for model destroy, just without the deletion of the resource group itself. All resources for a model are in the resource group.

Copy link
Member Author

Choose a reason for hiding this comment

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

There's issues with deleting just resources with a given model tag - the Azure query APi is very limited in what you can use as an expression, eg you can't filter on tag name and value and resource group or even resource type. So we can't easily identify all the resources for a given model. e will document that the admin needs to create one resource group per model.

@wallyworld
Copy link
Member Author

$$merge$$

@hpidcock hpidcock added the 2.8 label Jul 15, 2020
@jujubot jujubot merged commit 2bfd256 into juju:2.8 Jul 15, 2020
jujubot added a commit that referenced this pull request Jul 20, 2020
#11841

Merge from 2.8 to bring forward:
- #11838 from wallyworld/fstab-uuids
- #11833 from manadart/2.8-link-layer-address-extract
- #11821 from wallyworld/azure-bootstrap-fixes
- #11832 from manadart/2.8-interface-infos-dedup
- #11830 from ycliuhw/fix/lp-1887440
- #11825 from wallyworld/merge-2.7-20200713
- #11828 from achilleasa/2.8-make-manual-cleanup-script-more-robust
- #11826 from SimonRichardson/remove-nbd-integration-tests
- #11824 from hpidcock/fix-operator-imagepath
- #11798 from jujubot/increment-to-2.8.2
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants