docs: image updater first class status proposal #11787
Conversation
Signed-off-by: Jaideep Rao <[email protected]>
Codecov ReportPatch and project coverage have no change.
Additional details and impacted files@@ Coverage Diff @@
## master #11787 +/- ##
=======================================
Coverage 49.14% 49.15%
=======================================
Files 248 248
Lines 42891 42891
=======================================
+ Hits 21079 21082 +3
+ Misses 19693 19691 -2
+ Partials 2119 2118 -1 ☔ View full report in Codecov by Sentry. |
Signed-off-by: Jaideep Rao <[email protected]>
…er-first-class-support-proposal
|
Hi @jannfis @wtam2018 @alexmt @jessesuen @leoluz @crenshaw-dev @daniel-codefresh This is a follow up proposal for making Image Updater a first class citizen in Argo CD. It discusses how we envision Image Updater can fit into Argo CD by leveraging the Application CR fields and how this would set it up for future development while providing value to the users and community at large. All comments/feedback are much appreciated, TIA! |
…er-first-class-support-proposal
|
@leoluz @jannfis Benefits of merging image-updater into application-controller:
Drawbacks:
Please add any other aspects I might have missed or been unaware of that could help better inform this discussion, Thanks! |
…er-first-class-support-proposal
Signed-off-by: Jaideep Rao <[email protected]>
jaideepr97
force-pushed
the
image-updater-first-class-support-proposal
branch
from
8f5807f to
cb8cfdd
Compare
| helm: | ||
| imageName: <name of helm parameter to set for the image name> # for git write back | ||
| imageTag: <name of helm parameter to set for the image tag> | ||
| imageSpec: <name of helm parameter to set for canonical name of image> | ||
| kustomize: | ||
| imageName: <original_image_name> # for git write back |
There was a problem hiding this comment.
This seems redundant with the image values already provided in the imageList.path
There was a problem hiding this comment.
I think we would need this if users already have specific, non-standard field names for their helm/kustomize parameters right?
There was a problem hiding this comment.
For some additional context: helm.imageTag here is the name of the helm parameter that should hold the value for the updated image tag in spec.source.helm.parameters. So if someone were to set helm.imageTag: someImage.version image updater would know to set spec.source.helm.parameters.someImage.version=<updated_version> in the application spec
|
|
||
| ``` | ||
|
|
||
| There is a need to introduce state by tracking the status of image Updates in the application CR. This is mainly important to honor application rollbacks, avoid repeated image updates in case of failure, as well as to support future use cases such as enabling image-updater to create pull requests against the source repositories. The above detailed `.status.imageUpdates` field could be extended to store metadata regarding pull request creation that would serve to avoid duplicate/excessive number of pull requests being opened, for example. |
There was a problem hiding this comment.
As suggested above, I think it would be great to consider a solution where we don't have to commit back to Git at all in order to have the image update functionality. We could have something similar to how the ignoreDifference configuration works today but used for updating images and defining the current tag.
There was a problem hiding this comment.
Git write backs have the advantage of reproducibility and version controlling a crucial component of applications upgrades
…er-first-class-support-proposal
…er-first-class-support-proposal
…ns and review Signed-off-by: Jaideep Rao <[email protected]>
…er-first-class-support-proposal
Signed-off-by: Jaideep Rao <[email protected]>
…er-first-class-support-proposal
…er-first-class-support-proposal
Signed-off-by: Jaideep Rao <[email protected]>
Signed-off-by: Jaideep Rao <[email protected]>
…er-first-class-support-proposal
| path: "../../base" | ||
|
|
||
| # (optional) credentials for git write-back | ||
| secret: |
There was a problem hiding this comment.
Is it possible to re-use ArgoCD repo auth (e.g using GitHub App)? or is this something in the roadmap?
There was a problem hiding this comment.
Good point. I think the goal is to re-use Argo CD provided authentication mechanism by default (and we do it right now, too). Currently, we are lagging a little behind Argo CD's authentication mechanisms, though. But yes, we definitely want to streamline authentication to Git between Image Updater and Argo CD.
| forceUpdate: true/false | ||
|
|
||
| # specify strategy to be followed when determining which images have qualifying updates | ||
| updateStrategy: semver/digest/lexical/most-recently-built |
There was a problem hiding this comment.
should most-recently-built match the existing value latest?
There was a problem hiding this comment.
latest will be deprecated in Image Updater by the next iteration and is to be replaced by most-recently-build.
| env: DOCKER_HUB_CREDDS | ||
|
|
||
| # or use an external script mounted to image-updater FS to generate credentials | ||
| ext: <path/to/script> |
There was a problem hiding this comment.
include credsexpire here as well?
There was a problem hiding this comment.
Hi @mubarak-j thanks for the feedback!
You mean just a timestamp to know when the credentials expire right?
There was a problem hiding this comment.
added, thanks for the suggestion!
|
|
||
| # (optional) specify helm parameter names to be used for image update write back | ||
| helm: | ||
| imageName: someImage.image |
There was a problem hiding this comment.
I know this is the existing config, but in the future can we consider not requiring helm.imageName if a user is updating only one image?
There was a problem hiding this comment.
The problem here is that we don't know (or can make an informed guess on) the name of the parameter to set. There is a default that we use, that probably works with a couple of off-the-shelve charts.
| - image: quay.io/some/image | ||
| lastTransitionTime: <update_timestamp> | ||
| oldTag: v1.0.0 | ||
| newTag: v1.1.0 |
There was a problem hiding this comment.
this is nice and would help us with more accurate notifications for image updates. currently notifications pickup both tags () during rollout/sync.
| - <pattern 1> | ||
| - <pattern 2> | ||
| forceUpdate: true/false | ||
| updateStrategy: semver/digest/lexical/most-recently-built |
There was a problem hiding this comment.
should most-recently-built match the existing value latest?
Signed-off-by: Jaideep Rao <[email protected]>
…er-first-class-support-proposal
|
@mubarak-j Thanks for taking a look at this! |
Signed-off-by: Jaideep Rao <[email protected]>
…er-first-class-support-proposal
|
|
||
| 2. Change Image updater operational model to become webhook aware - At present the updater polls the configured image registries periodically to detect new available versions of images. In the future Image Updater would need to be able to support webhooks so that it can move to a pub/sub model and updates can be triggered as soon as a qualifying image version is found. | ||
|
|
||
| 3. Supporting container verification - As described earlier, this could be a useful feature to reduce security risks introduced into Argo CD while providing all the benefits of having automatic image updates available. |
There was a problem hiding this comment.
Suggestion: add better support for apps in mono-repo which is currently blocked by argoproj-labs/argocd-image-updater#475 (comment)
I believe mono-repo is a common use case with ArgoCD and this support is crucial for adoption similar to other controllers in ArgoCD.
|
|
||
| This section highlights some of the key aspects of Image Updater's present design constraints and behaviors in order to provide context for the upcoming sections of the proposal to people who may not be familiar with the project. Full details may be found in the Image Updater documentation (https://argocd-image-updater.readthedocs.io/en/stable/) | ||
|
|
||
| - Image Updater can only update container images for applications whose manifests are rendered using either Kustomize or Helm and - especially in the case of Helm - the templates need to support specifying the image's tag (and possibly name) using a parameter (i.e. image.tag). |
There was a problem hiding this comment.
Could it also support setting the image registry via a separate parameter? There are widely used Helm charts in the wild that build the repository name not just from repository and tag but also registry. An example is prometheus-operator.
|
I glanced over the code reviews and most of them are enhancements that do not alter the spirit of the proposal, and they're not backward incompatible.
@jannfis @jaideepr97 I really appreciate your work and I hate for it to become irrelevant/obsolete. |
|
Argo image updater needs support, Can we agree to the above mentioned comment and make a call on roadmap or elect community maintainers to the image updater repo? |
Signed-off-by: Jaideep Rao [email protected]
Note on DCO:
If the DCO action in the integration test fails, one or more of your commits are not signed off. Please click on the Details link next to the DCO action for instructions on how to resolve this.
Checklist: