Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Removal of field in update is not applied #1286

Closed
porridge opened this issue Jan 17, 2020 · 1 comment · Fixed by #1332
Closed

Removal of field in update is not applied #1286

porridge opened this issue Jan 17, 2020 · 1 comment · Fixed by #1332

Comments

@porridge
Copy link
Member

What happened:

I ran an update on a cassandra instance to apply PROMETHEUS_EXPORTER_ENABLED=false to a default installation.
It did not remove the prometheus-exporter from the statefulset.

$ kubectl kudo install --instance cassandra-off cassandra
$ kubectl kudo update -p PROMETHEUS_EXPORTER_ENABLED=false --instance cassandra-off
Instance cassandra-off was updated.
$ kubectl describe statefulset cassandra-off-node
[...]
Pod Template:
[...]
  Containers:
   cassandra:
    Image:       mesosphere/cassandra:3.11.5-0.1.2-SNAPSHOT
[...]
    Environment:
[...]
      PROMETHEUS_EXPORTER_ENABLED:  false
[...]
   prometheus-exporter:
    Image:      mesosphere/cassandra-prometheus-exporter:2.2.1-0.1.2-SNAPSHOT
[...]
Events:
  Type    Reason            Age                  From                    Message
  ----    ------            ----                 ----                    -------
  Normal  SuccessfulDelete  5m56s (x2 over 18h)  statefulset-controller  delete Pod cassandra-off-node-2 in StatefulSet cassandra-off-node successful
  Normal  SuccessfulCreate  5m39s (x3 over 18h)  statefulset-controller  create Pod cassandra-off-node-2 in StatefulSet cassandra-off-node successful
  Normal  SuccessfulDelete  4m9s (x2 over 18h)   statefulset-controller  delete Pod cassandra-off-node-1 in StatefulSet cassandra-off-node successful
  Normal  SuccessfulCreate  3m59s (x3 over 18h)  statefulset-controller  create Pod cassandra-off-node-1 in StatefulSet cassandra-off-node successful
  Normal  SuccessfulDelete  3m33s (x2 over 18h)  statefulset-controller  delete Pod cassandra-off-node-0 in StatefulSet cassandra-off-node successful
  Normal  SuccessfulCreate  3m19s (x3 over 18h)  statefulset-controller  create Pod cassandra-off-node-0 in StatefulSet cassandra-off-node successful

You can see that the value for the parameter was propagated correctly, and the statefulset was updated.

What you expected to happen:

It should have removed a container from the stateful set.

How to reproduce it (as minimally and precisely as possible):

See the commands in first section.

Anything else we need to know?:

The prometheus-exporter container definition is conditional on PROMETHEUS_EXPORTER_ENABLED being true, but it is a part of a largr resource (statefulset) which is being applied unconditionally. This makes this issue different from #1147.

I also confirmed that leaving out the prometheus-exporter container works correctly when PROMETHEUS_EXPORTER_ENABLED=false at the moment of initial installation. Therefore I'm reasonably confident that this must be the patch implementation which causes removal of fields to be ignored.

cc @mpereira

Environment:

  • Kubernetes version (use kubectl version):
Client Version: version.Info{Major:"1", Minor:"16", GitVersion:"v1.16.3", GitCommit:"b3cbbae08ec52a7fc73d334838e18d17e8512749", GitTreeState:"clean", BuildDate:"2019-11-13T11:23:11Z", GoVersion:"go1.12.12", Compiler:"gc", Platform:"linux/amd64"}
Server Version: version.Info{Major:"1", Minor:"15", GitVersion:"v1.15.1", GitCommit:"4485c6f18cee9a5d3c3b4e523bd27972b1b53892", GitTreeState:"clean", BuildDate:"2019-07-18T09:09:21Z", GoVersion:"go1.12.5", Compiler:"gc", Platform:"linux/amd64"}
  • Kudo version (use kubectl kudo version):
KUDO Version: version.Info{GitVersion:"0.10.0", GitCommit:"d2310b12", BuildDate:"2020-01-10T18:54:59Z", GoVersion:"go1.13.6", Compiler:"gc", Platform:"linux/amd64"}
  • Operator: cassandra
  • operatorVersion: master (after 0.1.2)
  • OS (e.g. from /etc/os-release): PRETTY_NAME="Debian GNU/Linux 10 (buster)"
  • Kernel (e.g. uname -a): Linux beczulka 5.3.0-0.bpo.2-amd64 #1 SMP Debian 5.3.9-2~bpo10+1 (2019-11-13) x86_64 GNU/Linux
@porridge
Copy link
Member Author

Unassigning for now, since I will be unavailable next week.
If anyone decides to tackle this in my absence, they may find the above draft PR with a minimal (if ugly) repro useful.
If not, I'll return to this in February.

@porridge porridge removed their assignment Jan 24, 2020
@ANeumann82 ANeumann82 self-assigned this Feb 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants