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

feat(vector): Add option to roll vector pods on secrets and extra objects change #439

Merged
merged 1 commit into from
Dec 18, 2024

Conversation

anas-aso
Copy link
Contributor

Changes to secrets or extra manifests defined does not trigger a rolling restart.
This PR adds an option to do that.

Normally I would turn .Values.rollWorkload into an object, then defined something like this

rollWorkload:
  configMap: true
  secrets: false
  extraObjects: false

but that would be a breaking change. Maybe something to consider for the next major release of this chart.

@jszwedko jszwedko changed the title Add option to roll vector pods on secrets and extra objects change feat(vector): Add option to roll vector pods on secrets and extra objects change Dec 17, 2024
Copy link
Member

@jszwedko jszwedko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @anas-aso ! I agree that it'd be nice to turn rollWorkload into an object, but your approach avoids the breaking change. Out of curiosity, do you think it'd make sense to have rollWorkload roll if any of the config map, secrets, or extra objects changes? Or is it useful to be able to configure which changes will cause a roll?

@anas-aso
Copy link
Contributor Author

do you think it'd make sense to have rollWorkload roll if any of the config map, secrets, or extra objects changes? Or is it useful to be able to configure which changes will cause a roll

@jszwedko depends on the use case. For things where Vector's hot reload works (--watch-config), I would prefer to not recreate the pods. But for things like environment variables read from a k8s secret (example below), a rolling restart is a must.

env:
  - name: MY_SINK_TOKEN
    valueFrom:
      secretKeyRef:
        name: vector-aggregator
        key: MY_SINK_TOKEN

My use case is simple:

  1. hot reload Vector on config change (faster deployment time)
  2. recreate pods when an env variable read from a k8s secrets is changed.

I understand the concern of adding more knobs to the chart, so I am open to other suggestions if you have any.

@anas-aso anas-aso requested a review from jszwedko December 18, 2024 20:50
Copy link
Member

@jszwedko jszwedko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gotcha, I see, this change seems reasonable then to be able to only role when, for example, secrets change but not otherwise.

@jszwedko jszwedko enabled auto-merge (squash) December 18, 2024 20:56
@jszwedko jszwedko merged commit d7a05fc into vectordotdev:develop Dec 18, 2024
6 of 8 checks passed
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 this pull request may close these issues.

2 participants