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.

Sign up for GitHub

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

Jump to bottom

A dedicated field to enable or disable application syncPolicy #21647

Closed
anandrkskd opened this issue Jan 23, 2025 · 9 comments · Fixed by #21999 or #22482
Closed

A dedicated field to enable or disable application syncPolicy #21647

anandrkskd opened this issue Jan 23, 2025 · 9 comments · Fixed by #21999 or #22482
Assignees
@anandrkskd
Labels
component:sync enhancement New feature or request good first issue Good for newcomers

Comments

@anandrkskd
Copy link
Contributor

anandrkskd commented Jan 23, 2025 •
edited
Loading

Summary

Adding a dedicated field to set application sync Policy to enable or disable automated sync.

Motivation

An Application (app-of-apps pattern) which contains child Application resources only and which have autoSync with selfHeal enabled. The parent Application to force child Applications to have autoSync completely disabled. I need parent Application selfHeal to disable autoSync of child applications again when somebody enables it directly in Kubernetes.

Autosync is disabled only when spec.syncPolicy.automated is not present on resource and to write child Application resources in the way that ArgoCD forces it in Kubernetes with field spec.syncPolicy.automated absent.
Setting field spec.syncPolicy: {} and spec.syncPolicy.automated: null none of them works if set in child application in ArgoCD.

example scenario
  • creating an application (apps of apps) 
        cat <<EOF | kubectl create -f-
    apiVersion: argoproj.io/v1alpha1
    kind: Application
    metadata:
      name: app-of-apps
      namespace: argocd1
      finalizers:
      - resources-finalizer.argocd.argoproj.io
    spec:
      destination:
        namespace: helm-guestbook-1
        server: https://kubernetes.default.svc
      project: default
      source:
        path: .
        repoURL: https://github.com/anandrkskd/argocd-issue-autosync.git
        targetRevision: main
      syncPolicy:
        automated:
          selfHeal: true
          prune: true
    EOF
  • once the applications are up, updating syncPolicy
    $ argocd app set --sync-policy auto --sync-option CreateNamespace=true helm-guestbook-1
    $ argocd app set --sync-policy auto --sync-option CreateNamespace=true helm-guestbook-2

Automatic synchronization on child Applications is supposed to be turned off, but it is turned on (didn't revert to what is present in git), even though the parent Application has selfHeal enabled.

Proposal

Expected behavior is to have a possibility to enforce autosync disabled on child Application by parent Application selfHeal by having a flag to disable autosync in automated block, like:

syncPolicy:
    automated:
      enabled: true           <- new flag proposal
      prune: true
      selfHeal: true

To make compatible,

  • if the syncPolicy block is absent, no syncPolicy will change
  • if the syncPolicy contains automated field, the default will be set to true
  • if the syncPolicy.automated.enabled field is set to false, it will stop automated sync, even if the syncPolicy.automated.prune/selfHeal is set.
@anandrkskd anandrkskd added the enhancement New feature or request label Jan 23, 2025
@leoluz leoluz added good first issue Good for newcomers and removed component:sync labels Jan 23, 2025
@nitishfy
Copy link
Member

I'd like to take this up.

@nitishfy nitishfy self-assigned this Jan 23, 2025
@agaudreault agaudreault mentioned this issue Jan 23, 2025
Open
@agaudreault
Copy link
Member

Some implementation ideas given in the contributor meeting was to change the automated from a field that is nullable to a non-nullable struct. This way, the implementation easily know that enabled:false is defined even if the manifest does not contain the object.

@anandrkskd
Copy link
Contributor Author

Hey @nitishfy, I have already started working on a solution for this, if you don't mind I would like to work on this.

@keithchong
Copy link
Contributor

Would there be UI impacts to this? You can create a new app and specify these sync options, and, edit an existing app and change the sync options:

Image Image

@nitishfy
Copy link
Member

Hey @nitishfy, I have already started working on a solution for this, if you don't mind I would like to work on this.

Sure. Pleas make sure to comment under the issue that you're working on it already in order to avoid conflicts. Thanks, I am assigning this issue to you.

@nitishfy nitishfy assigned anandrkskd and unassigned nitishfy Jan 24, 2025
@anandrkskd
Copy link
Contributor Author

I will make sure from next time.

Thank you, @nitishfy

@andrii-korotkov-verkada
Copy link
Contributor

@anandrkskd, do you have any updates or need any help?

Closed
@anandrkskd
Copy link
Contributor Author

Hey @andrii-korotkov-verkada , I have a draft PR up, need to add tests, docs for the changes and fix the CI failures

@andrii-korotkov-verkada
Copy link
Contributor

Thanks! Left some comments.

@anandrkskd anandrkskd mentioned this issue Mar 5, 2025
Merged
14 tasks
@aali309 aali309 mentioned this issue Mar 25, 2025
Merged
14 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@anandrkskd anandrkskd

Labels
component:sync enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Milestone
No milestone
6 participants
@leoluz @keithchong @anandrkskd @agaudreault @nitishfy @andrii-korotkov-verkada