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

feat: Health status customization in Argo for CRD resources #655

Closed
wants to merge 26 commits into from

Conversation

almoelda
Copy link

Health handling for CRDs suggested by @agaudreault
for argoproj/argo-cd#21119

almoelda and others added 4 commits December 28, 2024 13:39
@almoelda almoelda requested a review from a team as a code owner December 28, 2024 14:09
Copy link

codecov bot commented Dec 28, 2024

Codecov Report

Attention: Patch coverage is 75.60976% with 20 lines in your changes missing coverage. Please review.

Project coverage is 54.70%. Comparing base (8849c3f) to head (c881350).
Report is 5 commits behind head on master.

Files with missing lines Patch % Lines
pkg/health/health_customresourcedefinition.go 74.35% 19 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #655      +/-   ##
==========================================
+ Coverage   54.26%   54.70%   +0.44%     
==========================================
  Files          64       65       +1     
  Lines        6164     6352     +188     
==========================================
+ Hits         3345     3475     +130     
- Misses       2549     2602      +53     
- Partials      270      275       +5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

almoelda and others added 16 commits December 29, 2024 13:54
Signed-off-by: Almo Elda <[email protected]>
- [x] fix warnings about case of `as` to `AS` in Dockerfile
  - `FromAsCasing: 'as' and 'FROM' keywords' casing do not match (line 1)`
- [x] shorten go version in go.mod
- [x] update Dockerfile Go version from 1.17 to 1.22 to match go.mod
- [x] upgrade alipine/git image version to latest, current was 4 years old
  - -from alpine/git:v2.24.3 (4 years old) to alpine/git:v2.45.2
- [x] fix warning with linting
  - `WARN [config_reader] The configuration option 'run.skip-files' is deprecated, please use 'issues.exclude-files'`
- [x] add .tool-versions (asdf) to .gitignore

Signed-off-by: jmeridth <[email protected]>
Signed-off-by: Almo Elda <[email protected]>
* fix: Server side diff now works correctly with some fields removal

Helps with argoproj/argo-cd#20792

Removed and modified sets may only contain the fields that changed, not including key fields like "name". This can cause merge to fail, since it expects those fields to be present if they are present in the predicted live.
Fortunately, we can inspect the set and derive the key fields necessary. Then they can be added to the set and used during a merge.
Also, have a new test which fails before the fix, but passes now.

Failure of the new test before the fix
```
            	Error:      	Received unexpected error:
            	            	error removing non config mutations for resource Deployment/nginx-deployment: error reverting webhook removed fields in predicted live resource: .spec.template.spec.containers: element 0: associative list with keys has an element that omits key field "name" (and doesn't have default value)
            	Test:       	TestServerSideDiff/will_test_removing_some_field_with_undoing_changes_done_by_webhook
```

Signed-off-by: Andrii Korotkov <[email protected]>

* Use new version of structured merge diff with a new option

Signed-off-by: Andrii Korotkov <[email protected]>

* Add DCO

Signed-off-by: Andrii Korotkov <[email protected]>

* Try to fix sonar exclusions config

Signed-off-by: Andrii Korotkov <[email protected]>

---------

Signed-off-by: Andrii Korotkov <[email protected]>
Signed-off-by: Almo Elda <[email protected]>
* fix: github actions versions and warnings

- [x] upgrade github/actions/cache GitHub Action to latest
 - this fixings the following warnings (example list [here](https://github.com/argoproj/gitops-engine/actions/runs/11885468091)):
   - Your workflow is using a version of actions/cache that is scheduled for deprecation, actions/[email protected]. Please update your workflow to use the latest version of actions/cache to avoid interruptions. Learn more: https://github.blog/changelog/2024-09-16-notice-of-upcoming-deprecations-and-changes-in-github-actions-services/
   - The `save-state` command is deprecated and will be disabled soon. Please upgrade to using Environment Files. For more information see: https://github.blog/changelog/2022-10-11-github-actions-deprecating-save-state-and-set-output-commands/
   - The `set-output` command is deprecated and will be disabled soon. Please upgrade to using Environment Files. For more information see: https://github.blog/changelog/2022-10-11-github-actions-deprecating-save-state-and-set-output-commands/
- [x] update dependabot config
  - prefix PRs with `chore(deps)`
  - group non major version updates into 1 PR

Signed-off-by: jmeridth <[email protected]>

* fix: switch to SHA from tags

more secure, as tags are mutable

Signed-off-by: jmeridth <[email protected]>

---------

Signed-off-by: jmeridth <[email protected]>
Signed-off-by: Almo Elda <[email protected]>
* chore: add CODEOWNERS and EMERITUS.md

Setting up CODEOWNERS file so people are automatically notified about new PRs. Can
eventually setup ruleset that requires at least one review before merging.

I based currently list in CODEOWNERS on people who have recently merged PRs. I'm wondering
if reviewers and approvers are a team/group instead of list of folks.

Setup EMERITUS.md that contains list from OWNERS.  Feedback on this PR will update
this PR.

Signed-off-by: jmeridth <[email protected]>

* chore: match this repo's CODEOWNERS to argoproj/argo-cd CODEOWNERS

Signed-off-by: jmeridth <[email protected]>

---------

Signed-off-by: jmeridth <[email protected]>
Signed-off-by: Almo Elda <[email protected]>
Change links:

- [x] from kubernetes slack to cncf slack
- [x] from k8s gitop channel to cncf argo-cd-contributors channel

Signed-off-by: jmeridth <[email protected]>
Signed-off-by: Almo Elda <[email protected]>
* fix: avoid resources lock contention utilizing channel

Signed-off-by: Mykola Pelekh <[email protected]>

* feat: process events in batch when the mode is enabled (default is `false`)

Signed-off-by: Mykola Pelekh <[email protected]>

* test: update unit tests to verify batch events processing flag

Signed-off-by: Mykola Pelekh <[email protected]>

* feat: make eventProcessingInterval option configurable (default is 0.1s)

Signed-off-by: Mykola Pelekh <[email protected]>

* fixup! feat: make eventProcessingInterval option configurable (default is 0.1s)

Signed-off-by: Mykola Pelekh <[email protected]>

---------

Signed-off-by: Mykola Pelekh <[email protected]>
Signed-off-by: Almo Elda <[email protected]>
Signed-off-by: Almo Elda <[email protected]>
…e and changed condition msg to a signle variable to be returned

Signed-off-by: Almo Elda <[email protected]>
Signed-off-by: Almo Elda <[email protected]>
Signed-off-by: Almo Elda <[email protected]>
…e and changed condition msg to a signle variable to be returned

Signed-off-by: Almo Elda <[email protected]>
Signed-off-by: Almo Elda <[email protected]>
Copy link

@almoelda almoelda closed this Dec 29, 2024
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.

4 participants