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

Add support to limit applied policies in automation by specifying a selector #619

Merged

Conversation

Nitive
Copy link
Contributor

@Nitive Nitive commented Dec 21, 2023

Add support for policy selector in ImageUpdateAutomation response

Selector can be specified the same way as Deployment.spec.selector (See kubectl explain Deployment.spec.selector for details)

Example:

apiVersion: image.fluxcd.io/v1alpha1
kind: ImageUpdateAutomation
metadata:
  name: update-teleport
spec:
  policySelector:
    matchLabels:
      app.kubernetes.io/component: teleport
      app.kubernetes.io/instance: teleport
  ...

This allowes to have more flexible image updates, for example, we can batch updates of different components to separate branches. This was previously discussed in #499 and fluxcd/flux2#107

@Nitive Nitive force-pushed the image-update-automation-policy-selector branch 2 times, most recently from ac3f809 to 9f7ce4b Compare December 21, 2023 09:56
@verdel
Copy link

verdel commented Feb 19, 2024

@stefanprodan, if you have the time, I would ask you to review this PR.

@stefanprodan
Copy link
Member

We discussed this proposal at the Flux dev meeting, we decided to include this in the upcoming Flux minor release scheduled for end of April. @darkowlzz will ask you at some point to rebase this on a diffrent branch. Thanks for your patience.

@Nitive
Copy link
Contributor Author

Nitive commented Feb 19, 2024

@stefanprodan thanks!

@darkowlzz
Copy link
Contributor

darkowlzz commented Mar 14, 2024

Hey @Nitive , as mentioned above, the refactored image-automation-controller code is at #647 . It's not fully ready yet but the code where you can add the policy selector and tests associated with it are pretty much ready as of today. You should be able to modify the getPolicies() function to accept the selector and use it in the list call. And there's Test_getPolicies() which you can modify and add tests for it. Once done, you can create a PR against the refactor branch.
In case you're busy at the moment, please let me know. I can adapt the code in this PR to the new branch myself.
We are hoping to release this in April. So please let us know if you'd be able to work on it before then.

@Nitive
Copy link
Contributor Author

Nitive commented Mar 18, 2024

Hey @darkowlzz, I will do it tomorrow

@Nitive Nitive force-pushed the image-update-automation-policy-selector branch from 9f7ce4b to 86e732c Compare March 19, 2024 15:14
@Nitive Nitive changed the base branch from main to refactor March 19, 2024 15:14
@Nitive
Copy link
Contributor Author

Nitive commented Mar 19, 2024

@darkowlzz, it's done. Can you approve workflows please so tests can run?

Copy link
Contributor

@darkowlzz darkowlzz left a comment

Choose a reason for hiding this comment

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

Thanks for updating the code.
Can you also update the PR description as per the current implementation?

In addition to the implementation, can you also update the spec docs for v1beta2 API and document this new field in https://github.com/fluxcd/image-automation-controller/blob/refactor/docs/spec/v1beta2/imageupdateautomations.md#writing-an-imageupdateautomation-spec ?

api/v1beta1/imageupdateautomation_types.go Outdated Show resolved Hide resolved
api/v1beta2/imageupdateautomation_types.go Outdated Show resolved Hide resolved
api/v1beta2/imageupdateautomation_types.go Outdated Show resolved Hide resolved
internal/controller/imageupdateautomation_controller.go Outdated Show resolved Hide resolved
@darkowlzz darkowlzz added enhancement New feature or request area/api API related issues and pull requests labels Mar 19, 2024
@Nitive Nitive force-pushed the image-update-automation-policy-selector branch from 86e732c to b1c34d1 Compare March 20, 2024 08:57
@Nitive
Copy link
Contributor Author

Nitive commented Mar 20, 2024

@darkowlzz, thanks for the detailed feedback! I've applied the changes you suggested

@Nitive
Copy link
Contributor Author

Nitive commented Mar 20, 2024

I'll update the documentation shortly (update: done)

@Nitive Nitive force-pushed the image-update-automation-policy-selector branch from b1c34d1 to 5f0d956 Compare March 20, 2024 09:30
Copy link
Contributor

@darkowlzz darkowlzz left a comment

Choose a reason for hiding this comment

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

The code changes and tests look good to me. Thanks.

Left some minor suggestions. Once they are addressed, I think we are good to merge this.

api/v1beta2/condition_types.go Outdated Show resolved Hide resolved
docs/spec/v1beta2/imageupdateautomations.md Show resolved Hide resolved
internal/controller/imageupdateautomation_controller.go Outdated Show resolved Hide resolved
@Nitive Nitive force-pushed the image-update-automation-policy-selector branch from 5f0d956 to 595e5ae Compare March 21, 2024 09:48
@Nitive
Copy link
Contributor Author

Nitive commented Mar 21, 2024

@darkowlzz, fixed

Copy link
Contributor

@darkowlzz darkowlzz left a comment

Choose a reason for hiding this comment

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

Did some manual testing and it just works.
LGTM!
Thanks.

@darkowlzz darkowlzz merged commit 8982f9b into fluxcd:refactor Mar 21, 2024
3 checks passed
@Nitive Nitive deleted the image-update-automation-policy-selector branch March 27, 2024 13:08
@Nitive Nitive restored the image-update-automation-policy-selector branch March 27, 2024 13:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/api API related issues and pull requests enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants