Eksctl: `eksctl gitops apply` CLI flag validations

[martina-if]

Today the value of --output-path is not checked before starting to run the command's actions so if it points to an existing folder that already contains something it will fail too late.

eksctl should validate values for all flags as early as possible.

Example:

$ EKSCTL_EXPERIMENTAL=true eksctl gitops apply --cluster martina-test-3 --region eu-north-1 [email protected]:martina-if/flux-test-3 --output-path=/tmp/gitops-repos2/  --quickstart-profile app-dev --git-email [email protected]
[ℹ]  Generating public key infrastructure for the Helm Operator and Tiller
[ℹ]    this may take up to a minute, please be patient
[!]  Public key infrastructure files were written into directory "/tmp/eksctl-helm-pki113134613"
[!]  please move the files into a safe place or delete them
[ℹ]  Generating manifests
.
.
.
[!]  Helm Operator is not ready yet (Get http://127.0.0.1:42359/healthz: EOF), retrying ...
[ℹ]  Helm Operator started successfully
[ℹ]  see https://docs.fluxcd.io/projects/helm-operator for details on how to use the Helm Operator
[ℹ]  Waiting for Flux to start
[ℹ]  Flux started successfully
[ℹ]  see https://docs.fluxcd.io/projects/flux for details on how to use Flux
[ℹ]  Committing and pushing manifests to [email protected]:martina-if/flux-test-3
[master 23ddde8] Add Initial Flux configuration
 2 files changed, 39 insertions(+), 38 deletions(-)
 rewrite flux/tiller-ca-cert-configmap.yaml (86%)
Counting objects: 5, done.
Delta compression using up to 4 threads.
Compressing objects: 100% (5/5), done.
Writing objects: 100% (5/5), 1.83 KiB | 1.83 MiB/s, done.
Total 5 (delta 3), reused 0 (delta 0)
remote: Resolving deltas: 100% (3/3), completed with 3 local objects.
To github.com:martina-if/flux-test-3
   31443a8..23ddde8  master -> master
[ℹ]  Flux will only operate properly once it has write-access to the Git repository
fatal: destination path '/tmp/gitops-repos2/flux-test-3' already exists and is not an empty directory.

[marccarre]

In addition to the above, it may be only me, but I _feel_ like the UX related to that flag isn't very "smooth". Or at the very least, it isn't very clear to me how to use this flag and what it relates to. More specifically:

1. Given the user provides the URL of a configuration repository via --git-url, chances that their already have a local clone are high.
   
   
   
   1. How should eksctl deal with this scenario?
   
   
   2. What if users point to it? (It currently fails, should we make eksctl smarter about it? e.g.: check if non-empty directory and if git remote is identical to --git-url and if so, re-use that local clone?)
   
   
   3. What if users run eksctl gitops apply from their local clone's directory?
   
   
   4. What if they don't and now have a second clone?
   
   
2. What is the intent behind cloning the repository under . as an "output" for the end-user?
   
   
   
   1. Should we output this at all? (I personally find it confusing 😅 )
   
   
   2. Should we use a temporary directory instead if/when eksctl needs to work with the repository?
   
   
   3. Should we improve the documentation of the flag?
      
      --output-path string Path to directory where the GitOps repo will be cloned (default "./")
   
   

[martina-if]

Very good points. Perhaps it makes more sense to always use a temporary directory (like we do for cloning the quick start repo), and if the flag is specified then it's because the user wants to reuse a local clone?

[marccarre]

That would make a lot more sense to me. 🙂
But before changing anything, do we know the original intent, do we know why we went for this? Was there a specific reason? Was it something "product" specifically wanted? And if so, why?

[martina-if]

The original user story description didn't have it. I think that flag was something I took from the generate profile command because I thought it would be good to have a folder for the user to see what was done. And also because I needed a path in common for both parts of the command.

@gemagomez I guess this is not urgent but it would be good to do something to improve UX and avoid having the command failing in the middle of the operation just because the flag is wrong (even though it's optional).

[marccarre]

@kalbir would you have the answers to these questions (https://github.com/weaveworks/eksctl/issues/1274#issuecomment-528290291) by any chance?

[kalbir]

Is what is going on here that we create a local clone of the users git repo as part of gitops apply?

If that is the scenario then I think what @martina-if says above is right. This wasn't particularly intended as part of the apply functionality, it was something we thought of as part of generate profile that got carried across. We thought that the user would need to generate some files locally before they pushed them to their git repo and so we gave them the option on where to put those files.

So I think 2.i is the key question here. I agree that users don't need us to give them a local clone of their git repo, we should just create a temporary one if we need to and clean up aftwards. I _think_ that makes all of the other questions moot (at least I hope it does!)