-
Notifications
You must be signed in to change notification settings - Fork 187
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
Move the GC checksum from labels to annotations #362
Conversation
@JaneLiuL I found some serious issues with |
controllers/utils.go
Outdated
// --- /tmp/LIVE-656588208/kustomize.toolkit.fluxcd.io.v1beta1.Kustomization.namespace.name 2021-06-07 12:58:20.738794982 +0200 | ||
// +++ /tmp/MERGED-532750671/kustomize.toolkit.fluxcd.io.v1beta1.Kustomization.namespace.name 2021-06-07 12:58:20.798795908 +0200 | ||
// @@ -0,0 +1,36 @@ | ||
func parseDiffOutput(in []byte) map[string]string { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove this as it's unrelated to this PR.
@@ -21,12 +21,10 @@ import ( | |||
"crypto/sha1" | |||
"encoding/json" | |||
"fmt" | |||
"github.com/fluxcd/pkg/apis/kustomize" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This needs to be added further down, the first import section is for Go std lib.
} | ||
} | ||
if !exists { | ||
//kus.Transformers = append(kus.Transformers, transformerFileName) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove these commented lines.
@@ -648,6 +649,9 @@ func (r *KustomizationReconciler) apply(ctx context.Context, kustomization kusto | |||
} | |||
} | |||
|
|||
changeSet := "" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change is unrelated to this PR please undo it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please update the GC docs here https://github.com/fluxcd/kustomize-controller/blob/main/docs/spec/v1beta1/kustomization.md#garbage-collection
controllers/kustomization_gc.go
Outdated
@@ -137,9 +137,25 @@ func (kgc *KustomizeGarbageCollector) Prune(timeout time.Duration, name string, | |||
return changeSet, true | |||
} | |||
|
|||
// We can't drop the label check in this version. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please replace this with:
Check both labels and annotations for the checksum to preserve backwards compatibility
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix done~
func (kg *KustomizeGenerator) generateLabelTransformer(checksum, dirPath string) error { | ||
labels := selectorLabels(kg.kustomization.GetName(), kg.kustomization.GetNamespace()) | ||
|
||
// add checksum label only if GC is enabled |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This has been lost, please restore it for the annotions
return err | ||
} | ||
|
||
labelsFile := filepath.Join(dirPath, transformerAnnotationFileName) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
labelsFile := filepath.Join(dirPath, transformerAnnotationFileName) | |
annotationsFile := filepath.Join(dirPath, transformerAnnotationFileName) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix done~
) | ||
|
||
const ( | ||
transformerFileName = "kustomization-gc-labels.yaml" | ||
transformerFileName = "kustomization-gc-labels.yaml" | ||
transformerAnnotationFileName = "kustomization-annotation-labels.yaml" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
transformerAnnotationFileName = "kustomization-annotation-labels.yaml" | |
transformerAnnotationFileName = "kustomization-gc-annotations.yaml" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix done~
Signed-off-by: Jane Liu L <jane.l.liu@ericsson.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR moves the
kustomize.toolkit.fluxcd.io/checksum
from labels to annotations. This should fix any conflicts with 3rd party controllers like Stash (fix: #315) that are copying the labels from their custom resources to generated objects.