Feature suggestions for offline mode (kn service --target)

[rhuss]

Feature request

Some minor tuning for the kn service create --target offline mode:

• The success message should mention which file has been created. Until now it looks like the usual kn service create message without any context that it has operated in offline mode
• Empty sections should be filtered out in the file created. This might be a bit tricky, but I think it should be possible as a post-processing step.
• I got some feedback on the --target naming. Not everyone liked it and has a different association with it. Maybe we should revisit it. Let's discuss again on this ticket whether we want to change and maybe find a better alternative.

[rhuss]

For reference: Here's the previous discussion that resulted in the --target name: https://github.com/knative/client/pull/1122#pullrequestreview-547101347

[dsimansk]

In the PR discussion I stated the following list 1. offline | 2. target | 3. local. I can easily agree that offline sounds too negative. Then might be the --local flag a better bet?

[github-actions[bot]]

This issue is stale because it has been open for 90 days with no activity. It will automatically close after 30 more days of inactivity. Reopen the issue with /reopen. Mark the issue as fresh by adding the comment /remove-lifecycle stale.

[rhuss]

/remove-lifecycle stale

[rhuss]

I think we should use this issue as well for nailing down the final option name for the offline mode. --target seems to be contentious, so let's open the bike shedding contest again.

[salaboy]

@rhuss I've been looking at this --target option through the lens of func as we need a --dry-run (https://github.com/knative-sandbox/kn-plugin-func/issues/1060) option which pretty much needs something like what target is doing, but instead of writing the yaml files into separate files inside a directory it should just print them out to the standard output. Our main use case is not being offline but more using a GitOps approach where the output of the --dry-run command is directly picked up by another command like kubectl apply -f.

I was wondering if we can improve the target mechanism to just print all the content of all the generated files using a separator (---) which users then can redirect to a file if they want to.

What do you think?

[dsimansk]

I was wondering if we can improve the target mechanism to just print all the content of all the generated files using a separator (---) which users then can redirect to a file if they want to.

Per today' WG call with @lance and @maximilien we agreed that should be feasible solution to redirect the output. Add a special case e.g. --target - to output to stdout directly.

Update: thinking twice something like nicer constructor for NewKnServingGitOpsClient would be probably even better and sufficient. :)

[salaboy]

@dsimansk yes.. 100% for a nicer constructor for STDOUT.

I don't want to open an old discussion, but maybe we can have an extra flag --dry-run that overrides --target with stdout to make sure we follow very well know conventions in CLIs. For example helm: https://helm.sh/docs/chart_template_guide/debugging/

[rhuss]

@salaboy, I'm not convinced about --dry-run as the proper option name for creating a YAML file and printing it offline. --dry-run actually means (at least how to interpret it): Do something as you would do without the option given, but without side effects. Like make --dry-run to find out which targets are triggered. So a 'dry run' is a test run to determine what would happen. That makes sense for multi-step actions, not for atomic actions like kn service create, which only has one interaction with the API server. I could see the value of a --dry-run to send over the service to the API server and tell him only to run the validation and return an error if it is invalid. For functions, I would expect an --dry-run to perform all validation to ensure for build and deploy to ensure that the multi-step function process will likely succeed and not stop in between because of an error that could have been detected before.

For the helm case that you mention, I consider this a bit of a misusage of --dry-run: Yes, it will cause the server side to render something but don't apply it (the knative equivalent would be to send a service definition to the K8s API server but instruct it not to apply but just check if it could be applied). Btw, I thought Tiller was already gone, so you don't use --dry-run for rendering a template on the client side? You would use a dedicate command for that.

For the use case, we want to support here, I think --output or --target are better suited as this specifies what should happen with the specified service: Send it to K8s, store it in a file, or print it out to standard out.

[salaboy]

@rhuss yes I tend to agree, and using helm as an example was pretty bad.. I will be happy with --output I find --target really confusing. As soon as we can get the YAML out without applying it to the cluster I am fine with it.

The primary use case that I had in mind is to be able to do something like: func deploy --output stdout and then do any patching with kustomize or any other tools that can process that YAML output.

How can we make this happen? Is there someone working on this already?

[rhuss]

I'm open for renaming it to --output (with the usual deprecation cycle) (maybe with possible values yaml & json and instead of a filename, print it always to standard out. That would be conformance with how kubectl get does it.

Nobody is currently working on this, any help is appreciated.