Skip to content

Conversation

@ishitasequeira
Copy link
Collaborator

@ishitasequeira ishitasequeira commented Dec 18, 2024

Update references of cache, env, health to use modules from registry-scanner

@codecov-commenter
Copy link

codecov-commenter commented Dec 18, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 71.99%. Comparing base (14e60b8) to head (5ec1dec).
Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #972      +/-   ##
==========================================
- Coverage   72.44%   71.99%   -0.46%     
==========================================
  Files          27       24       -3     
  Lines        3727     3642      -85     
==========================================
- Hits         2700     2622      -78     
+ Misses        885      880       -5     
+ Partials      142      140       -2     

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

@ishitasequeira ishitasequeira changed the title Update references of cache, common, env, health to use modules from registry-scanner Update references of cache, env, health to use modules from registry-scanner Dec 18, 2024
@ishitasequeira ishitasequeira marked this pull request as ready for review December 18, 2024 13:17
"github.com/argoproj-labs/argocd-image-updater/pkg/version"
"github.com/argoproj-labs/argocd-image-updater/registry-scanner/pkg/env"
"github.com/argoproj-labs/argocd-image-updater/registry-scanner/pkg/health"
"github.com/argoproj-labs/argocd-image-updater/registry-scanner/pkg/log"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is the health server referenced by registry-scanner for it to be relocated?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It is not. Do you feel we should keep it in image-updater itself?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah, since it's checks the health of argocd-image-updater, and it won't be able to run within registry-scanner. We can address that in a separate PR.

const DefaultTargetFilePattern = ".argocd-source-%s_%s.yaml"
const DefaultTargetFilePatternWithoutNamespace = ".argocd-source-%s.yaml"
const DefaultHelmValuesFilename = "values.yaml"

Copy link
Collaborator

Choose a reason for hiding this comment

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

I see both image-updater and registry-scanner have constants.go. Do we want to keep only fields required by registry-scanner in registry-scanner/constants.go, and remove those not used by registry-scanner?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, I am working on a logic to separate them out. It needs a bit of refactoring. I will have the PR updated.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I will move this change to a different PR.

Signed-off-by: Ishita Sequeira <[email protected]>
@chengfang chengfang merged commit 736f12f into argoproj-labs:master Dec 19, 2024
11 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.

3 participants