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: broken check for context in notificationController #1626

Merged

Conversation

anandrkskd
Copy link
Contributor

What type of PR is this?

/kind bug

What does this PR do / why we need it:
This PR fix the broken check for context fields in notification configmap and notificationConfiguration CR.

Have you updated the necessary documentation?

  • Documentation update is required by this PR.
  • Documentation has been updated.

Which issue(s) this PR fixes:

Fixes GITOPS-5970

How to test changes / Special notes to the reviewer:

  • run controller on local using make install run
  • create an argo-cd instance with notification controller enabled
  • create a notificationConfiguraion with single context and check for notification-controller pod logs
  • update the context field in the notificationConfiguration with more context and check for notification-controller pod logs, there should not be indefinitely repeated logs similar to time="2024-11-21T09:40:54Z" level=info msg="invalidated cache for resource in namespace: argocd-debug with the name: argocd-notifications-cm" .

@anandrkskd anandrkskd force-pushed the fix-broken-notification-context-check branch from e796502 to ef16945 Compare December 18, 2024 10:03
@anandrkskd anandrkskd force-pushed the fix-broken-notification-context-check branch from ef16945 to 1574590 Compare December 18, 2024 10:10
Signed-off-by: Anand Kumar Singh <[email protected]>
Copy link
Collaborator

@svghadi svghadi left a comment

Choose a reason for hiding this comment

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

Left a small suggestion, rest all looks good. Thanks

@anandrkskd anandrkskd force-pushed the fix-broken-notification-context-check branch from 347604a to 2d270ab Compare January 20, 2025 07:27
Signed-off-by: Anand Kumar Singh <[email protected]>
@anandrkskd anandrkskd force-pushed the fix-broken-notification-context-check branch from 2d270ab to 97274b7 Compare January 20, 2025 07:40
Copy link
Collaborator

@svghadi svghadi left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks.

@svghadi svghadi merged commit eace21c into argoproj-labs:master Jan 20, 2025
7 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