[WIP/Feedback needed] Capture more attributes to create multiple deployment #211

GregoireW · 2021-12-05T19:34:44Z

This PR add way to get more data from the image tag and add a way to get multiple tag selected via a discriminator. This open a way to manage ephemeral environment for each PR for instance.

Filter tag can be describe like:

  filterTags:
    attributes:
    - $sha1
    discriminator: $pr
    extract: $ts
    pattern: ^pr-(?P<pr>.*)-(?P<ts>\d*)-(?P<sha1>.*)$

Which means if you have tags like

- pr-123-202112031125-ae5fde12
- pr-123-202112041743-e15de12a
- pr-124-202112051110-15fde2ae
- pr-124-202112051625-5fde2b34

this will translate to a status:

status:
  distribution:
    "123":
      attributes:
        pr: "123"
        sha1: e15de12a
        ts: "202112041743"
      image: img-demo/demo
      tag: pr-123-202112041743-e15de12a
    "124":
      attributes:
        pr: "124"
        sha1: 5fde2b34
        ts: "202112051625"
      image: img-demo/demo
      tag: pr-124-202112051625-5fde2b34
  latestDiscriminator: "123"
  latestImage: img-demo/demo:pr-123-202112041743-e15de12a
  nbDistribution: 2

I added some additional attributes to be able to get more data on the generated object like:

metadata:
  labels:
    pr: "1245" # {"$imagepolicy": "flux-system:pr", "template": "{{.attributes.pr}}" }
    sha1: "5fde2b34" # {"$imagepolicy": "flux-system:pr", "template": "{{.attributes.sha1}}" }

For this to work, there is some change to do on the image-automation-controller, which is done here: fluxcd/image-automation-controller#274

Signed-off-by: GregoireW <24318548+GregoireW@users.noreply.github.com>

relu

Hello @GregoireW and thank you for contributing to Flux! I've left some comments on your proposed changes and would like to debate a bit more on this matter before we can go forward with this.

The attributes/parts extraction makes sense to me and I see it is a feature requested just recently in #212 as well.

relu · 2021-12-07T11:06:24Z

api/v1beta2/imagepolicy_types.go

+	// policy against each of those groups. This value if present is a capture
+	// group to be extracted from the regular expression pattern.
+	// +optional
+	Discriminator string `json:"discriminator,omitempty"`


I'm not quite sure I understand the need for a discriminator. For this use case, it is recommended to just define a different image policy, scoped to the identifier (PR id in the prefix in your documented use case). e.g. ^pr-123-(?P<ts>\d*)-(?P<sha1>.*)$.

Based on your description, you want to create ephemeral environments for each pull request, so why not just create an image policy resource as part of the automation that would build that in the first place? Having the discriminator doesn't really help as you still need to somehow tell the IUA which discriminator value you want to use for the respective setter marker.

Thing is I do not want to use the setter policy, I want to have ephemeral environment, so if I have 3 open PR, I want to have 3 environments. Setter policy is just a setter on yaml file, it cannot help me there as it would create a single environment.
If I want to create 3 environments I would have to create 3 policies with associated kubernetes objects but then I feel it contradict the gitops approach as the CI would have to create deployment objet (and then why use flux to automate what is already done by the CI)

It is why there is another PR on the automation controller to be able to use one policy and duplicate some objects.

The goal would be to to be able to create a docker image on the CI (and delete some when PR is closed) and then flux on its own would be able to trigger what need to be triggered.

In general, changes to code can be accompanied by changes to configuration. For example, if you add a feature to your app, you might have some new argument that needs to be supplied to the image; or, to have a feature flag set, to enable the feature. For this reason, I think of preview environments as being driven by changes to configs (YAMLs), rather than new images; i.e., automation belongs outside the rather restricted view of the image reflector.

relu · 2021-12-07T11:21:42Z

api/v1beta2/imagepolicy_types.go

+	Distribution map[string]ImageAndAttributes `json:"distribution,omitempty"`
+	// Discriminator of the latest image.
+	// +optional
+	LatestDiscriminator string  `json:"latestDiscriminator,omitempty"`


This is where I see a contradiction. If you actually have the image policy keep track of the value for what the latest discriminator would be then why would this even exist in the first place if you actually want to share this policy across multiple discriminator scopes? This is pretty much equivalent to an image policy definition like this:

filterTags: extract: $pr$ts pattern: ^pr-(?P<pr>.*)-(?P<ts>\d*)-.*$

Which would generate a number such as 124202112051625 which can be compared, but doesn't really help in your situation if you want to scope it to the PR identifier since it will always return whatever the latest PR + timestamp values combination would be.

The latest image is kept to be compatible with the setter policy, the latest discriminator is kept for a more practical reason.

As mention before, I created a "duplication" policy on the automation controller. So from 1 kubernetes object (which have a comment using a policy), I want to create multiple object, 1 per discriminator value.

But what with the original object ? is this something we can hide ? is this something we want to deploy ? ...

I chose to kep the last distriminator value only on this purpose: the original object is set to the latest image ( with the sorted value) so I need to keep the latest discriminator to be able to find attributes.

relu · 2021-12-07T11:58:14Z

api/v1beta2/imagepolicy_types.go

+	Discriminator string `json:"discriminator,omitempty"`
+	// Additional attributes we want to extract from the tag.
+	// +optional
+	Attributes []string `json:"attributes,omitempty"`


I kind of like this idea of having some ability to extract attribute parts from the tag value. What I'm not quite sure about is if this should actually be the responsibility of the image policy or if it should be something implemented at the level of the image automation controller.

Thinking on easiest way to use this,

Image automation is a global thing (process multiple policy) so configuration to extract data cannot be set on the image configuration itself
It may be possible to use additional parameter on the json to apply a policy, but then we may need to duplicate regexp which may be not so easy to use.

stefanprodan · 2021-12-07T14:47:31Z

Hi @GregoireW such a feature request has to follow the RFC process described here: https://github.com/fluxcd/flux2/tree/main/rfcs

PS. I don't think container images are enough to create preview environments, as in most cases a new app version is accompanied by config changes. Please see fluxcd/flux2#831 (comment)

ljakimczuk · 2021-12-08T10:31:59Z

@GregoireW We're also interested in having this feature. Let me know if I can support you on the RFC you in any way.

GregoireW · 2021-12-08T13:04:14Z

@GregoireW We're also interested in having this feature. Let me know if I can support you on the RFC you in any way.

I will refactor this PR to only extract additional attributes, I think I will update the crd to have:

  filterTags:
    attributes:
    - $sha1
    - $pr
    extract: $ts
    pattern: ^pr-(?P<pr>.*)-(?P<ts>\d*)-(?P<sha1>.*)$

Which will end up if scanning an image: img-demo/demo:pr-123-202112041743-e15de12a

status:
  attributes:
    pr: "123"
    sha1: e15de12a
    ts: "202112041743"
  latestImage: img-demo/demo:pr-123-202112041743-e15de12a

I will also open an issue on automation controller to discuss how we can use those attributes. ( accept template, do like today )

GregoireW · 2021-12-13T13:35:18Z

Will open a discussion about this.

relu · 2022-01-19T12:38:21Z

RFC: fluxcd/flux2#2222

GregoireW mentioned this pull request Dec 5, 2021

[WIP/Feedback needed] Add a duplicate strategy to deploy ephemeral environment fluxcd/image-automation-controller#274

Closed

GregoireW added 2 commits December 5, 2021 20:43

Add discriminator to manage ephemeral environment

859821b

Signed-off-by: GregoireW <24318548+GregoireW@users.noreply.github.com>

Add latest discriminator in status

0d202dc

Signed-off-by: GregoireW <24318548+GregoireW@users.noreply.github.com>

GregoireW force-pushed the feature/discriminator branch from 713c518 to 0d202dc Compare December 5, 2021 19:55

relu requested changes Dec 7, 2021

View reviewed changes

relu mentioned this pull request Dec 7, 2021

ImagePolicy .spec.filterTags.extract field question #212

Open

relu added the blocked/requires-rfc Requires a design proposal label Dec 7, 2021

GregoireW mentioned this pull request Dec 8, 2021

Substitutions for setters fluxcd/image-automation-controller#97

Open

GregoireW closed this Dec 13, 2021

GregoireW deleted the feature/discriminator branch December 13, 2021 15:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP/Feedback needed] Capture more attributes to create multiple deployment #211

[WIP/Feedback needed] Capture more attributes to create multiple deployment #211

GregoireW commented Dec 5, 2021 •

edited

relu left a comment

relu Dec 7, 2021

GregoireW Dec 7, 2021

squaremo Dec 7, 2021

relu Dec 7, 2021

GregoireW Dec 7, 2021

relu Dec 7, 2021

GregoireW Dec 7, 2021

stefanprodan commented Dec 7, 2021

ljakimczuk commented Dec 8, 2021 •

edited

GregoireW commented Dec 8, 2021

GregoireW commented Dec 13, 2021

relu commented Jan 19, 2022

[WIP/Feedback needed] Capture more attributes to create multiple deployment #211

[WIP/Feedback needed] Capture more attributes to create multiple deployment #211

Conversation

GregoireW commented Dec 5, 2021 • edited

relu left a comment

Choose a reason for hiding this comment

relu Dec 7, 2021

Choose a reason for hiding this comment

GregoireW Dec 7, 2021

Choose a reason for hiding this comment

squaremo Dec 7, 2021

Choose a reason for hiding this comment

relu Dec 7, 2021

Choose a reason for hiding this comment

GregoireW Dec 7, 2021

Choose a reason for hiding this comment

relu Dec 7, 2021

Choose a reason for hiding this comment

GregoireW Dec 7, 2021

Choose a reason for hiding this comment

stefanprodan commented Dec 7, 2021

ljakimczuk commented Dec 8, 2021 • edited

GregoireW commented Dec 8, 2021

GregoireW commented Dec 13, 2021

relu commented Jan 19, 2022

GregoireW commented Dec 5, 2021 •

edited

ljakimczuk commented Dec 8, 2021 •

edited