-
Notifications
You must be signed in to change notification settings - Fork 510
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
Conversation
…om resource groups
There was a problem hiding this 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 { |
There was a problem hiding this comment.
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 { |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
|
#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
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