Code review comment for lp:~gz/pyjuju/add_maas_constraints

Revision history for this message
Martin Packman (gz) wrote :

> so a user on the cli is going to do something like
>
> juju deploy xyz --constraints="maas-tags=foo&bar"
>
> except bar doesn't exist, and they'll never see an error, just an
> instance which stays forever in pending state, and errors in the
> provisioning agent log.

Thanks for clarifying, that is a painful scenario. So, this is already an issue with providers that permit constraints that can't be fully validated at the juju cli level before passing on to the provisioning agent? For instance, supplying 'ec2-zone' that is a simble ascii character, but does not exist in the region? Is there a bug filed already for better error reporting in cases like this where the provisioning agent fails to deploy something? Just retrying forever isn't really ideal.

> given that this is effectively free form vocabulary of values, it would
> be nice to see a validation of the constraint value to prevent this.

Okay, I'll go back to the earlier code I had which checked tags were valid at constraint creation. It's a little painful because juju can't sensibly cache the list of tags, as the scenario where someone creates a new tag using the maas cli client then wants to juju deploy a service constrained on that tag immediately seems quite likely. As convert callbacks aren't deferred, it's not possible to recheck for a newly added tag on the fly.

> https://codereview.appspot.com/6569053/diff/1/juju/providers/maas/maas.py#newc
> ode48
> juju/providers/maas/maas.py:48: ("maas-name", "name"),
> is maas-name still supported?

I see no reason to drop it as part of this branch, and the underlying code in MaaS is still present for now. It's simply not useful if you want to use other constraints.

« Back to merge proposal