helm.operator-sdk/upgrade-force annotation value is not actually parsed

[anfanycw]

Bug Report

What did you do?

1. Created an helm-based operator for locust with operator-sdk new ...
2. Created operator and generated CRD yamls in minikube
3. Applied generated custom resource
4. Updated a value in the custom resource so a configuration change is recognized and re-applied the custom resource yaml.

What did you expect to see?
Expected to see all existing helm chart resources to be replaced. In this case, a new set of master and worker pods for locust.

What did you see instead? Under which circumstances?
Only the resource applicable to the updated config value was changed. In this case I decremented the worker replica count which only replaced the master pod and destroyed one worker pod. This debug statement showed up in the operator logs:

{"level":"info","ts":1587606295.6086833,"logger":"helm.controller","msg":"Could not parse annotation as a boolean","annotation":"helm.operator-sdk/upgrade-force","value informed":"T"}

This is when I noticed this: https://github.com/operator-framework/operator-sdk/pull/2773/files#diff-66372f3cbaf2399b5e8895c882e4eadfR336. force is not being parsed.

Environment

• operator-sdk version:

"v0.17.0", commit: "2fd7019f856cdb6f6618e2c3c80d15c3c79d1b6c", kubernetes version: "v1.17.2", go version: "go1.14.2 darwin/amd64"

• go version:

go version go1.14.2 darwin/amd64

• Kubernetes version information:

Client Version: version.Info{Major:"1", Minor:"18", GitVersion:"v1.18.2", GitCommit:"52c56ce7a8272c798dbc29846288d7cd9fbae032", GitTreeState:"clean", BuildDate:"2020-04-16T23:34:25Z", GoVersion:"go1.14.2", Compiler:"gc", Platform:"darwin/amd64"}
Server Version: version.Info{Major:"1", Minor:"18", GitVersion:"v1.18.0", GitCommit:"9e991415386e4cf155a24b1da15becaa390438d8", GitTreeState:"clean", BuildDate:"2020-03-25T14:50:46Z", GoVersion:"go1.13.8", Compiler:"gc", Platform:"linux/amd64"}

• Kubernetes cluster kind:

Helm

Possible Solution

On line: https://github.com/operator-framework/operator-sdk/pull/2773/files#diff-66372f3cbaf2399b5e8895c882e4eadfR336. force is not being parsed.

...
if i, err := strconv.ParseBool(force)
...

Additional context
Add any other context about the problem here.

[camilamacedo86]

Hi @anfanycw,

According to the error see "value informed":"T" you informed "T" instead of "True". Am I right?
Could you please check it with the value as True?

[anfanycw]

@camilamacedo86 ParseBool accepts 1, t, T, TRUE, true, True, 0, f, F, FALSE, false, False. “T” was just one of the cases I tried besides the original from the README (True) before looking at the code.

[camilamacedo86]

I will do a test to check it as soon as possible. A possible way to verify that and/or may its fix is by creating a unit test in the to cover this code and ensure that it will work. I do not think that it has been covered by tests.

However, your collaboration would be very welcome as well. Please, if you wish and have time feel free to come up with the fix and tests in a PR.

[joelanford]

The problem is that we need to pass force to strconv.ParseBool(), not helmUpgradeForceAnnotation

https://github.com/operator-framework/operator-sdk/blob/master/pkg/helm/controller/reconcile.go#L336

[dian-xh]

greetings,
tried with the new 0.17.1 image, and still seeing this issue when parsing the annotation boolean:

cr managed by helm operator:

    "metadata": {
      "name": "test",
      "annotations": {
        "helm.operator-sdk/upgrade-force": "True"
      }

log:

{"level":"info","ts":1590607171.116301,"logger":"helm.controller","msg":"Could not parse annotation as a boolean","annotation":"helm.operator-sdk/upgrade-force","value informed":"True"}
{"level":"info","ts":1590607171.9757097,"logger":"helm.controller","msg":"Updated release","namespace":"test","name":"logging","apiVersion":"myapi","kind":"MyType","release":"test","force":false}

I don't have permission to reopen this ticket. let me know if I should create a new one. thanks!

[camilamacedo86]

Hi @dian-xh,

I will be working on in this fix, thank you for let us know.
/assign @camilamacedo86

[estroz]

@dian-xh I am not able to reproduce this issue in v0.17.1. The CR being reconciled is being created from your CSV's alm-examples right?

[dian-xh]

Yes

[camilamacedo86]

HI @dian-xh,

I am trying to reproduce the issue faced by you and I also could not face the error. So, lets keeping this close as history and track your scenario in the new issue because it shows no be tthe same case.

I also try to check your project and I will add all info in this new issue. Could you please follow up there? See; #3144

[dian-xh]

@camilamacedo86 sounds good. thanks!