add-cloud produces config with redundant region endpoints

Bug #1641130 reported by Aaron Bentley
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Canonical Juju
Expired
Medium
Unassigned

Bug Description

Region endpoints are optional, needed only when a region's endpoint differs from the cloud's main endpoint. add-cloud treats them as mandatory. I initially observed this behaviour in interactive add-cloud, but it seems to apply to non-interactive forms as well.

This produces config that violates "Don't Repeat Yourself" and is therefore harder to read and maintain.

For example, here is the canonistack config the QA team uses:
  canonistack:
    type: openstack
    auth-types: [userpass]
    endpoint: https://keystone.canonistack.canonical.com:443/v2.0/
    regions:
      lcy01: {}
      lcy02: {}

Here is the minimum config that can be produced with add-cloud:
  canonistack:
    type: openstack
    auth-types: [userpass]
    endpoint: https://keystone.canonistack.canonical.com:443/v2.0/
    regions:
      lcy01:
        endpoint: https://keystone.canonistack.canonical.com:443/v2.0/
      lcy02:
        endpoint: https://keystone.canonistack.canonical.com:443/v2.0/

For the latter, the user must enter the same URL three times instead of once, increasing the risk of typos and being less pleasant to use.

For vsphere, add-cloud uses the cloud endpoint as the region endpoint, making redundancy mandatory:
  vsphere:
    type: vsphere
    auth-types: [userpass]
    endpoint: 10.245.0.131
    regions:
      dc0:
        endpoint: 10.245.0.131

There is no need for a region endpoint that matches the cloud endpoint. The QA team successfully uses configs without region endpoints many times a day, so we know that they can work. It is easier to read, because it is obvious that there are no discrepancies between cloud and region endpoint. It is easier to maintain because, should an endpoint need to change, only one change is needed.

For the interactive form, updating vsphere should be trivial. It just needs to stop adding region endpoints at all. For openstack, one way to handle it would be to accept "" (i.e. the user pressing Enter with nothing else on the line) as meaning "Do not specify a custom endpoint for this region."

For the non-interactive form, juju could validate the supplied YAML, and then use it as-is, rather than rewriting it.

Aaron Bentley (abentley)
description: updated
Revision history for this message
Richard Harding (rharding) wrote :

I talked about this with nate and I think the best experience is to have the endpoint provided become the default value for the future region endpoint value.

    Enter the region endpoint url [10.245.0.131]:

In this way, you just hit enter and get a reasonable default value and still allow the user to change it if it's required to something unique.

Revision history for this message
Aaron Bentley (abentley) wrote :

If the user just hits enter, does the generated config have an 'endpoint' value in the region with the same value as the cloud endpoint, or does the region have no specified endpoint so that it falls back to the cloud endpoint?

Revision history for this message
Richard Harding (rharding) wrote :

So the values in the file will have the duplicate data. Since this is meant to be machine read data vs user dealt with data I don't find the duplication of the info offensive. I do find the duplicating of user input an issue though and agree we'll address that side by using the default input value to help that out.

Changed in juju:
milestone: none → 2.1.0
assignee: nobody → Richard Harding (rharding)
Revision history for this message
Aaron Bentley (abentley) wrote :

The format emitted by show-cloud and "clouds --format yaml" is almost identical to this format, so we are expecting humans to read this format.

AFAIK, the interactive add-cloud does not support several things that users might want to add, such as descriptions and "config:" values. I think that this file will be edited by humans as a reasonably common event.

show-cloud and "clouds --format yaml" strip redundant region endpoints, so it is clear that juju considers this formatting more readable.

Given that this file will be read and maintained by humans, I still recommend the more readable and maintainable approach of adding only the endpoints that are specified by the user, rather than duplicating them into the regions.

Changed in juju:
milestone: 2.1.0 → 2.1-beta4
status: Triaged → Fix Committed
Curtis Hovey (sinzui)
Changed in juju:
status: Fix Committed → Fix Released
Aaron Bentley (abentley)
Changed in juju:
status: Fix Released → Triaged
Revision history for this message
Aaron Bentley (abentley) wrote :

This bug has not been fixed. Adding a cloud with 2.1-beta4 still produces redundant regions in the clouds.yaml file.

$ JUJU=/home/abentley/canonical/juju-versions/2.1-beta4/usr/lib/juju-2.1/bin/juju
$ $JUJU --version
2.1-beta4-xenial-amd64
$ $JUJU add-cloud canonistack $JUJU_DATA/example-clouds-minimal.yaml
$ cat $JUJU_DATA/clouds.yaml
clouds:
  canonistack:
    type: openstack
    auth-types: [userpass]
    endpoint: https://keystone.canonistack.canonical.com:443/v2.0/
    regions:
      lcy01:
        endpoint: https://keystone.canonistack.canonical.com:443/v2.0/
      lcy02:
        endpoint: https://keystone.canonistack.canonical.com:443/v2.0/

Revision history for this message
Richard Harding (rharding) wrote :

I'm not sure that I agree. In looking at clouds --format=yaml I see things such as:

---
azure:
  defined: public
  type: azure
  description: Microsoft Azure
  auth-types: [interactive, service-principal-secret]
  regions:
    centralus:
      endpoint: https://management.azure.com
      identity-endpoint: https://graph.windows.net
      storage-endpoint: https://core.windows.net
    eastus:
      endpoint: https://management.azure.com
      identity-endpoint: https://graph.windows.net
      storage-endpoint: https://core.windows.net

The yaml format is intended for a machine readable API and the hope is that users don't need to interact unless there's an issue. At that point, being as clear as possible and reducing assumptions is better. I consider this much like a well documented sample config file that a package installs with. There's a lot of commented out config and hints in that file to help guide a user if they go to that file and edit the config manually. If the user goes into this file, I'd rather have it clear that these can be different urls and reduce any possible confusion.

Revision history for this message
Aaron Bentley (abentley) wrote :

This file is not yet a machine-only API, because users have to manually edit it to get descriptions, per-cloud config values, and potentially other things I haven't noticed.

Anyhow, feel free to mark it "won't fix", but please don't mark it "fix committed"/"fix released".

Revision history for this message
Aaron Bentley (abentley) wrote :

Azure is not a good counterexample. In our official configurations, Azure does not have "endpoint" defined at the cloud level, only the region level, so defining it at the region level is not redundant with the cloud level. If I define endpoint at the cloud level, I get:

azure:
  defined: public
  type: azure
  description: Microsoft Azure
  auth-types: [interactive, service-principal-secret]
  endpoint: https://management.azure.com
  regions:
    centralus:
      identity-endpoint: https://graph.windows.net
      storage-endpoint: https://core.windows.net
    eastus:
      identity-endpoint: https://graph.windows.net
      storage-endpoint: https://core.windows.net

Changed in juju:
milestone: 2.1-beta4 → 2.1.0
Changed in juju:
assignee: Richard Harding (rharding) → nobody
Curtis Hovey (sinzui)
Changed in juju:
milestone: 2.1.0 → 2.1.1
Revision history for this message
Anastasia (anastasia-macmood) wrote :

Removing 2.1 milestone as we will not be addressing this issue in 2.1.

no longer affects: juju/2.2
Changed in juju:
milestone: 2.1.1 → none
Revision history for this message
Canonical Juju QA Bot (juju-qa-bot) wrote :

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

Changed in juju:
status: Triaged → Expired
tags: added: expirebugs-bot
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.