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

docs: Add an explanation of using ignoring differences for app of apps pattern #18945

Conversation

andrii-korotkov-verkada
Copy link
Contributor

@andrii-korotkov-verkada andrii-korotkov-verkada commented Jul 4, 2024

Some fields updates trigger app of apps being out of sync on a regular basis, or prevent from persistently modifying some behaviors like auto sync without doing the change in the code. This update shows how to do this using ignore differences configuration.

Checklist:

  • Either (a) I've created an enhancement proposal and discussed it with the community, (b) this is a bug fix, or (c) this does not need to be in the release notes.
  • The title of the PR states what changed and the related issues number (used for the release note).
  • The title of the PR conforms to the Toolchain Guide
  • I've included "Closes [ISSUE #]" or "Fixes [ISSUE #]" in the description to automatically close the associated issue.
  • I've updated both the CLI and UI to expose my feature, or I plan to submit a second PR with them.
  • Does this PR require documentation updates?
  • I've updated documentation as required by this PR.
  • I have signed off all my commits as required by DCO
  • I have written unit and/or e2e tests for my change. PRs without these are unlikely to be merged.
  • My build is green (troubleshooting builds).
  • My new feature complies with the feature status guidelines.
  • I have added a brief description of why this PR is necessary and/or what this PR solves.
  • Optional. My organization is added to USERS.md.
  • Optional. For bug fixes, I've indicated what older releases this fix should be cherry-picked into (this may or may not happen depending on risk/complexity).

Copy link

bunnyshell bot commented Jul 4, 2024

❌ Preview Environment deleted from Bunnyshell

Available commands (reply to this comment):

  • 🚀 /bns:deploy to deploy the environment

Copy link

bunnyshell bot commented Jul 4, 2024

❌ Preview Environment deleted from Bunnyshell

Available commands (reply to this comment):

  • 🚀 /bns:deploy to deploy the environment

@andrii-korotkov-verkada andrii-korotkov-verkada force-pushed the docs-for-app-of-apps-update branch 3 times, most recently from 49edc3b to 4af1a41 Compare July 9, 2024 15:10
@andrii-korotkov-verkada
Copy link
Contributor Author

Looks like License Compliance triggers on master, since I don't think my changes break it.

```
```

### Ignoring some differences in managed applications
Copy link
Member

Choose a reason for hiding this comment

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

Seems like this would make more sense in docs/user-guide/diffing.md. Perhaps in an example section that could contain known patterns.

What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'd like to keep some reference here, since the page is dedicated to app of apps and that's where people would come to when searching for app of apps. They'd probably not go to diffing page right away. The filters are a bit specific for app of apps, so it seems to make more sense to keep them here. Though I can potentially add changes to the diffing page and link them from this page.

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
### Ignoring some differences in managed applications
### Ignoring differences in child applications

Copy link
Contributor

Choose a reason for hiding this comment

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

@agaudreault if you Google "Argo CD app of apps" this page is the first result and is essentially the canonical source for app of apps. I think we should leave changing that pattern to another PR. In the meantime, for this PR @andrii-korotkov-verkada lets just add a link from diffing customization page to this one to discuss app of apps.

Copy link
Contributor

@todaywasawesome todaywasawesome left a comment

Choose a reason for hiding this comment

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

I made some suggestions just to clean up wording a bit to clarify things. You can just click "sign off and commit suggestion" if you're ok with those changes.

Could you also clarify why the annotations need to be ignored?

The only thing I would add is a link from the diffing customization page.

```
```

### Ignoring some differences in managed applications
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
### Ignoring some differences in managed applications
### Ignoring differences in child applications


### Ignoring some differences in managed applications

If you don't want your app of apps becoming out of sync on a regular basis and/or want to manually modify some configurations for managed applications like auto sync being enabled, you can configure ignoring differences in the app of apps, e.g.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
If you don't want your app of apps becoming out of sync on a regular basis and/or want to manually modify some configurations for managed applications like auto sync being enabled, you can configure ignoring differences in the app of apps, e.g.
To allow changes in child apps without triggering an out-of-sync status, or modification for debugging etc, the app of apps pattern works with [diff customization](../user-guide/diffing/). The example below shows how to ignore changes to syncPolicy and other common values.

Comment on lines 141 to 145
# To allow manually turn off auto sync for apps and preserve that setting
- /spec/syncPolicy/automated
# These are automatically updated on a regular basis. Not ignoring last applied configuration since it's used for computing diffs after normalization.
- /metadata/annotations/argocd.argoproj.io~1refresh
- /operation
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
# To allow manually turn off auto sync for apps and preserve that setting
- /spec/syncPolicy/automated
# These are automatically updated on a regular basis. Not ignoring last applied configuration since it's used for computing diffs after normalization.
- /metadata/annotations/argocd.argoproj.io~1refresh
- /operation
# Allow manually disabling auto sync for apps, useful for debugging.
- /spec/syncPolicy/automated
# These are automatically updated on a regular basis. Not ignoring last applied configuration since it's used for computing diffs after normalization.
- /metadata/annotations/argocd.argoproj.io~1refresh
- /operation

Copy link
Contributor

Choose a reason for hiding this comment

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

@andrii-korotkov-verkada Can you explain why you ignore the annotations? I wouldn't think this would add anything.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@todaywasawesome, I've noticed that some annotations and /operation were constantly triggering out of sync status for the app of apps despite no changes in the manifests, since they could be added/removed/updated by automation. I thought it'll be cleaner to ignore them.

```
```

### Ignoring some differences in managed applications
Copy link
Contributor

Choose a reason for hiding this comment

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

@agaudreault if you Google "Argo CD app of apps" this page is the first result and is essentially the canonical source for app of apps. I think we should leave changing that pattern to another PR. In the meantime, for this PR @andrii-korotkov-verkada lets just add a link from diffing customization page to this one to discuss app of apps.

@christianh814
Copy link
Member

@andrii-korotkov-verkada Are you able to resolve some of the feedback from Dan?

@andrii-korotkov-verkada
Copy link
Contributor Author

Yeah, I'll take a look in upcoming days.

…s pattern

Some fields updates trigger app of apps being out of sync on a regular basis, or prevent from persistently modifying some behaviors like auto sync without doing the change in the code. This update shows how to do this using ignore differences configuration.

Signed-off-by: Andrii Korotkov <andrii.korotkov@verkada.com>
@andrii-korotkov-verkada
Copy link
Contributor Author

Updated the PR.

Copy link
Contributor

@todaywasawesome todaywasawesome left a comment

Choose a reason for hiding this comment

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

Looks good, thanks!

@todaywasawesome todaywasawesome enabled auto-merge (squash) July 17, 2024 16:13
@todaywasawesome todaywasawesome merged commit a06cdb3 into argoproj:master Jul 17, 2024
23 of 24 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.

None yet

4 participants