v3 backport - fix --take-ownership#30768
Merged
scottrigby merged 2 commits intohelm:dev-v3from Apr 21, 2025
Merged
Conversation
If a resource exists in the cluster and is to be adopted by helm install --take-ownership, it is left unchanged while helm reports the installation to have succeeded. This is due to CRs and CRDs being merged without three-way-merge, which results in an empty patch. By using a three-way-merge transparently when --take-ownership is used, the helm behaves as expected without breaking previous behavior. Fixes helm#30622 Signed-off-by: Patrick Seidensal <pseidensal@suse.com>
Signed-off-by: Evans Mungai <mbuevans@gmail.com>
018c294 to
0906fe7
Compare
scottrigby
approved these changes
Apr 17, 2025
Member
There was a problem hiding this comment.
diff looks good (between v4 and v3)
diff -y <(gh pr diff 30697) <(gh pr diff 30768)
Manual test with steps from #30622 also works as well. Exact commands if anyone wants to follow along:
-
create CRD
kubectl apply -f - <<EOF apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition metadata: name: datas.crd.com spec: group: crd.com scope: Namespaced names: plural: datas singular: data kind: Data versions: - name: v1 served: true storage: true schema: openAPIV3Schema: type: object x-kubernetes-preserve-unknown-fields: true EOF
-
apply a CR (removed ns from original example)
kubectl apply -f - <<EOF kind: Data apiVersion: crd.com/v1 metadata: name: crd-test labels: test: adopt spec: color: blue size: large EOF
-
build helm binary from this PR
binary will be in ./bin/helm
gh pr checkout 30768 make clean build
-
create and install chart containing a different version of that same CR
name=pr-30768 mkdir -p $name/templates cat > $name/Chart.yaml <<EOF apiVersion: v2 name: $name version: 0.1.0 EOF cat > $name/templates/test.yaml <<EOF kind: Data apiVersion: crd.com/v1 metadata: name: crd-test labels: test: adopt spec: weight: heavy EOF ./bin/helm install pr-30768 pr-30768/ --take-ownership
-
ensure generation has incremented and spec values have been merged
kubectl get datas.crd.com crd-test -oyaml
you should see
metadata: generation: 2
and
spec: color: blue size: large weight: heavy
sabre1041
approved these changes
Apr 20, 2025
Contributor
sabre1041
left a comment
There was a problem hiding this comment.
LGTM. Confirmed same results with the instructions/steps provided by @scottrigby
1 task
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
v3 back port for #30697