Ask user to be specific when there is a VPC

[TibsAtWork]

If the user is creating/updating a service in a cloud that has a VPC, and they have specified the cloud with --cloud, and they have not specified either --project-vpc-id=<id> or --no-project-vpc, then stop with an error asking them to do one or the other.

Fixes: #290

About this change: What it does, why it matters

When a new service is created at the console, it will be created in "Public Internet" even if there is a VPC available.

When avn service create is used to create a new service, it will default to using a VPC if there is one available in the target cloud. This is unexpected, and can lead to (for instance):

avn service cli <pg-service-name>

giving an error like

psql: error: could not translate host name "<pg-service-name>" to address: nodename nor servname provided, or not known

This change refuses to do either when there is a VPC for the requested cloud - the user is requested to be specific about which alternative they wanted.

If the user doesn't specify a cloud (which is common for service update when the service is not being moved, and allowed for service creation), then we don't require the VPC switches.

Note: There are some additional changes to make mypy happy with the existing code - I've added -> None in appropriate places.

Testing

There are new tests at the end of tests/test_cli.py. Given a change of this nature, actual tests seemed necessary. In the longer term, it may not be necessary to keep them, as this functionality is unlikely to change.

I've also tested manually. Using project dev-sandbox I did:

• python -m aiven.client service create --service-type pg --cloud google-europe-north1 --plan hobbyist tibs-pg-vanilla

  That says

  ERROR	command failed: UserError: Cloud google-europe-north1 has a VPC. Specify --project-vpc-id=<vpc-id> to use it, or --no-project-vpc to use the public internet
  

  which is what we want.

• python -m aiven.client service create --service-type pg --cloud google-europe-north1 --plan hobbyist tibs-pg-novpc --no-project-vpc

  That created a service with "Public Internet", obeying the switch.

• python -m aiven.client service create --service-type pg --cloud google-europe-north1 --plan hobbyist tibs-pg-vpc --project-vpc-id=c6a08e40-778c-40b3-bfaf-17a3d3800ab7

  That created a service with "Project VPC", obeying the switch

• python -m aiven.client service create --service-type pg --cloud google-europe-west2 --plan hobbyist tibs-pg-also-no-vpc

  That created a service with "Public Internet", as google-europe-west2 is not in the list reported by avn vpc list

Further testing:

• python -m aiven.client service update tibs-pg-novpc --cloud google-europe-west1

  This says:

  ERROR	command failed: UserError: Cloud google-europe-west1 has a VPC. Specify --project-vpc-id=<vpc-id> to use it, or --no-project-vpc to use the public internet
  

  which seems sensible as we don't know whether the user wanted a VPC on that new cloud or not.

• python -m aiven.client service update tibs-pg-novpc --plan startup-4

  This should just change the plan - it doesn't matter whether there is a VPC in the cloud if we're not changing that. And it behaves as expected.

• python -m aiven.client service create --service-type pg --plan hobbyist tibs-pg-guess

  When we create a new service without specifying the cloud, we expect to get some sort of cached decision for the cloud. In this case, it created a service in europe-north1 with "Project VPC". Since it's not documented what happens in this case, I'm happy to ignore it for the purpose of this PR. (The previous create was in google-europe-west2, and the one before that was google-europe-north1, with VPC, so this what to expect is not obvious, and I assume is related to how the backend service works.)

[rikonen]

Looks good to me for the service creation part but for service update this would now cause an error when not explicitly specifying the VPC related parameters even if also cloud is not specified. That would be annoying as the old behavior in that case would've been not to touch the VPC/cloud config at all and only update whatever fields were provided but now you'd be forced to always pass the extra parameters.

[TibsAtWork]

Thanks for the comment - this is where experience with the tool is useful. I'm going to "think out loud" below to check my understanding...

Looking at behaviour of the existing client:

• avn project switch dev-sandbox

• avn service create tibs-demo-pg --service-type pg --cloud google-europe-north1 --plan hobbyist --no-project-vpn

  Gives "Public Internet" as expected.

• avn service update tibs-demo-pg --cloud google-europe-west1

  Now we've got "Project VPC", which is the surprise we don't want. Let's put it back to the original state again:

• avn service update tibs-demo-pg --cloud google-europe-north1 --no-project-vpc

  We're back to europe-north-1 and "Public Internet"

• avn service update tibs-demo-pg --plan startup-4

  Which leaves us on the same cloud, with "Public Internet", and just changes the plan.

I also wondered what would happen if I didn't give the --cloud switch to avn service create:

• avn service create tibs-demo-pg --service-type pg --plan hobbyist --no-project-vpc

  And that created me a service in europe-north1 with "Public Internet" - obviously some state is being remembered somewhere. Presumably it uses the last cloud specification I used for some sort of action.

So I do see that your point is correct and needs fixing, and it also looks as if it may apply to service creation as well.

[TibsAtWork]

I shall be be on holiday until Monday 2022-06-13, and won't be able to look at this again until then. Thanks for the very useful comments so far!

[TibsAtWork]

@rikonen I'm back from holiday, and also back from being ill after that (not related!), so would appreciate you and the Aiven Client team looking at this to see if it makes sense.

[rikonen]

Latest version looks good logic-wise but should remove the debug prints and squash commits.

[TibsAtWork]

Oh goodness - thanks for spotting the debug prints - I thought I'd removed those (shows how hard it is to read one's own code). And happy to squash.

[TibsAtWork]

@rikonen Hopefully that looks rather better.