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

Make Argo Workflows Deployment Restart on Config change using Helm ConfigMap annotation #2622

Open
ak-a opened this issue Apr 4, 2024 · 4 comments
Labels
argo-workflows enhancement New feature or request on-hold Issues or Pull Requests with this label will never be considered stale size/S

Comments

@ak-a
Copy link

ak-a commented Apr 4, 2024

Is your feature request related to a problem?

Using Argo-CD to deploy Argo Workflows means that it might not be automatically restarted if a configuration change is made that doesn't affect a deployment.

Related helm chart

argo-workflows

Describe the solution you'd like

Add an annotation to the Argo Workflows helm chart as documented:
https://helm.sh/docs/howto/charts_tips_and_tricks/#automatically-roll-deployments
Then the deployment will update when a ConfigMap updates and so a new deployment will rollout when using Argo CD.

Describe alternatives you've considered

Argo Workflows does have a config watcher, but this has been causing issues argoproj/argo-workflows#11657. Whilst a fix is in the works, this still has load on a cluster. The Helm/yaml annotation method is simpler (IMHO) and much cheaper in terms of resource.

Additional context

No response

@ak-a ak-a added the enhancement New feature or request label Apr 4, 2024
@ak-a
Copy link
Author

ak-a commented Apr 4, 2024

I initially requested in the wrong repo argoproj/argo-workflows#12876

@agilgur5
Copy link
Contributor

agilgur5 commented Apr 4, 2024

As I wrote in the upstream issue, this feature should be optional for the Controller as it is built into Workflows already. The value should also be explicitly documented as such.
Note that there is also currently no way to turn off the built-in feature without turning off other features, like the semaphore ConfigMap watcher, per argoproj/argo-workflows#12622.

This also would be good to have for the Server, which does not yet have a built-in watcher: argoproj/argo-workflows#10970

and much cheaper in terms of resource.

For this part, I don't know that this is necessarily correct. Disabling the watcher will certainly use less constant memory, but it may not be significant. In terms of CPU, disk I/O, and network I/O, restarting the entire deployment instead of an in-memory hot reload is certainly more expensive (and creates churn on the cluster).

I think it would be more appropriate to call it a trade-off when it comes to performance.

You could also call it an optional workaround for a current bug (and any future such bugs), which is caused by an upstream k8s issue, and which can cause performance degradation.

@nhavens
Copy link
Contributor

nhavens commented Apr 4, 2024

I agree with @agilgur5. I think this should be a toggle(s) in the helm chart values file. Either use the config watcher to hot-reload config changes, or use the helm checksum/config annotation convention to automatically restart both the server and controller deployments on config changes. Note that we always need to use the checksum/config annotation for the server until argoproj/argo-workflows#10970 has been resolved.

If the toggle is enabled:

  • Set the WATCH_CONTROLLER_SEMAPHORE_CONFIGMAPS env var to false for the controller.
  • Annotate both the server and controller deployments' pod templates with the checksum/config annotation to get them to automatically restart on config changes.

If the toggle is disabled (the default value):

  • Omit the WATCH_CONTROLLER_SEMAPHORE_CONFIGMAPS env var for the controller, or explicitly set it to true.
  • Annotate only the server deployment's pod template with the checksum/config annotation.

Copy link

github-actions bot commented Jun 4, 2024

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@agilgur5 agilgur5 added size/S pinned Workaround for missing Stale action feature (https://github.com/actions/stale/issues/990) and removed no-issue-activity labels Jun 4, 2024
@mkilchhofer mkilchhofer added on-hold Issues or Pull Requests with this label will never be considered stale and removed pinned Workaround for missing Stale action feature (https://github.com/actions/stale/issues/990) labels Oct 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
argo-workflows enhancement New feature or request on-hold Issues or Pull Requests with this label will never be considered stale size/S
Projects
None yet
Development

No branches or pull requests

4 participants