✨ Enable verbosity control from chart values

[francostellari]

Summary

Enable verbosity control from chart values

Related issue(s)

Fixes #

[pdettori]

/lgtm

[pdettori]

@francostellari could you please check if the e2e test works in your dev env ? It is failing in the CI for this PR.

[MikeSpreitzer]

Debugging this would have been easier with e81fb04 (from #60) --- it would show that the OCM Status Add-On controller is not running.

Further, a listing of Pod objects in the hub would show a line like the following.

open-cluster-management       addon-status-controller-7496df988b-47ggd                    0/1     CrashLoopBackOff   10 (4m17s ago)   30m

Getting the log of that would point to the problem:

mspreitz@mjs13 ocm-status-addon % kubectl --context kind-hub logs -n open-cluster-management deployment/addon-status-controller   
Defaulted container "status-controller" out of: status-controller, create-crd (init)
Error: invalid argument "{{.Values.verbosity}}" for "-v, --v" flag: strconv.ParseUint: parsing "{{.Values.verbosity}}": invalid syntax
Usage:
  addon controller [flags]

Flags:
      --component-namespace string   Namespace of the component.
      --enable-leader-election       Enables the leader election for the controller
  -h, --help                         help for controller
      --kubeconfig string            Location of the master configuration file to run from.

Global Flags:
      --log-flush-frequency duration   Maximum number of seconds between log flushes (default 5s)
      --logging-format string          Sets the log format. Permitted formats: "text". (default "text")
  -v, --v Level                        number for the log level verbosity
      --vmodule pattern=N,...          comma-separated list of pattern=N settings for file-filtered logging (only works for text log format)

invalid argument "{{.Values.verbosity}}" for "-v, --v" flag: strconv.ParseUint: parsing "{{.Values.verbosity}}": invalid syntax

Dumping the YAML of the Deployment would show the problem even more directly:

mspreitz@mjs13 ocm-status-addon % kubectl --context kind-hub get deploy -n open-cluster-management addon-status-controller -o yaml
apiVersion: apps/v1
kind: Deployment
...
    spec:
      containers:
      - args:
        - controller
        - -v={{.Values.verbosity}}

@pdettori , @francostellari: the problem here is that this PR changes the source YAML for the controller to ONLY make sense as input to make chart, but there are other make targets that use the same source directly (without building and using the chart). make deploy, called from test/e2e/setup-ocm-and-addon.sh, is one example.

[francostellari]

@MikeSpreitzer I guess we should change the make deploy to be a make chart + helm install --context...?
That would also have the additional benefit of doing a minimal testing of the chart.
Applying the YAML of the config via kubectl is not a good idea since one expects it to be designed for the helm chart not for kubectl.
Do you agree?

[MikeSpreitzer]

@francostellari : I tend to agree with always using the chart. Note that there is already a make target named install-local-chart.

[francostellari]

@MikeSpreitzer I fixed make deploy, the test should run ok now

[MikeSpreitzer]

This should probably be transposed; at least the agent will have additional flags that chart users should be able to control (#60 exposes several).

[MikeSpreitzer]

Yes, make deploy now goes through the chart. However, there remain three other targets that use the source without going through the chart: install, uninstall, and undeploy.

[MikeSpreitzer]

Also, I see the following in a make recipe but I do not see anything that undoes the local file mod.

	cd config/manager && $(KUSTOMIZE) edit set image controller=$(shell echo ${IMG} | sed 's/\(:.*\)v/\1/')

[francostellari]

Yes, make deploy now goes through the chart. However, there remain three other targets that use the source without going through the chart: install, uninstall, and undeploy.

I feel that this is behind the scope of this PR

[francostellari]

Also, I see the following in a make recipe but I do not see anything that undoes the local file mod.

	cd config/manager && $(KUSTOMIZE) edit set image controller=$(shell echo ${IMG} | sed 's/\(:.*\)v/\1/')

I feel that this is behind the scope of this PR

[MikeSpreitzer]

Yes, this is good enough to merge. Open issues can be addressed in later PRs.