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

ImageUpdateAutomation enhancements with v1beta2 API #643

Closed
3 tasks done
Tracked by #4712
darkowlzz opened this issue Mar 4, 2024 · 6 comments
Closed
3 tasks done
Tracked by #4712

ImageUpdateAutomation enhancements with v1beta2 API #643

darkowlzz opened this issue Mar 4, 2024 · 6 comments
Labels
enhancement New feature or request umbrella-issue Umbrella issue for tracking progress of a larger effort

Comments

@darkowlzz
Copy link
Contributor

darkowlzz commented Mar 4, 2024

Similar to other Flux controllers, image-automation-controller is undergoing refactoring. The refactor includes overall code organization changes and design enhancements. This issue discusses the enhancements and API changes.

Refactor PR #647.

Update ResultV2

In order to address #437 (inclusion of previous and new value in the update result), the update Result data structure needs to be changed. The current structure is based on file -> objects -> images. This only accounts for updates that involve the full image (name+tag) updates. Partial image updates (name or tag) are not recorded in the result. In order to provide complete details about what was updated, we need to record the complete change regardless of image reference.
A new ResultV2 can be introduced to capture all the kinds of updates and the previous value with the structure file -> objects -> changes. A change constitutes the old value, the new value and the setter responsible for the update. This can be used to construct commit messages like the following with old and new values:

Automation: default/test-update-auto

- File: foo-deployment.yaml
  - Object: Deployment/default/foo
    Changes:
    - 2.2.2 -> 5.0.3
    
- File: podinfo-deployment.yaml
  - Object: Deployment/default/infopod
    Changes:
    - v1.0 -> 5.0.3
  - Object: Deployment/default/podinfo
    Changes:
    - ghcr.io/stefanprodan/podinfo:4.0.6 -> ghcr.io/stefanprodan/podinfo:5.0.3
    - bar -> ghcr.io/stefanprodan/podinfo
    - 4.0.6 -> 5.0.3

In order to keep the current commit templates working as before, TemplateData will be extended by introducing the new field for file change based data.

type TemplateData struct {
        AutomationObject types.NamespacedName
        Updated          update.Result
        Changed          update.ResultV2
}

The new result will be an opt-in feature.

See #642 for the implementation.

ImagePolicy selector support

Currently, all the ImagePolicies present in a namespace are considered when performing an update. Based on a lot of user feedback, policy selectors will be introduced will allow selecting image policies based on their labels via .spec.policySelector field in ImageUpdateAutomation. An implementation of this exists in #619. Along with this, the selected image policies will also be recorded in the status of ImageUpdateAutomation object with their latest images. This will help evaluate and reason about the result of an update, enhancing troubleshooting experience.

apiVersion: image.toolkit.fluxcd.io/v1beta2
kind: ImageUpdateAutomation
metadata:
  name: update-app
spec:
  policySelector:
    matchLabels:
      app.kubernetes.io/component: foo
      app.kubernetes.io/instance: bar
  ...
status:
  observedPolicies:
    policyA:
      name: foo/bar1
      tag: v1.0
    policyB:
      name: foo/bar2
      tag: v1.5
  ...

short-circuit reconciliations

When nothing has changed (git repository commits and the image policies latest images), the reconciliations can be returned early, avoiding full reconciliations. If the latest images of the ImagePolicies have not changed and the remote git repository has no new commit, the full reconciliation operation can be skipped. This will require recording the ImagePolicies and their latest images observed in the last reconciliation, which is described above. And also recording the last observed revision in the git repository. We use similar technique in source-controller to avoid rebuilding the artifact if there's no new commit in the git repository. These can be stored in the status of the ImageUpdateAutomation, .status.observedPolicies and .status.observedSourceRevision.

status:
  ...
  lastPushCommit: ddaa3f4a10ec532c903890c6b51fdf64674df570
  observedPolicies:
    podinfo:
      name: ghcr.io/stefanprodan/podinfo
      tag: 5.0.3
    podinfo-test-1:
      name: ghcr.io/stefanprodan/podinfo
      tag: 4.0.6
    podinfo-test-2:
      name: ghcr.io/stefanprodan/podinfo
      tag: 4.0.0
  observedSourceRevision: main@sha1:ff3f5bd30ba6da07f5f6830eedc6bb62e471f8fb

During a reconciliation, git repository can be checked for new commit and the image policies can be checked for new latest images before performing the full reconciliation.
The observedSourceRevision will follow any updates in the remote git repository and it will diverge from the lastPushCommit until new updates are pushed.

Limitation: When using a different push branch from the checkout branch or even a refspec, it's difficult to detect a drift in the remote branch at present. This short-circuit reconciliation will only apply when the push branch and checkout branch are the same. In the future, we can work on adding capabilities to also check push branch and refspec and extend this capability.

TODO:

  • Introduce ResultV2
  • Introduce policy selector and observed policies
  • short-circuit reconciliations with observed last commit and observed image policies
@darkowlzz darkowlzz added enhancement New feature or request umbrella-issue Umbrella issue for tracking progress of a larger effort labels Mar 4, 2024
@stefanprodan
Copy link
Member

stefanprodan commented Mar 4, 2024

We should take into account the digest feature that will land sometime this year and design IAC with this in mind.

I guess we could store the digest in observedPolicies like this:

status:
  observedPolicies:
  - name: policyA
    latestImage: foo/bar1:v1.0@sha256:<hex>
  - name: policyB
    latestImage: foo/bar2:v1.5@sha256:<hex>

What about ResultV2, how would digest fit in there without having to introduce v3?

@darkowlzz
Copy link
Contributor Author

What about ResultV2, how would digest fit in there without having to introduce v3?

A Change in ResultV2 is string based and has no concept of image, tag or digest. If the update includes a digest, it'll be recorded just as a string.

Considering the previous work in fluxcd/image-reflector-controller#368, for manifest with marker:

      containers:
      - name: c
        image: image:v1.0.0 # {"$imagepolicy": "automation-ns:policy"}

If the ImagePolicy's latest image contains a digest, the update will also include the digest. The ImageRef.String() will help obtain a string of the latest image with digest.

If we introduce a digest setter as in #508, allowing the following setter use case

      containers:
      - name: c
        image: image:v1.0.0 # {"$imagepolicy": "automation-ns:policy:digest"}

The ResultV2 Change will have the old value as image:v1.0.0 and new value as whatever the digest value is.

Results like the following will be supported depending on the commit template and setter:
ghcr.io/stefanprodan/podinfo:4.0.6 -> ghcr.io/stefanprodan/podinfo:5.0.3@sha256:2d9a00b3981628a533ff43352193b1838b0a4bf6b0033444286f563205e51a2c

@darkowlzz
Copy link
Contributor Author

darkowlzz commented Mar 4, 2024

In addition, it would be better to store the image policy result in ImageUpdateAutomation status in the format defined in fluxcd/image-reflector-controller#368. Not a string but as ImageRef with Name and Tag as separate fields. This way IAC will be prepared for the corresponding IRC API change. We can easily append Digest to ImageRef when we add support for it.

status:
  observedPolicies:
  - name: policyA
    latestRef:
      name: foo/bar1
      tag: v1.0
  - name: policyB
    latestRef:
      name: foo/bar2
      tag: v1.5

@stefanprodan
Copy link
Member

If the update includes a digest, it'll be recorded just as a string.

Awesome, so we are all covered here.

it would be better to store the image policy result in ImageUpdateAutomation status in the format defined ImageRef

Yes let's do this!

@darkowlzz
Copy link
Contributor Author

Updated the examples in the issue description to be in the new ImageRef format.

@darkowlzz
Copy link
Contributor Author

Slight change in the observedPolicies format. While implementing this, I realized how it is different from the usual observations from spec that we reflect in the status. In case of reflecting spec value in status, the values don't change their order, in case of a list. But in this case, a list of ImagePolcies is queried from the kubernetes API server, which may not guarantee the order of the policies in the list.
It's better to use a map of policy name and ImageRef instead, making it easier to compare and detect any change in ImageRef for a given policy name.

status:
  ...
  observedPolicies:
    podinfo:
      name: ghcr.io/stefanprodan/podinfo
      tag: 5.0.3
    podinfo-test-1:
      name: ghcr.io/stefanprodan/podinfo
      tag: 4.0.6
    podinfo-test-2:
      name: ghcr.io/stefanprodan/podinfo
      tag: 4.0.0
  ...

Will update the example in the issue description accordingly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request umbrella-issue Umbrella issue for tracking progress of a larger effort
Projects
None yet
Development

No branches or pull requests

2 participants