add-credential/cloud should have option for parseable ERROR output

[Joshua Genet]

Just to preface (since I'm doing 2.7 release testing right now), this isn't a release-blocking bug.

I'll use add-credential for this example, but it also applies to add-cloud. It may also apply to other commands.

Our automation passes a credentials file into add-credential. Thus we have no knowledge of the credential name (ubuntu, steve, jim, etc). If add-credential causes an 'already exists' error, we use update-credential instead. Since we do not know the credential name, we have to parse it from the plain-English ERRROR output. It would be much better if we had a stable API (similar to --format=yaml) to parse this from.

[Jason Hobbs (jason-hobbs)]

Or, some other way to discover the credential name in a parseable manner

[Anastasia (anastasia-macmood)]

@Joshua Genet,

Just out of curiosity... Don't you *know* the credential (or cloud) name already? I am sure it is one of the parameters that the operation requires.

I am not sure why you need to parse the ERROR output to discover this information if you are the one that has provided it in the first place ... :)

[Joshua Genet (genet022)]

Good question. Our parameter is the path to credentials.yaml. Not the actual credential name. There may be a better way to do this that I'm not thinking of, but here's pseudocode for what we're doing:

try:
    juju add-credential <cloud> -f <path>
except:
    if credential already exists:
        juju update-credential <cloud> <credential_name>

The problem is that update-credential requires <credential_name>. Whereas add-credential does not. We only know <path> and <cloud>. We could try to parse <credential_name> from the yaml, but it seems that juju already has the logic to do that (and the abillity to ignore the other valid yaml fields such as 'default-region')

[Anastasia (anastasia-macmood)]

I see...
We have talked about potentially adding a behavior to our 'update-*' command to also cater for 'add-*' or 'update-*' scenario. At this stage, we are thinking it's best provided via an additional option. Something along the line of:

'juju update-credential <cloud> <credential_name> --force-add'

The argument was that the notions of 'insert' and 'upsert' are fairly useful in databases. So Juju CRUD should allow for similar flexibility.

We do not want to lose the ability to err out when users are trying to 'add' something that already exists. So we did not want to change the behavior of 'add-*' commands.

We also do not want to lose the ability to err out when users are trying to update something that does not yet exist. So we did not want to change the default behavior of 'update-*' commands. However, adding to that behavior sounds feasible and not conflicting.

This will address both your previous concern about similar ability originally provided by 'add-* --replace' as well as eliminate the need for parsing errors to get a 'thing' name :D

[Richard Harding (rharding)]

Could we soak do this though by having update credential feed a list of
credentials by file like add-credential and read the bane from the file
like we wound in that case?

Maybe it's more add-credential --or-update -f...

On Thu, Nov 7, 2019, 6:45 PM Anastasia <email address hidden>
wrote:

> I see...
> We have talked about potentially adding a behavior to our 'update-*'
> command to also cater for 'add-*' or 'update-*' scenario. At this stage, we
> are thinking it's best provided via an additional option. Something along
> the line of:
>
> 'juju update-credential <cloud> <credential_name> --force-add'
>
> The argument was that the notions of 'insert' and 'upsert' are fairly
> useful in databases. So Juju CRUD should allow for similar flexibility.
>
> We do not want to lose the ability to err out when users are trying to
> 'add' something that already exists. So we did not want to change the
> behavior of 'add-*' commands.
>
> We also do not want to lose the ability to err out when users are trying
> to update something that does not yet exist. So we did not want to
> change the default behavior of 'update-*' commands. However, adding to
> that behavior sounds feasible and not conflicting.
>
> This will address both your previous concern about similar ability
> originally provided by 'add-* --replace' as well as eliminate the need
> for parsing errors to get a 'thing' name :D
>
> --
> You received this bug notification because you are subscribed to juju.
> https://bugs.launchpad.net/bugs/1851694
>
> Title:
> add-credential/cloud should have option for parseable ERROR output
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/juju/+bug/1851694/+subscriptions
>

[Anastasia (anastasia-macmood)]

@Richard Harding (rharding),

For update, we did not want to read the names from the file since they can mismatch on the controller. Although, of course, we can make that change, i.e. "if the credential name is not provided but the file is, use names from the file". Current behavior is the left over from the time when 'update-credential' did not take a file and the user had to provide the name of the credential from the client cache to be used during the update.

I'd rather we did not go back to polluting the behavior of an 'add-'. As per previous comment and the industry standards, 'add-' should always be clean - add new or if *it* exists, err out. However, the 'update-*' can gain the 'add-or-update' behavior while preserving the crisp default 'if only updating and *it* does not exist, err out'.

[Anastasia (anastasia-macmood)]

Due to current priorities, I am not currently working on this bug, so I am un-assigning myself from it.

[Canonical Juju QA Bot (juju-qa-bot)]

This bug has not been updated in 2 years, so we're marking it Low importance. If you believe this is incorrect, please update the importance.