fix: Child apps should not affect parent app's health by default (#3781)

[keithchong]

Signed-off-by: Keith Chong kykchong@redhat.com

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).
• 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.
• Optional. My organization is added to USERS.md.
• I've signed the CLA and my build is green (troubleshooting builds).

Fix is for #3781
For checklist item 3 above, from what I could tell, no UI or CLI changes are necessary.

See screenshots in the issue that show the before and after behaviour.

[codecov-commenter]

Codecov Report

Merging #4448 into master will decrease coverage by 0.10%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #4448      +/-   ##
==========================================
- Coverage   41.29%   41.18%   -0.11%     
==========================================
  Files         122      122              
  Lines       16584    16584              
==========================================
- Hits         6848     6830      -18     
- Misses       8750     8772      +22     
+ Partials      986      982       -4     

Impacted Files	Coverage Δ
util/health/health.go	80.00% <100.00%> (ø)
controller/appcontroller.go	48.10% <0.00%> (-2.07%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8b2e05c...1d458b0. Read the comment docs.

[OmerKahani]

Would there be a way to change this default? the idea that there is one place to see the state of the whole cluster is something we rely on

[alexmt]

I propose to delete the built-in application health check instead of introducing special for child application health.

@OmerKahani, in this case, you can add custom health check using lua script. Given that application CRD health assessment causes confusion for so may Argo CD users , this seems like a good compromise.

What do you think?

[keithchong]

@alexmt, I presume you mean to essentially remove that if statement at line 51, because that specifically checks for the Application. If you remove that, then the App will revert back to missing or unknown, if the children are worse than healthy (missing or unknown), which then means the children will affect the parent's status.

[keithchong]

@alexmt , is this what you had in mind? Because you don't want any child application's unhealthy state (unknown, missing, and degraded) to affect the parent, then if at least one child is healthy, then the parent is healthy, like in the screenshot below? There is also the case where if all child apps have missing states, then it seems acceptable that the parent should have a missing state too.

[keithchong]

Thanks @alexmt for clarifying what you wanted. The intended behaviour is that the health status no longer appears on the child apps.

[keithchong]

Closing this PR because the fix is in the gitops-engine. See argoproj/gitops-engine#153

@OmerKahani, please add your comments to issue #3781, regarding @alexmt 's question.

[keithchong]

The refactor piece to pull in the newer version of the gitops-engine will be done in a different PR.