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

Multi-Source App that is OutOfSync without selfHeal and with manifest-generate-paths is synced when refresh is triggered #20809

Closed
3 tasks done
Ezzahhh opened this issue Nov 15, 2024 · 7 comments · Fixed by #20811
Assignees
Labels
question Issue is a question or reach for support version:2.13 Latest confirmed affected version is 2.13

Comments

@Ezzahhh
Copy link
Contributor

Ezzahhh commented Nov 15, 2024

Checklist:

  • I've searched in the docs and FAQ for my answer: https://bit.ly/argocd-faq.
  • I've included steps to reproduce the bug.
  • I've pasted the output of argocd version.

Describe the bug

A multi-source app with selfHeal disabled and with manifest-generate-paths annotation will be synced (when it is already OutOfSync) if a refresh on it is triggered (either manually or through automatic refreshes 3min by default) and the last sync is not the same as HEAD.

Perhaps these PRs could be related? #19799 #19828

To Reproduce

Create a Multi-Source AppSet
argocd.argoproj.io/manifest-generate-paths annotation

  ignoreApplicationDifferences:
    - jsonPointers:
        - /spec/syncPolicy

Enable auto-sync
default selfHeal to true
Choose an App of the AppSet to disable the selfHeal.
Make a change on the cluster to force the App OutOfSync.
Push to the repo so that the last sync of the App and HEAD will be different.
Wait for the automatic refresh or refresh manually on the App.
Observe the OutOfSync App to be synced despite self heal is disabled.

Expected behavior

An OutOfSync multi-source app with selfHeal disabled should not be synced when unrelated commits to HEAD occur and the App is refreshed. In other words, the selfHeal and manifest-generate-paths should be respected regardless.

Version

{
    "Version": "v2.13.0+347f221",
    "BuildDate": "2024-11-04T12:09:06Z",
    "GitCommit": "347f221adba5599ef4d5f12ee572b2c17d01db4d",
    "GitTreeState": "clean",
    "GoVersion": "go1.23.1",
    "Compiler": "gc",
    "Platform": "linux/amd64",
    "KustomizeVersion": "v5.4.3 2024-07-19T16:40:33Z",
    "HelmVersion": "v3.15.4+gfa9efb0",
    "KubectlVersion": "v0.31.0",
    "JsonnetVersion": "v0.20.0"
}
@Ezzahhh Ezzahhh added the bug Something isn't working label Nov 15, 2024
@Ezzahhh Ezzahhh changed the title Multi-Source App that is OutOfSync with selfHeal disabled and manifest-generate-paths is synced when unrelated App is synced Multi-Source App that is OutOfSync without selfHeal and with manifest-generate-paths is synced when unrelated App is synced Nov 15, 2024
@Ezzahhh Ezzahhh changed the title Multi-Source App that is OutOfSync without selfHeal and with manifest-generate-paths is synced when unrelated App is synced Multi-Source App that is OutOfSync without selfHeal and with manifest-generate-paths is synced when refresh is triggered Nov 15, 2024
@andrii-korotkov-verkada andrii-korotkov-verkada added the version:2.13 Latest confirmed affected version is 2.13 label Nov 16, 2024
@andrii-korotkov-verkada
Copy link
Contributor

Is autosync enabled?

@andrii-korotkov-verkada andrii-korotkov-verkada added the more-information-needed Further information is requested label Nov 16, 2024
@Ezzahhh
Copy link
Contributor Author

Ezzahhh commented Nov 16, 2024

@andrii-korotkov-verkada Yes, auto-sync is enabled. I've been reviewing some of the code and it seems like this could potentially be intended behaviour? However, I think it's confusing to be able to disable self heal and then have subsequent non-related commits to cause a sync - I think it's expected if a related commit will cause a sync only if the App has actually been changed (e.g. paths to manifest-generate-paths has changed relevant to the App).

@andrii-korotkov-verkada
Copy link
Contributor

Yes, autosync enabled would cause automatic updates after refresh if there are changes - that's intended. Self-heal serves a different purpose, to override manual changes. Do you think documentation update would help clarify things?

@andrii-korotkov-verkada andrii-korotkov-verkada added question Issue is a question or reach for support and removed bug Something isn't working labels Nov 16, 2024
@Ezzahhh
Copy link
Contributor Author

Ezzahhh commented Nov 16, 2024

Yes, I think it might help clarify the self heal functionality as I made some assumptions about how it's meant to work (which was incorrect). The confusing thing for me is that when using non-multi-source apps there is no issue because when deploying helm I am usually not changing the helm target version (but the values) so only changes to that specific App values (like changing image tag) will cause a sync but other commits to the repo for other Apps do not cause unrelated syncing. However, since we point to HEAD in multi-source apps to obtain values, Argo only knows that the target revision has changed but not whether the multi-source App has any of its files materially changed (I think we do this check in GetRepoObjs but it's not passed into autoSync). As a result, we get this divergence of expected behaviour when making the switch from non-multi-source to multi-source - maybe this is worth pointing out in the docs?

Perhaps just an extra sentence in this section?

Thank you for the quick response and clarifying btw!

@andrii-korotkov-verkada
Copy link
Contributor

when using non-multi-source apps there is no issue because when deploying helm I am usually not changing the helm target version (but the values)

since we point to HEAD in multi-source apps to obtain values

What's the reason for this discrepancy?

@Ezzahhh
Copy link
Contributor Author

Ezzahhh commented Nov 16, 2024

Originally, our Apps were not multi-source so they look like:

      source:
        chart: my-chart
        repoURL: my-chart.com
        targetRevision: 0.1.0
        helm:
          releaseName: my-release
          values: |
            # my values

Usually we would only make two kind of changes: 1) changing the values (like the image tag); or 2) changing the target revision on the chart. Doing either of these changes would understandably cause a sync, regardless if Apps have selfheal disabled or not.

As far as I understand, in this case, Argo is keeping track of the target revision from the helm repository (e.g. 0.1.0). I think this is why subsequent unrelated pushes to the argo repo does not cause a sync/refresh even if it is OutOfSync with self heal disabled.

Once we convert this into a multi-source app, it looks like this:

      sources:
        - repoURL: [email protected]:x/x-x-x.git
          targetRevision: HEAD
          ref: values
        - chart: my-chart
          repoURL: my-chart.com
          targetRevision: 0.1.0
          helm:
            releaseName: my-release
            ignoreMissingValueFiles: true
            valueFiles:
              - $values/chart/values.yaml
            values: |
              # my values

Argo now keeps track of two revisions. One for helm and the other being the HEAD commit of my argo repo. Now, whenever an unrelated commit is pushed to the argo repo, it will trigger a refresh of this OutOfSync app because the HEAD target revision will change on any commit.

In the non-multi-source app, any subsequent refreshes that are triggered will not cause a sync (as long as the helm chart version is not changed or any value overrides in the app). I think it just comes down to the fact that the git repo's commit is not directly relevant for non-multi-source helm apps. Perhaps it will have the same behaviour as multi-source if the helm chart points to the repo as opposed to a helm repository? I am not sure.

@andrii-korotkov-verkada
Copy link
Contributor

I see, I think you just need to have autosync disabled to avoid this.

@andrii-korotkov-verkada andrii-korotkov-verkada removed the more-information-needed Further information is requested label Nov 16, 2024
andrii-korotkov-verkada added a commit to andrii-korotkov-verkada/argo-cd that referenced this issue Nov 16, 2024
…-source apps (argoproj#20809)

In single-source apps self-heal disabled guarantees that with no source changes the live changes won't get overridden. That's no longer the case for multi-source apps, since one of the sources changing can trigger a sync of everything. Add a note explaining this behavior.

Signed-off-by: Andrii Korotkov <[email protected]>
andrii-korotkov-verkada added a commit to andrii-korotkov-verkada/argo-cd that referenced this issue Nov 16, 2024
…-source apps (argoproj#20809)

Closes argoproj#20809

In single-source apps self-heal disabled guarantees that with no source changes the live changes won't get overridden. That's no longer the case for multi-source apps, since one of the sources changing can trigger a sync of everything. Add a note explaining this behavior.

Signed-off-by: Andrii Korotkov <[email protected]>
andrii-korotkov-verkada added a commit to andrii-korotkov-verkada/argo-cd that referenced this issue Nov 18, 2024
…-source apps (argoproj#20809)

Closes argoproj#20809

In single-source apps self-heal disabled guarantees that with no source changes the live changes won't get overridden. That's no longer the case for multi-source apps, since one of the sources changing can trigger a sync of everything. Add a note explaining this behavior.

Signed-off-by: Andrii Korotkov <[email protected]>
ishitasequeira pushed a commit that referenced this issue Nov 18, 2024
…-source apps (#20809) (#20811)

Closes #20809

In single-source apps self-heal disabled guarantees that with no source changes the live changes won't get overridden. That's no longer the case for multi-source apps, since one of the sources changing can trigger a sync of everything. Add a note explaining this behavior.

Signed-off-by: Andrii Korotkov <[email protected]>
dmosesson pushed a commit to dmosesson/argo-cd that referenced this issue Nov 20, 2024
…-source apps (argoproj#20809) (argoproj#20811)

Closes argoproj#20809

In single-source apps self-heal disabled guarantees that with no source changes the live changes won't get overridden. That's no longer the case for multi-source apps, since one of the sources changing can trigger a sync of everything. Add a note explaining this behavior.

Signed-off-by: Andrii Korotkov <[email protected]>
adriananeci pushed a commit to adriananeci/argo-cd that referenced this issue Dec 4, 2024
…-source apps (argoproj#20809) (argoproj#20811)

Closes argoproj#20809

In single-source apps self-heal disabled guarantees that with no source changes the live changes won't get overridden. That's no longer the case for multi-source apps, since one of the sources changing can trigger a sync of everything. Add a note explaining this behavior.

Signed-off-by: Andrii Korotkov <[email protected]>
Signed-off-by: Adrian Aneci <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Issue is a question or reach for support version:2.13 Latest confirmed affected version is 2.13
Projects
None yet
2 participants