From ed739d29beacc700bddef50ca9a6eefd7c68de62 Mon Sep 17 00:00:00 2001 From: Sebastian Poehn Date: Wed, 12 Feb 2020 15:46:05 +0100 Subject: [PATCH] Workaround obsolete API versions This is a workaround for changes in API versions like extensions/v1beta1 => apps/v1 Problem: From Helm's perspective those are new Resources as the API version cannot be changed. However there is a compat layer in k8s that will return the old resources for the new API version. Helm will reject the resource 'creation' as there is already a resource of the same kind and version. Solution: In general ignore the Version. Due to the fact that there might be clashes of Kind we track CRD and none-CRD resources seperate. Open issue: Corner case with same Kind in different CRDs and a pre-existing resource that would clash with the one created by Helm. Signed-off-by: Sebastian Poehn --- pkg/action/upgrade.go | 66 +++++++++++++++++++++++++++++--------- pkg/action/upgrade_test.go | 56 ++++++++++++++++++++++++++++++++ 2 files changed, 107 insertions(+), 15 deletions(-) diff --git a/pkg/action/upgrade.go b/pkg/action/upgrade.go index 825920793..7cf07637a 100644 --- a/pkg/action/upgrade.go +++ b/pkg/action/upgrade.go @@ -202,19 +202,7 @@ func (u *Upgrade) performUpgrade(originalRelease, upgradedRelease *release.Relea return upgradedRelease, errors.Wrap(err, "unable to build kubernetes objects from new release manifest") } - // Do a basic diff using gvk + name to figure out what new resources are being created so we can validate they don't already exist - existingResources := make(map[string]bool) - for _, r := range current { - existingResources[objectKey(r)] = true - } - - var toBeCreated kube.ResourceList - for _, r := range target { - if !existingResources[objectKey(r)] { - toBeCreated = append(toBeCreated, r) - } - } - + toBeCreated := detectNewResources(¤t, &target) if err := existingResourceConflict(toBeCreated); err != nil { return nil, errors.Wrap(err, "rendered manifests contain a new resource that already exists. Unable to continue with update") } @@ -425,6 +413,54 @@ func recreate(cfg *Configuration, resources kube.ResourceList) error { } func objectKey(r *resource.Info) string { - gvk := r.Object.GetObjectKind().GroupVersionKind() - return fmt.Sprintf("%s/%s/%s/%s", gvk.GroupVersion().String(), gvk.Kind, r.Namespace, r.Name) + return fmt.Sprintf("%s/%s/%s", objectKind(r), r.Namespace, r.Name) +} + +func detectNewResources(current *kube.ResourceList, target *kube.ResourceList) kube.ResourceList { + existingResources := make(map[string]string) + for _, r := range *current { + existingResources[objectKey(r)] = objectGroupVersion(r) + } + + var toBeCreated kube.ResourceList + for _, r := range *target { + e := existingResources[objectKey(r)] + // Generally we do not care about object versions + // Kubernetes has a compat layer that handles migrations + // However the same Kind might exist in diffent Groups + // In that case we have actually a new Resource + if e == "" || isCRD(e) != isCRD(objectGroupVersion(r)) { + toBeCreated = append(toBeCreated, r) + } + } + + return toBeCreated +} + +func objectGroupVersion(r *resource.Info) string { + return r.Object.GetObjectKind().GroupVersionKind().GroupVersion().String() +} + +func objectKind(r *resource.Info) string { + return r.Object.GetObjectKind().GroupVersionKind().Kind +} + +func isCRD(groupVersion string) bool { + s := strings.Split(groupVersion, "/") + if len(s) == 1 { + // "v1" + return false + } + group := s[0] + s = strings.Split(group, ".") + if len(s) == 1 { + // "apps/v1" + return false + } + if s[len(s)-2] == "k8s" && s[len(s)-1] == "io" { + // "networking.k8s.io/v1" + return false + } + // "crd.projectcalico.org/v1" + return true } diff --git a/pkg/action/upgrade_test.go b/pkg/action/upgrade_test.go index f25d115c4..552af5a97 100644 --- a/pkg/action/upgrade_test.go +++ b/pkg/action/upgrade_test.go @@ -18,9 +18,11 @@ package action import ( "fmt" + "strings" "testing" "helm.sh/helm/v3/pkg/chart" + "helm.sh/helm/v3/pkg/kube" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -253,3 +255,57 @@ func TestUpgradeRelease_ReuseValues(t *testing.T) { is.Equal(expectedValues, updatedRes.Config) }) } + +func TestIsCRD(t *testing.T) { + is := assert.New(t) + is.True(isCRD("crd.projectcalico.org/v1")) + is.True(isCRD("kafka.strimzi.io/v1alpha1")) + is.False(isCRD("v1")) + is.False(isCRD("app/v1")) + is.False(isCRD("banana/v17")) + is.False(isCRD("extensions/v1beta1")) + is.False(isCRD("networking.k8s.io/v1")) +} + +func TestDetectNewResources(t *testing.T) { + t.Skip("This is a live test, comment this line to run") + is := assert.New(t) + c := kube.New(nil) + test1KubernetesNetworkPolicyV1, err := c.Build(strings.NewReader(test1KubernetesNetworkPolicyV1Manifest), false) + is.NoError(err) + test1KubernetesNetworkPolicyV1Beta1, err := c.Build(strings.NewReader(test1KubernetesNetworkPolicyV1Beta1Manifest), false) + is.NoError(err) + test2KubernetesNetworkPolicyV1, err := c.Build(strings.NewReader(test2KubernetesNetworkPolicyV1Manifest), false) + is.NoError(err) + + // Update of resource + is.Nil(detectNewResources(&test1KubernetesNetworkPolicyV1, &test1KubernetesNetworkPolicyV1)) + // Update of resource - API version changed + is.Nil(detectNewResources(&test1KubernetesNetworkPolicyV1Beta1, &test1KubernetesNetworkPolicyV1)) + // New resource was added + is.Equal(test2KubernetesNetworkPolicyV1, detectNewResources(&test1KubernetesNetworkPolicyV1, &test2KubernetesNetworkPolicyV1)) +} + +const test1KubernetesNetworkPolicyV1Manifest = ` +kind: NetworkPolicy +apiVersion: networking.k8s.io/v1 +metadata: + name: test1 +spec: {} +` + +const test1KubernetesNetworkPolicyV1Beta1Manifest = ` +kind: NetworkPolicy +apiVersion: extensions/v1beta1 +metadata: + name: test1 +spec: {} +` + +const test2KubernetesNetworkPolicyV1Manifest = ` +kind: NetworkPolicy +apiVersion: networking.k8s.io/v1 +metadata: + name: test2 +spec: {} +`