eksctl-io / eksctl

The official CLI for Amazon EKS
https://eksctl.io
Other
4.93k stars 1.41k forks source link

Improve visibility around which config file fields are supported by each command #5386

Open nikimanoledaki opened 2 years ago

nikimanoledaki commented 2 years ago

This issues raises a UX question around how to improve the visibility of the config file fields supported by each command.

As mentioned in this comment:

3. Unclear which fields are supported in --config-file

Another important issue since we are dropping flag support and supporting new features through the use of the config file.

You can find out the supported flags by using --help.

How can a user find out which config file fields can be used with a specific command in a familiar, seamless CLI experience?

Currently, the list of supported config file fields has to be added manually to the docs. However, we don't have a schema or a consistent, automated way of doing and maintaining this. We also have the examples. I wonder if there's a better way of doing this, preferably through the CLI for better UX (like with --help). :)

In the CLI, we could improve the validation of unsupported fields by logging any fields that are unsupported and remain unchanged.

In the docs, it would be nice to auto-generate the config file schema that is supported per command.

github-actions[bot] commented 2 years ago

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days.

TiberiuGC commented 1 year ago

Adding a proposal below to improve visibility with a CLI option

UX Details

For each eksctl command that supports using a cluster config file, add a new flag --check-config-file (better naming suggestions are welcome) that will only be available to use when a config file is provided, e.g.

eksctl create cluster --config-file config.yaml --check-config-file



This shall give the user some sort of insights on the config file they have provided, as in which fields are part of which of the following categories: supported and provided, supported but not provided, ignored etc. We can opt to display an amended config file, where we add default values for missing (optional) fields, and color everything accordingly to the category they fall into. Fields that are not supported, or required but missing shall always result in throwing a validation error.

Say the user is using the following config file for creating fargate profiles

...

metadata:
  name: my-cluster
  region: us-west-2
  version: "1.27"

nodeGroups:
  - name: my-nodegroup
    amiFamily: "amazonlinux2"
    desiredCapacity: 2

fargateProfiles:
  - name: fp-default
    selectors:
      # All workloads in the "default" Kubernetes namespace will be
      # scheduled onto Fargate:
      - namespace: default
      # All workloads in the "kube-system" Kubernetes namespace will be
      # scheduled onto Fargate:
      - namespace: kube-system

Thus, running eksctl create fargateprofile --config-file config.yalm --check-config-file shall return something like

image

Where:

Implementation details

Each command that supports using a config file usually has its own dedicated cluster config loader func (e.g. CreateNodegroupLoader). Apart from unmarshaling the config file into cluster config struct, this is usually responsible for some additional validations. We can extend the loader’s functionality to sort the provided fields into their respective categories. To achieve this we need to know in advance which command uses which fields, thus, I suggest creating for each command a new struct type that is a subset of the cluster config, only containing the relevant fields for that command. With the above example in mind, for create fargateprofile command we shall have:

type CreateFargateProfileClusterConfig struct {
     Metadata ClusterMeta
     FargateProfiles []*FargateProfile
}

Other considerations

Below is a starting point for creating a custom yaml printer that supports colouring the output https://gist.github.com/TiberiuGC/38053ab5d8bbc252ce139202f02b4e0a