[k8s] add-k8s command has ambigious UX

[Peter Matulis]

juju bootstrap aws aws
juju deploy kubernetes-core
juju deploy -n 3 ceph-mon --constraints mem=1G
juju deploy -n 3 ceph-osd --storage osd-devices=32G,2 --storage osd-journals=8G,1 --constraints mem=1G
juju add-relation ceph-osd ceph-mon
juju add-relation ceph-mon:admin kubernetes-master
juju add-relation ceph-mon:client kubernetes-master
juju scp kubernetes-master/0:config ~/.kube/config

By mistake I specified cloud type 'gce' when adding this cluster to Juju:

juju add-k8s --local --region=gce/us-east1 --storage=ceph-ext4 aws-cluster

Yet it still worked:

k8s substrate "gce/us-east1" added as cloud "aws-cluster" with storage provisioned
by the existing "ceph-ext4" storage class
operator storage provisioned by the workload storage class.
You can now bootstrap to this cloud by running 'juju bootstrap aws-cluster'.

Further testing shows that any string for cloud type will work:

juju add-k8s --local --region=xxblahxx/us-east1 --storage=ceph-ext4 aws-cluster

as long as a string is included:

juju add-k8s --local --region=/us-east1 --storage=ceph-ext4 aws-cluster

ERROR validating cloud region "/us-east1": cloud region "/us-east1" not valid

[Ian Booth (wallyworld)]

"gce" is actually the correct thing to type so it was not a mistake.

If xxblahxx is typed that should result in an error.

[Ian Booth (wallyworld)]

I realise I didn't notice the initial bootstrap was to aws.
So to clarify some more, the need for manually specifying cloud/region is only needed if juju cannot detect automatically. For the public clouds with CDK and the integrator charm, or the public k8s clusters, or openstack with the integrator charm, the detection will be automatic. I say "will be" because there's some remaining work I think on the integrator charm side.
Juju needs to know the cloud and region so it can make choices about using recommended/preferred storage for the cloud type, as well as for jaas it need to be able to direct the add-k8s request to the correct controller.

[Peter Matulis (petermatulis)]

Ok, but whatever string I supply ('gce', 'xxxblahxxx', etc), in this scenario at least, doesn't make any difference. The command will complete, which is strikingly odd.

I'm also unclear whether the integrator charm is even needed in this scenario (using non-AWS native storage, such as Ceph in this case).

[Anastasia (anastasia-macmood)]

When a user is required to supply a cloud, it might be worthwhile validating it against user clouds on the controller.

[Yang Kelvin Liu (kelvin.liu)]

Juju will try to figure out which cloud it is on.
In above scenario, I think, Juju already figured out the cloud by itself, so the cloud provided from the option was ignored.
Yes, in this case, we probably should print a 'warn' message like:
"Juju detected the cloud is "ec2" but the cloud provided was 'gce' and it's wrong, so will be ignored".

[Anastasia (anastasia-macmood)]

The problem here is not a missing message. It's not even the missing cloud validation. But the inherent ambiguity of the command's UX:

1. The options allow to specify both cloud and region where both values can be cloud and/or region! In addition to being confusing, it's not consistent with any of Juju other commands - we either ask for a cloud and region (where each accepts only cloud or region name respectively) or we ask for cloud and/or region together but only ONCE.

2. The command is also capable of "guessing" the cloud... Why ask the user then? The order of precedence is not clear... I assume that if we guess the cloud, we'd use it? We need to message that to the user better then. I am also guessing that if Juju could not figure out the cloud, it'll use the one provided... In which case in addition to messaging to the user what we are using, we also need to validate that we can use provided cloud AND that provided region is valid, if provided.

3. It's never great to quietly ignore user provided information. In this case, if the user is provided a cloud, we should at least let them know that we are ignoring it. Better yet, we'd err out stating that the cloud we think we should be using ('blah') is not the cloud that user provided.

[Anastasia (anastasia-macmood)]

 https://github.com/juju/juju/pull/10511

[Anastasia (anastasia-macmood)]

Above commit is obsolete. Different solution will be implemented.

[Anastasia (anastasia-macmood)]

PR: https://github.com/juju/juju/pull/10523