search

canonical / cloud-init

Official upstream for the cloud-init: cloud instance initialization
https://cloud-init.io/
Other
2.85k stars 854 forks source link

[enhancement]: rethink cloud-init schema cli UI #5457

Open holmanb opened 2 months ago

holmanb commented 2 months ago

Enhancement

Currently cloud-init schema --annotate has useful output. However, cloud-init schema's output has much to be desired. Compare:

annotate

# cloud-init schema --system --annotate
Found cloud-config data types: user-data, network-config

1. user-data at /var/lib/cloud/instances/cb88d1af-f827-4381-bc89-4fad787bcaaf/cloud-config.txt:
#cloud-config

# from 1 files
# part-001

---
grub-dpkg:      # D1
    enabled: false
users:
-   lock-passwd: false      # D2
    name: osadmin
    ssh-authorized-keys:        # D3
    - ssh-rsa "AAAAB3NzaC1yc2EAAAADAQABAAABAQCc/K1T62elOkJKc94sWovn06RE7D274Fxx5n+eT/oClAwk7632awMtWjO+lIRpzOlEcYCR6wexlmL9mZNEFJoROMMRyCxRilNAiDKOq0X5j1HXY2ky9KjtLxo2b8yXFRBcYKK4W8aHU94NkNek81jdi9lC+L6jUrZ25d7xylqnEbd4TgrBphiowdh5B3RY+j6ePthLWWhHopRcVDgq91yYacelCCIfSDaGBya9iXPZSwoKtPin4n5PdSGmSOA3fIkvV5JjLS1QbuY5tzhXKuH9MdAqrvutF1bprs/GDf0IY8DO7DDoo8rXjhhgGtCdg1i5UitZIAM3hn/Vk5F//5eJ
        JOOS8312@EB-OR6120158"
    sudo:
    - ALL=(ALL) NOPASSWD:ALL
...

# Deprecations: -------------
# D1: An alias for ``grub_dpkg`` Deprecated in version 22.2. Use ``grub_dpkg`` instead.
# D2: Default: ``true`` Deprecated in version 22.3. Use ``lock_passwd`` instead.
# D3:  Deprecated in version 18.3. Use ``ssh_authorized_keys`` instead.

  Valid schema user-data

2. network-config at /var/lib/cloud/instances/cb88d1af-f827-4381-bc89-4fad787bcaaf/network-config.json:
  Valid schema network-config

non-annotate

# cloud-init schema --system
Found cloud-config data types: user-data, network-config

1. user-data at /var/lib/cloud/instances/cb88d1af-f827-4381-bc89-4fad787bcaaf/cloud-config.txt:
Cloud config schema deprecations: grub-dpkg: An alias for ``grub_dpkg`` Deprecated in version 22.2. Use ``grub_dpkg`` instead., users.0.lock-passwd: Default: ``true`` Deprecated in version 22.3. Use ``lock_passwd`` instead., users.0.ssh-authorized-keys:  Deprecated in version 18.3. Use ``ssh_authorized_keys`` instead.
  Valid schema user-data

2. network-config at /var/lib/cloud/instances/cb88d1af-f827-4381-bc89-4fad787bcaaf/network-config.json:
  Valid schema network-config

Without --annotate, the output prints the keys that are invalid and the annotations, but for some reason also prints the unrendered message output. Adding the command message might be useful, but the purpose of this command is validating inputs, not telling the user what the purpose of input keys is.

I would recommend the following improvements:

1) deprecate the --annotate flag, and make this behavior the default 2) remove the file path - this is an implementation detail that the user doesn't need access to and is a distraction to the task at hand: validate inputs 3) remove markdown characters from output - the double backticks all over the place are harsh on the eyes 4) add color - a flag --color=[auto|always|never] which defaults to auto and adds color to the output when stdout is a tty could make this look really nice (a common default in tools like grep). Something that makes the # D1 a different color from the message text and possibly even some basic yaml syntax highlighting could make this tool a pleasure to work with, not just "does the job". Consider how much easier the following highlighted version is on the eyes compared to the above (intentionally not highlighted) tools. 

# cloud-init schema --system --annotate
Found cloud-config data types: user-data, network-config

1. user-data at /var/lib/cloud/instances/cb88d1af-f827-4381-bc89-4fad787bcaaf/cloud-config.txt:
#cloud-config

# from 1 files
# part-001

---
grub-dpkg:      # D1
    enabled: false
users:
-   lock-passwd: false      # D2
    name: osadmin
    ssh-authorized-keys:        # D3
    - ssh-rsa "AAAAB3NzaC1yc2EAAAADAQABAAABAQCc/K1T62elOkJKc94sWovn06RE7D274Fxx5n+eT/oClAwk7632awMtWjO+lIRpzOlEcYCR6wexlmL9mZNEFJoROMMRyCxRilNAiDKOq0X5j1HXY2ky9KjtLxo2b8yXFRBcYKK4W8aHU94NkNek81jdi9lC+L6jUrZ25d7xylqnEbd4TgrBphiowdh5B3RY+j6ePthLWWhHopRcVDgq91yYacelCCIfSDaGBya9iXPZSwoKtPin4n5PdSGmSOA3fIkvV5JjLS1QbuY5tzhXKuH9MdAqrvutF1bprs/GDf0IY8DO7DDoo8rXjhhgGtCdg1i5UitZIAM3hn/Vk5F//5eJ
        JOOS8312@EB-OR6120158"
    sudo:
    - ALL=(ALL) NOPASSWD:ALL
...

# Deprecations: -------------
# D1: An alias for ``grub_dpkg`` Deprecated in version 22.2. Use ``grub_dpkg`` instead.
# D2: Default: ``true`` Deprecated in version 22.3. Use ``lock_passwd`` instead.
# D3:  Deprecated in version 18.3. Use ``ssh_authorized_keys`` instead.

  Valid schema user-data

2. network-config at /var/lib/cloud/instances/cb88d1af-f827-4381-bc89-4fad787bcaaf/network-config.json:
  Valid schema network-config
blackboxsw commented 2 months ago

I agree that the output needs help to make it more useful in the default case.

This command currently is a wall of text which makes it easy to lose the specific warnings/errors nested inline amongst other messaging.

I expect that we may need to craft any of these numbered items into separate issues as we start working them to make then achievable. Items 1 and 2 represent some a need for some fairly significant changes to structured formatting.

  1. Deprecating --annotate and making it default behavior: agreed but let's also first think through a machine-readable --format=json|yaml option here to point folks like https://github.com/canonical/subiquity/blob/main/subiquity/cloudinit.py#L89 at a less fragile/tightly coupled dependency on these output messages a. I think we want the following content represented in a --format=yaml output:

    • type: (user-data, vendor-data, meta-data)
    • source_path: '/some/path/to/source/data/file.yaml
    • errors: [...]
    • deprecated: [...] b. Each error or deprecated item could contain the following keys:
      • key_path: None or the known path into the cloud-config dictionary for the offending key: e.g. users.1.lock-passwd in your example above
      • line: None or the line number within the source_path file where the schema error/warning occurs
      • column: None or the column number within the source_path file where the schema error/warning occurs
      • message: The schema error/warning message

  2. I think we can remove the file path provided we include it in somehow in a --format=(yaml|json) machine-readable output as it does provide triage with an understanding of which instance-id the config file source_path was sourced from, which may be stale/incorrect because an instance-id can change over time in /var/log/cloud-init.log and it can be helpful to know where that config is sourced from in case of instance-id updates.

  3. and 4.: Removing double-ticks surrounding key-name in CLI messaging and potentially supporting color mode.: Sure, though that's probably something we'd like to separate into a different issue as 1 and 2 feel closely related/inter-dependent features on functional content delivery. The coloration and final string formatting feels a bit more contained to a shallower style/usability/design component that probably involves less of a spec/discussion and more of a design review