Quiet flag for uninterrupted single cluster run of quickstart script

[ehearneRedHat]

What:

KUADRANT_QUIET, DNS_PROVIDER and MULTICLUSTER flag detection for running quickstart script uninterrupted has been added.

How:

It checks for whether KUADRANT_QUIET exists and if so, will run an uninterrupted run of quickstart in single cluster and no DNS provider mode.

If DNS_PROVIDER is set alongside KUADRANT_QUIET, then it will run the quickstart script in single cluster mode, with dns provider.

If MULTICLUSTER is set alongside KUADRANT_QUIET or DNS_PROVIDER, or both, it will run in multicluster mode with or without dns provider .

Why:

Developers or System Administrators may find this useful so they do not need to use prompts to spin up Kuadrant quickly, such as for testing purposes in CI/CD pipelines.

Verify:

Run:

• KUADRANT_QUIET=<whatever> ./hack/quickstart-setup.sh
• KUADRANT_QUIET=<whatever> DNS_PROVIDER=<whatever> ./hack/quickstart-setup.sh
• KUADRANT_QUIET=<whatever> DNS_PROVIDER=<whatever> MULTICLUSTER=<whatever> ./hack/quickstart-setup.sh
• KUADRANT_QUIET=<whatever> MULTICLUSTER=<whatever> ./hack/quickstart-setup.sh

Of course, other combinations exist for those env vars. The idea is that all these combinations run successfully and without error to make the script more automated.

[ehearneRedHat]

#743

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 82.95%. Comparing base (ece13e8) to head (a550805).
Report is 135 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #744      +/-   ##
==========================================
+ Coverage   80.20%   82.95%   +2.74%     
==========================================
  Files          64       77      +13     
  Lines        4492     5978    +1486     
==========================================
+ Hits         3603     4959    +1356     
- Misses        600      673      +73     
- Partials      289      346      +57     

Flag	Coverage Δ
bare-k8s-integration	4.55% <ø> (?)
controllers-integration	72.84% <ø> (?)
gatewayapi-integration	11.12% <ø> (?)
integration	?
istio-integration	56.62% <ø> (?)
unit	33.23% <ø> (+3.20%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components	Coverage Δ
api/v1beta1 (u)	71.42% <ø> (ø)
api/v1beta2 (u)	92.17% <93.93%> (+0.75%)	⬆️
pkg/common (u)	88.13% <ø> (-0.70%)	⬇️
pkg/istio (u)	73.88% <ø> (-0.03%)	⬇️
pkg/log (u)	94.73% <ø> (ø)
pkg/reconcilers (u)	∅ <ø> (∅)
pkg/rlptools (u)	83.33% <ø> (+3.87%)	⬆️
controllers (i)	82.10% <85.81%> (+5.29%)	⬆️

see 43 files with indirect coverage changes

[david-martin]

I like the idea of the QUIET flag.
I wonder though if we can make it less restrictive on what it does when it's in quiet mode?

What about env vars for each of the prompts? For example, if DNS_PROVIDER is set, skip the prompt for that.

[R-Lawton]

Having a quiet flag raised the question in my head is the quickstart to noisy to begin with and should we remove some extra prompts or combine prompts ? For example remove the are you ready prompt and combine the 2 DNS provider ones together so by inputting a provider that's a yes and have no also as a option @david-martin Thoughts ?

[david-martin]

For example remove the are you ready prompt and combine the 2 DNS provider ones together so by inputting a provider that's a yes and have no also as a option

sounds reasonable to make it easier for users and to simplify the script.

[Boomatang]

I have thoughts on this and in general I think this is solving the symptom of the problem but not the problem.

Firstly I think the flag name of QUIET is the wrong name. This seems to be more to do with running in single cluster mode than being quiet. Secondly I generally really dislike using environment variables with generic names in tooling that is expected to run on someone else's machine. Who is to say they do not have a QUIET variable set for some different process. They might not know why the quickstart guide is not working as expected, if they are wanting to set a multi cluster configuration.

Finally, if we look at the problem, you are trying to configure the script to only run the single cluster setup in a hands free mode (I am guessing to make your dev process easier). What stops a user wanting to run the multi cluster configuration in a hands free mode, maybe be in a pipeline for example? What I think the correct approach here should be to allow the user pass in and / or be auto read from the directory the script is ran from, a configuration file. This configuration file should hold the answer to any input the script expects and the script asks any question that the user forgot to set configuration parameter for. If the values are there, log that the value comes from the configuration file.

The use of a configuration file in this manner means that very hand holding questions can be asked to new user the first time they use the script. While at the same time allowing someone that runs the script regularly to have multiply configurations saved that they can run in a hands free experience.

[ehearneRedHat]

@david-martin

What about env vars for each of the prompts? For example, if DNS_PROVIDER is set, skip the prompt for that.

Sounds good to me. It would allow for better flexibility .

[ehearneRedHat]

@R-Lawton

remove the are you ready prompt and combine the 2 DNS provider ones together so by inputting a provider that's a yes and have no also as a option

Makes sense to me. :)

[ehearneRedHat]

@Boomatang

Firstly I think the flag name of QUIET is the wrong name. This seems to be more to do with running in single cluster mode than being quiet.

This is true - I guess the name of the variable would have to be more specific if we were going with the original case. However, I believe that by adding further variables suggested by @david-martin, that this would allow the purpose of the QUIET variable to be more understandable.

Secondly I generally really dislike using environment variables with generic names in tooling that is expected to run on someone else's machine. Who is to say they do not have a QUIET variable set for some different process. They might not know why the quickstart guide is not working as expected, if they are wanting to set a multi cluster configuration.

I can see where you're coming from. If the QUIET variable name was set to something a little less generic, such as KUADRANT_QUIET , the chances of that variable being present on their machine would be much lower, and we can generally assume that users will need to set that flag in order to enter quiet mode.

Finally, if we look at the problem, you are trying to configure the script to only run the single cluster setup in a hands free mode (I am guessing to make your dev process easier). What stops a user wanting to run the multi cluster configuration in a hands free mode, maybe be in a pipeline for example? What I think the correct approach here should be to allow the user pass in and / or be auto read from the directory the script is ran from, a configuration file. This configuration file should hold the answer to any input the script expects and the script asks any question that the user forgot to set configuration parameter for. If the values are there, log that the value comes from the configuration file.

The use of a configuration file in this manner means that very hand holding questions can be asked to new user the first time they use the script. While at the same time allowing someone that runs the script regularly to have multiply configurations saved that they can run in a hands free experience.

I can see the benefits of using such an approach - it would mitigate the factor of someone already having that environment variable set on their machine as it would be reading these configurations from a file. On the other hand, it would make the quickstart script less convenient to use as a new file with configurations would need to be created. As such, I don't think it is something we should implement into a quickstart script, something that allows a user to get set up quite quickly.

However, I believe a compromise here would be to add the additional flags mentioned by @david-martin while changing the QUIET flag to KUADRANT_QUIET which would mitigate the potential problems suggested by yourself.

What are your thoughts ?

[Boomatang]

There is one detail that maybe I was clear about in the first comment. The configuration file would only be used if it is present. So you don't make the script less convenient by needing to generate the configuration file. It should work with out it.

Also remember that a lot of our guides expect the user to create files, like echo "some CR contains" | kubectl apply -f -, so doing some thing like that for the quick start might be acceptable. That is just a thought and I am not suggesting that is what we do. I am not a fan of that pattern.

But it does raise a valid question for this PR. Where is this new flag documented. How is a new user meant to know what it does? If that involves reading and understand all the documentation then why not write it to build up a configuration file during that process? The same would go for DNS_PROVIDER environment variable that @david-martin is suggesting. That would need to be documented to say what it does, but also the inverse is true. What happens when the environment variable is not set would also need to be documented.

One more thing that should be in the back of you mind when doing this is what happens to the cardinality caused by the introduction of the external configurations. I say external configurations as this affects both environment variable and configuration files. Take this example if QUIET=true and DNS_PROVIDER=true, does that mean the script should run in quiet mode but also ask the questions about what settings are need for the DNS provider?

[ehearneRedHat]

There is one detail that maybe I was clear about in the first comment. The configuration file would only be used if it is present. So you don't make the script less convenient by needing to generate the configuration file. It should work with out it.

This makes much more sense to me, thanks for clarifying that! I can now see the benefit of using such a configuration file, and will look to implement.

Also remember that a lot of our guides expect the user to create files, like echo "some CR contains" | kubectl apply -f -, so doing some thing like that for the quick start might be acceptable. That is just a thought and I am not suggesting that is what we do. I am not a fan of that pattern.

Thanks for pointing that out - I am now a fan of the idea with the idea that it is to complement, not to replace the quiet flag idea.

But it does raise a valid question for this PR. Where is this new flag documented. How is a new user meant to know what it does? If that involves reading and understand all the documentation then why not write it to build up a configuration file during that process? The same would go for DNS_PROVIDER environment variable that @david-martin is suggesting. That would need to be documented to say what it does, but also the inverse is true. What happens when the environment variable is not set would also need to be documented.

You are right, I will write up a ticket for documenting this new flag. I believe if we re-factor how we think about documenting, where this is an additional feature, and we can leave what we already have there in place, while adding this new section that outlines how to use these new environment variables or configuration, that it would require less work while still providing the user with the context.

Thinking about it, maybe it could also be another ticket for implementing the configuration too. That is to say, it could be something worked on after this PR, or simultaneously. I'll go ahead and write that one up, too.

One more thing that should be in the back of you mind when doing this is what happens to the cardinality caused by the introduction of the external configurations. I say external configurations as this affects both environment variable and configuration files. Take this example if QUIET=true and DNS_PROVIDER=true, does that mean the script should run in quiet mode but also ask the questions about what settings are need for the DNS provider?

Thanks for mentioning this! My understanding would be that when DNS_PROVIDER and KUADRANT_QUIET is set, that it would look for the related env variables associated with that provider, and if not set, the script would exit with a message to let the user know that those variables also need to be set so that the script can run correctly, therefore still complying with the script being ran without intervention.

To summarise, I will continue on the path with creating the environment variables needed as mentioned above, while creating tickets for documenting the process and creating a configuration file to complement the quickstart .

How does that sound ?

[Boomatang]

Yes for the most of it. I disagree with the use of environment variable in general. While yes a configuration file would complement the environment variable, I still see the problem of using environment variables. If it was feature flags, ya, they go hand in hand with configuration files. When I say feature flags an example call would look like quickstart-setup.sh --quiet=true.

The other piece of information I will share, more for context then any else. Environment variables do not act the same every where. I use Fish for my shell. There is 5 main levels of environment variables. The first being local which is only accessible from within the fish function that defines it. The there is shell level access which allows access within that session shell only. So you can do echo $HELLO_WORLD. Then there is exported mode which allows scripts to access the environment variable in the current shell session. The there is universal mode which allow any shell session to access the variable. Finally there is the universal exported mode, which allows any script on in any shell session to access the environment variable.

I should say, the plan you outlined in your summary sound good under the circumstances.

[ehearneRedHat]

Yes for the most of it. I disagree with the use of environment variable in general. While yes a configuration file would complement the environment variable, I still see the problem of using environment variables. If it was feature flags, ya, they go hand in hand with configuration files. When I say feature flags an example call would look like quickstart-setup.sh --quiet=true.

I can see the benefit of using feature flags, but wouldn't it nearly be over engineering the quickstart script if we were to start using them alongside env vars ? It could become confusing, and to mitigate, would mean re-designing the script entirely. Since we are already using env vars, I think it would be best to continue using them for now .

The other piece of information I will share, more for context then any else. Environment variables do not act the same every where. I use Fish for my shell. There is 5 main levels of environment variables. The first being local which is only accessible from within the fish function that defines it. The there is shell level access which allows access within that session shell only. So you can do echo $HELLO_WORLD. Then there is exported mode which allows scripts to access the environment variable in the current shell session. The there is universal mode which allow any shell session to access the variable. Finally there is the universal exported mode, which allows any script on in any shell session to access the environment variable.

I believe we defined exactly which shell to use at the top of the script, so it should use bash by default when running . I can see where it can present issue if using different shells, but since we have already defined the shell, I would expect the behaviour of running the script to be bash .

I should say, the plan you outlined in your summary sound good under the circumstances.

Thanks for letting me know! I will continue with that plan for now. :)

[ehearneRedHat]

Kuadrant/docs.kuadrant.io#129

[ehearneRedHat]

PR closed as new issue replaces it.