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

Vector aggregator chart update fails #25

Closed
tbondarchuk opened this issue Sep 2, 2021 · 4 comments · Fixed by #26
Closed

Vector aggregator chart update fails #25

tbondarchuk opened this issue Sep 2, 2021 · 4 comments · Fixed by #26

Comments

@tbondarchuk
Copy link

I'm using vector-aggregator chart with helmfile and update fails with:

Error: UPGRADE FAILED: cannot patch "vector" with kind StatefulSet: StatefulSet.apps "vector" is invalid: spec: Forbidden: updates to statefulset spec for fields other than 'replicas', 'template', and 'updateStrategy' are forbidden

helmfile diff (reducted):

monitoring, vector, StatefulSet (apps) has changed:
  # Source: vector-aggregator/templates/statefulset.yaml
  apiVersion: apps/v1
  kind: StatefulSet
  metadata:
    name: vector
    labels:
-     helm.sh/chart: vector-aggregator-0.16.1
+     helm.sh/chart: vector-aggregator-0.17.0
      app.kubernetes.io/name: vector
      app.kubernetes.io/instance: vector
      app.kubernetes.io/version: "0.16.1"
      app.kubernetes.io/managed-by: Helm
      app.kubernetes.io/part-of: vector
  spec:
    ...
    volumeClaimTemplates:
      # Vector will store it's data here.
      - metadata:
          name: data-dir
          labels:
-           helm.sh/chart: vector-aggregator-0.16.1
+           helm.sh/chart: vector-aggregator-0.17.0
            app.kubernetes.io/name: vector
            app.kubernetes.io/instance: vector
            app.kubernetes.io/version: "0.16.1"
            app.kubernetes.io/managed-by: Helm
            app.kubernetes.io/part-of: vector

Reason: {{- include "libvector.labels" $ | nindent 10 }} in sts template which comes from vector-shared

Looks like the only solution is to remove chart version from volumeClamTempalte's labels, please check this issue

@spencergilbert
Copy link
Contributor

spencergilbert commented Sep 2, 2021

Dang, immutable keys can be such a pain especially labels which seem very mutable...

I'll get a patch release out this morning, and also see if ct isn't checking the upgrade path.

EDIT: we are not today passing the --upgrade option during the ct install jobs
EDIT2: :( we also don't have a ci values file that tests the PVC options

@spencergilbert
Copy link
Contributor

@aliusmiles am I correct in my understanding that any aggregator deployed already is not upgradeable? It seems like the appropriate solution is to remove any labels that could change between upgrades and rip off the bandaid here?

@tbondarchuk
Copy link
Author

@spencergilbert I believe so, yes. I've started using aggregator chart a while back, when it's still was in beta but already published to your repo and had this issue couple of times I think: from beta to 16 then minor 16 update and now.

Unfortunately I didn't have enough time then to create an issue so just went with reinstall each time (dev env anyway). Guess either I'm the only one using it or just the first one to report;)

I believe chart/app version labels are not that important for volume and doing helm templating magic or move claim to separate resource from sts are not really worth an effort.

@spencergilbert
Copy link
Contributor

@aliusmiles definitely the first to report I think, I had also been using it frequently but similarly did a lot of uninstall and installs vs upgrades. I'll create a PR adding the --upgrade test and a ci file that creates a PVC, as well as adjusting the PVC labels to just be:

        labels:
          app.kubernetes.io/name: ...
          app.kubernetes.io/instance: ...
          app.kubernetes.io/component: aggregator

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants