Skip to content
This repository has been archived by the owner on Nov 1, 2022. It is now read-only.

Align antecedent annotation in flux and helm-operator #3296

Merged
merged 10 commits into from Mar 16, 2021

Conversation

alex-shpak
Copy link
Contributor

@alex-shpak alex-shpak commented Sep 16, 2020

Hi!

I noticed that Antecedent field in fluxcd API is not showing resource ID that created by HelmOperator. Apparently annotation name is different in flux and helm-operator. This pull request will align them.

const AntecedentAnnotation = "helm.fluxcd.io/antecedent"

https://github.com/fluxcd/helm-operator/blob/master/pkg/apis/helm.fluxcd.io/v1/types_helmrelease.go#L18

@hiddeco hiddeco added this to To do in Running PR backlog Oct 7, 2020
@squaremo
Copy link
Member

Good spot! Would be it sensible to make this backward compatible, do you think? In other words, to look for either the new or the old annotation. Also: can the const in the helm API package be used for the new annotation, or does that cause dependence problems?

@alex-shpak
Copy link
Contributor Author

Hi!
I can make it check both, if needed. Was not sure is that annotation is used at all now. Your call.

Regarding helm-operator dependency. Flux imports RC version of helm-operator
https://github.com/fluxcd/flux/blob/master/go.mod#L12-L17

and AntecedentAnnotation was introduced in 1.2.0. When I update dependency to 1.2.0 it compiles, but I'm not sure is it safe to do.

I will give it a try and see if tests are still passing.

@alex-shpak
Copy link
Contributor Author

alex-shpak commented Nov 27, 2020

Had to force-push to branch for the DCO check.

@alex-shpak
Copy link
Contributor Author

Well, tests has passed.

@squaremo squaremo moved this from To do to In progress in Running PR backlog Nov 30, 2020
@kingdonb
Copy link
Member

I have rebased this and ran go mod tidy on my own fork, should this be merged (or does it somehow represent a breaking change?) My fork at https://github.com/kingdonb/flux/tree/patch-alex-shpak-1 -- this looks like a mostly housekeeping change to me.

@kingdonb
Copy link
Member

kingdonb commented Feb 9, 2021

I didn't understand fully whether this change represents breaking something, or is still needed.

@alex-shpak do you still want to have this included in a forthcoming release of Flux v1?

@alex-shpak
Copy link
Contributor Author

Hi! Yes, would be great to include it in order to be able to connect created workloads to helm release.
There is chance that someone somewhere uses old annotation, but probably pretty low.

I will rebase.

@kingdonb kingdonb added this to the 1.21.3 milestone Feb 9, 2021
@yebyen yebyen mentioned this pull request Feb 10, 2021
Kingdon Barrett and others added 10 commits February 26, 2021 07:22
go.mod - reset k8s machinery to 1.17.17

This is the latest release that we can update to
-- see fluxcd#3378

Try to remove when distribution/distribution#2905 is out

docker/distribution patch still needed for now

(Put it back for now.) On 2021-02-25 this has been merged, 2.7.2 of
docker/distribution should include it hopefully relatively soon!

Signed-off-by: Kingdon Barrett <kingdon@weave.works>
Signed-off-by: Matt Willsher <matt@monki.org.uk>
Signed-off-by: Kingdon Barrett <kingdon@weave.works>
Signed-off-by: Herman Banken <hermanbanken@gmail.com>
Signed-off-by: Kingdon Barrett <kingdon@weave.works>
Signed-off-by: Alaa Qutaish <alaa.qutaish@gmail.com>
Signed-off-by: Kingdon Barrett <kingdon@weave.works>
Signed-off-by: Alex Shpak <alex-shpak@users.noreply.github.com>
Signed-off-by: Kingdon Barrett <kingdon@weave.works>
Signed-off-by: Alex Shpak <alex-shpak@users.noreply.github.com>
Signed-off-by: Kingdon Barrett <kingdon@weave.works>
@kingdonb
Copy link
Member

OK, I have rebased this for inclusion in the next release. It is included in #3442, the Omnibus Branch for 1.22.0.

Copy link
Member

@kingdonb kingdonb left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@alex-shpak
Copy link
Contributor Author

Great!

@kingdonb kingdonb merged commit efba938 into fluxcd:master Mar 16, 2021
@kingdonb kingdonb removed their assignment Mar 17, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants