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(argoconfig): Remove empty syncPolicy from application template #1172

Merged
merged 3 commits into from
Jan 12, 2024

Conversation

maxthier
Copy link
Contributor

@maxthier maxthier commented Jan 12, 2024

Description

This PR removes the empty syncPolicy from the application template. This change is needed because when you don't specify a syncPolicy in one of the helm-charts which use argoconfig, it will put just the empty syncPolicy in it. It seems like argoCD removes the empty value, which can be seen here:

time="2024-01-11T17:04:51Z" level=info msg="Refreshing app status (comparison expired, requesting refresh. reconciledAt: 2024-01-11 16:58:51 +0000 UTC, expiry: 3m0s), level (2)" application=infra-argocd/app-stackrox-secured-cluster-services
time="2024-01-11T17:04:51Z" level=info msg="Comparing app state (cluster: https://kubernetes.default.svc, namespace: infra-stackrox)" application=infra-argocd/app-stackrox-secured-cluster-services
time="2024-01-11T17:04:51Z" level=info msg="getRepoObjs stats" application=infra-argocd/app-stackrox-secured-cluster-services build_options_ms=0 helm_ms=13 plugins_ms=0 repo_ms=0 time_ms=408 unmarshal_ms=395 version_ms=0
time="2024-01-11T17:04:52Z" level=info msg="Normalized app spec: {\"spec\":{\"syncPolicy\":null}}" application=infra-argocd/app-stackrox-secured-cluster-services
time="2024-01-11T17:04:53Z" level=info msg="Update successful" application=infra-argocd/app-stackrox-secured-cluster-services
time="2024-01-11T17:04:53Z" level=info msg="Reconciliation completed" application=infra-argocd/app-stackrox-secured-cluster-services dedup_ms=0 dest-name= dest-namespace=infra-stackrox dest-server="https://kubernetes.default.svc" diff_ms=793 fields.level=2 git_ms=408 health_ms=1 live_ms=300 patch_ms=202 setop_ms=0 settings_ms=0 sync_ms=0 time_ms=2794

This leads to a diff between the desired and live manifest.
After this change the helm-charts which depend on this also have to update their dependency to fix the issue.

Issues

Checklist

  • This PR contains a description of the changes I'm making
  • I updated the version in Chart.yaml
  • I updated the changelog with an artifacthub.io/changes annotation in Chart.yaml, check the example in the documentation.
  • I updated applicable README.md files using pre-commit run
  • I documented any high-level concepts I'm introducing in docs/
  • CI is currently green and this is ready for review
  • I am ready to test changes after they are applied and released

@maxthier maxthier self-assigned this Jan 12, 2024
@github-actions github-actions bot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Jan 12, 2024
@maxthier maxthier requested a review from eyenx January 12, 2024 08:09
@maxthier maxthier marked this pull request as ready for review January 12, 2024 08:09
@maxthier maxthier requested review from hairmare and a team as code owners January 12, 2024 08:09
Copy link
Member

@eyenx eyenx left a comment

Choose a reason for hiding this comment

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

LGTM

@maxthier maxthier merged commit 7b44fb1 into main Jan 12, 2024
3 checks passed
@maxthier maxthier deleted the fix/outofsync branch January 12, 2024 08:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants