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

fix: controller option to not watch configmap #12622

Merged

Conversation

pquadri
Copy link
Contributor

@pquadri pquadri commented Feb 5, 2024

Fixes #11657

Original PR by @tooptoop4 here: #12400

Signed-off-by: Paolo Quadri <paolo.quadri@remote.com>
@pquadri pquadri force-pushed the fix/controller-option-to-not-watch-configmap branch from f795d40 to 05e5c36 Compare February 5, 2024 09:02
@tooptoop4
Copy link
Contributor

:shipit:

@agilgur5 agilgur5 self-assigned this Feb 5, 2024
@agilgur5 agilgur5 added solution/workaround There's a workaround, might not be great, but exists area/controller Controller issues, panics labels Feb 5, 2024
@pquadri
Copy link
Contributor Author

pquadri commented Feb 7, 2024

not sure about the failed test, probably a flaky one but i can't retry the action :/

@agilgur5
Copy link
Member

agilgur5 commented Feb 7, 2024

I gave it a retry. That was an Executor test, so sounds unrelated, but I'm not sure I've seen that specific test flake before (though some related tests have before TestStopBehavior) 🤔

Copy link
Member

@agilgur5 agilgur5 left a comment

Choose a reason for hiding this comment

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

Took a look at this in more depth now that it passes. Just one small addition to #12400, otherwise LGTM. Thanks for finishing this up!

docs/environment-variables.md Outdated Show resolved Hide resolved
workflow/controller/controller.go Outdated Show resolved Hide resolved
be more specific on the env var

Signed-off-by: Anton Gilgur <4970083+agilgur5@users.noreply.github.com>
be more specific on the env var

Signed-off-by: Anton Gilgur <4970083+agilgur5@users.noreply.github.com>
@agilgur5 agilgur5 enabled auto-merge (squash) February 8, 2024 22:17
@agilgur5 agilgur5 merged commit 5f4b235 into argoproj:main Feb 8, 2024
27 checks passed
isubasinghe pushed a commit to isubasinghe/argo-workflows that referenced this pull request Feb 27, 2024
Signed-off-by: Paolo Quadri <paolo.quadri@remote.com>
Signed-off-by: Anton Gilgur <4970083+agilgur5@users.noreply.github.com>
Co-authored-by: Anton Gilgur <4970083+agilgur5@users.noreply.github.com>
isubasinghe pushed a commit to isubasinghe/argo-workflows that referenced this pull request Feb 28, 2024
Signed-off-by: Paolo Quadri <paolo.quadri@remote.com>
Signed-off-by: Anton Gilgur <4970083+agilgur5@users.noreply.github.com>
Co-authored-by: Anton Gilgur <4970083+agilgur5@users.noreply.github.com>
Signed-off-by: Isitha Subasinghe <isubasinghe@student.unimelb.edu.au>
isubasinghe pushed a commit to isubasinghe/argo-workflows that referenced this pull request Mar 12, 2024
Signed-off-by: Paolo Quadri <paolo.quadri@remote.com>
Signed-off-by: Anton Gilgur <4970083+agilgur5@users.noreply.github.com>
Co-authored-by: Anton Gilgur <4970083+agilgur5@users.noreply.github.com>
github-merge-queue bot pushed a commit to linz/topo-workflows that referenced this pull request Mar 26, 2024
#### Motivation

In order to reduce the Argo controller logs due to [this
issue](argoproj/argo-workflows#11657), we can
use [this
workaround](argoproj/argo-workflows#12622) for
now.

#### Modification

Using `WATCH_CONFIGMAPS=false` workaround implies upgrading Argo
Workflows:
- upgrade Argo Workflows to `3.5.5`
- set `WATCH_CONFIGMAPS` to Argo Controller environment variable to
`false`

Non regression tested on a NonProd cluster.

#### Checklist

- [ ] Tests updated
- [x] Docs updated
- [x] Issue linked in Title
@xrafhue
Copy link

xrafhue commented Apr 3, 2024

Hello,

I try this workaround
image
but still having the issue
image

This increase our AWS CloudWatch costs.... we will add a filter on fluentbit to do not send this log...

Any idea why the workaround doesnt works ?

@tooptoop4
Copy link
Contributor

tooptoop4 commented Apr 3, 2024

@xrafhue the os variable should be WATCH_CONTROLLER_SEMAPHORE_CONFIGMAPS

@xrafhue
Copy link

xrafhue commented Apr 7, 2024

@tooptoop4 , after 2 days in all our clusters the logs are more generated. Thanks a lot.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/controller Controller issues, panics solution/workaround There's a workaround, might not be great, but exists
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Controller: invalid config map object received in config watcher. Ignored processing with CPU spike
4 participants