From baa199b094a10165b1d05b28bbccf87bcb6cce70 Mon Sep 17 00:00:00 2001 From: Michael Burman Date: Fri, 29 Nov 2024 04:28:32 -0800 Subject: [PATCH] Remove old CRD stored versions when upgrading (#71) * If upgrading CRD has different versions than the ones stored on the server, remove the previously stored versions as stored versions * Support older github workflow for now which is missing chartVersion from the pkg download --- Makefile | 6 +- pkg/helmutil/crds.go | 50 +++++- pkg/helmutil/crds_test.go | 158 ++++++++++++++++++ ...multiversion-clientconfig-mockup-both.yaml | 103 ++++++++++++ ...iversion-clientconfig-mockup-v1alpha1.yaml | 57 +++++++ ...tiversion-clientconfig-mockup-v1beta1.yaml | 61 +++++++ 6 files changed, 431 insertions(+), 4 deletions(-) create mode 100644 testfiles/crd-upgrader/multiversion-clientconfig-mockup-both.yaml create mode 100644 testfiles/crd-upgrader/multiversion-clientconfig-mockup-v1alpha1.yaml create mode 100644 testfiles/crd-upgrader/multiversion-clientconfig-mockup-v1beta1.yaml diff --git a/Makefile b/Makefile index 837fb88..aee807b 100644 --- a/Makefile +++ b/Makefile @@ -1,4 +1,4 @@ -VERSION ?= 0.3.0 +VERSION ?= 0.7.0 COMMIT := $(shell git rev-parse --short HEAD) DATE := $(shell date +%Y%m%d) @@ -14,7 +14,7 @@ SHELL = /usr/bin/env bash -o pipefail .SHELLFLAGS = -ec # ENVTEST_K8S_VERSION refers to the version of kubebuilder assets to be downloaded by envtest binary. -ENVTEST_K8S_VERSION = 1.28.x +ENVTEST_K8S_VERSION = 1.31.x GO_FLAGS ?= -v @@ -83,7 +83,7 @@ $(LOCALBIN): GOLANGCI_LINT ?= $(LOCALBIN)/golangci-lint ENVTEST ?= $(LOCALBIN)/setup-envtest -GOLINT_VERSION ?= 1.56.2 +GOLINT_VERSION ?= 1.61.0 .PHONY: envtest envtest: $(ENVTEST) ## Download envtest-setup locally if necessary. diff --git a/pkg/helmutil/crds.go b/pkg/helmutil/crds.go index 8f6e937..4c79c05 100644 --- a/pkg/helmutil/crds.go +++ b/pkg/helmutil/crds.go @@ -8,12 +8,15 @@ import ( "io" "os" "path/filepath" + "slices" "strings" "github.com/charmbracelet/log" "github.com/pkg/errors" + apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime" deser "k8s.io/apimachinery/pkg/runtime/serializer/yaml" k8syaml "k8s.io/apimachinery/pkg/util/yaml" @@ -54,7 +57,7 @@ func (u *Upgrader) Upgrade(ctx context.Context, chartVersion string) ([]unstruct } if fs, err := os.Stat(chartDir); os.IsNotExist(err) { - log.Info("Downloading chart release from remote repository", "repoURL", u.repoURL, "chartName", u.chartName, "chartVersion", chartVersion) + log.Info("Downloading chart release from remote repository", "repoURL", u.repoURL, "chartName", u.chartName, "chartVersion", chartVersion, "chartDir", chartDir) downloadDir, err := DownloadChartRelease(u.repoName, u.repoURL, u.chartName, chartVersion) if err != nil { return nil, err @@ -102,6 +105,51 @@ func (u *Upgrader) Upgrade(ctx context.Context, chartVersion string) ([]unstruct } else { log.Debug("Updating CustomResourceDefinition", "name", obj.GetName()) obj.SetResourceVersion(existingCrd.GetResourceVersion()) + + // TODO We need to check which versions we have available here before updating + unstructured := obj.UnstructuredContent() + var definition apiextensionsv1.CustomResourceDefinition + if err := runtime.DefaultUnstructuredConverter.FromUnstructured(unstructured, &definition); err != nil { + return nil, errors.Wrapf(err, "failed to convert unstructured to CustomResourceDefinition %s", obj.GetName()) + } + + updatedVersions := make([]string, 0, len(definition.Spec.Versions)) + for _, version := range definition.Spec.Versions { + updatedVersions = append(updatedVersions, version.Name) + } + log.Debug("Read CustomResourceDefinition versions", "name", obj.GetName(), "versions", updatedVersions) + + existing := existingCrd.UnstructuredContent() + var existingDefinition apiextensionsv1.CustomResourceDefinition + if err := runtime.DefaultUnstructuredConverter.FromUnstructured(existing, &existingDefinition); err != nil { + return nil, errors.Wrapf(err, "failed to convert unstructured to CustomResourceDefinition %s", obj.GetName()) + } + + storedVersions := existingDefinition.Status.StoredVersions + + if !slices.Equal(storedVersions, updatedVersions) { + + // Check if storedVersion has any versions that are not in updatedVersions + // If so, we need to remove them from the storedVersions + removed := false + for _, storedVersion := range storedVersions { + if !slices.Contains(updatedVersions, storedVersion) { + log.Debug("Removing CustomResourceDefinition version", "name", obj.GetName(), "version", storedVersion) + // storedVersions = slices.DeleteFunc(storedVersions, func(e string) bool { return e == storedVersion }) + removed = true + } + } + + if removed { + log.Debug("Updating CustomResourceDefinition versions", "name", obj.GetName(), "storedVersions", storedVersions, "updatedVersions", updatedVersions) + existingDefinition.Status.StoredVersions = updatedVersions + if err := u.client.Status().Update(ctx, &existingDefinition); err != nil { + return nil, errors.Wrapf(err, "failed to update CRD storedVersions %s", obj.GetName()) + } + obj.SetResourceVersion(existingDefinition.GetResourceVersion()) + } + } + if err = u.client.Update(ctx, &obj); err != nil { return nil, errors.Wrapf(err, "failed to update CRD %s", obj.GetName()) } diff --git a/pkg/helmutil/crds_test.go b/pkg/helmutil/crds_test.go index 9519866..b5c88b2 100644 --- a/pkg/helmutil/crds_test.go +++ b/pkg/helmutil/crds_test.go @@ -2,12 +2,17 @@ package helmutil_test import ( "context" + "fmt" + "io" "os" + "path/filepath" "strings" "testing" "time" "github.com/k8ssandra/k8ssandra-client/pkg/helmutil" + "github.com/k8ssandra/k8ssandra-client/pkg/util" + "github.com/pkg/errors" "github.com/stretchr/testify/require" apiextensions "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" "k8s.io/apimachinery/pkg/runtime" @@ -85,3 +90,156 @@ func cleanCache(repoName, chartName string) error { return os.RemoveAll(chartDir) } + +func TestUpgradingStoredVersions(t *testing.T) { + if testing.Short() { + t.Skip("skipping test in short mode.") + } + + require := require.New(t) + chartName := "test-chart" + namespace := env.CreateNamespace(t) + kubeClient := env.GetClientInNamespace(namespace) + require.NoError(cleanCache("k8ssandra", chartName)) + + // Copy testfiles + chartDir, err := helmutil.GetChartTargetDir(helmutil.K8ssandraRepoName, chartName) + require.NoError(err) + + crdDir := filepath.Join(chartDir, "crds") + _, err = util.CreateIfNotExistsDir(crdDir) + require.NoError(err) + crdSrc := filepath.Join("..", "..", "testfiles", "crd-upgrader", "multiversion-clientconfig-mockup-v1alpha1.yaml") + require.NoError(copyFile(crdSrc, filepath.Join(crdDir, "clientconfig.yaml"))) + + testOptions := envtest.CRDInstallOptions{ + PollInterval: 100 * time.Millisecond, + MaxTime: 10 * time.Second, + } + + // creating new upgrader + u, err := helmutil.NewUpgrader(kubeClient, helmutil.K8ssandraRepoName, helmutil.StableK8ssandraRepoURL, chartName, []string{}) + require.NoError(err) + + crds, err := u.Upgrade(context.TODO(), "0.1.0") + require.NoError(err) + + targetCrd := &apiextensions.CustomResourceDefinition{} + objs := []*apiextensions.CustomResourceDefinition{} + for _, crd := range crds { + err = runtime.DefaultUnstructuredConverter.FromUnstructured(crd.UnstructuredContent(), targetCrd) + require.NoError(err) + objs = append(objs, targetCrd) + } + + require.NotEmpty(objs) + require.NotEmpty(targetCrd.GetName()) + require.NoError(envtest.WaitForCRDs(env.RestConfig(), objs, testOptions)) + require.NoError(kubeClient.Get(context.TODO(), client.ObjectKey{Name: targetCrd.GetName()}, targetCrd)) + + require.Equal([]string{"v1alpha1"}, targetCrd.Status.StoredVersions) + + // Upgrade to 0.2.0 + + require.NoError(cleanCache("k8ssandra", chartName)) + _, err = util.CreateIfNotExistsDir(crdDir) + require.NoError(err) + crdSrc = filepath.Join("..", "..", "testfiles", "crd-upgrader", "multiversion-clientconfig-mockup-both.yaml") + require.NoError(copyFile(crdSrc, filepath.Join(crdDir, "clientconfig.yaml"))) + + crds, err = u.Upgrade(context.TODO(), "0.2.0") + require.NoError(err) + for _, crd := range crds { + err = runtime.DefaultUnstructuredConverter.FromUnstructured(crd.UnstructuredContent(), targetCrd) + require.NoError(err) + objs = append(objs, targetCrd) + } + require.NotEmpty(objs) + require.NoError(envtest.WaitForCRDs(env.RestConfig(), objs, testOptions)) + require.NoError(kubeClient.Get(context.TODO(), client.ObjectKey{Name: targetCrd.GetName()}, targetCrd)) + require.Equal([]string{"v1alpha1", "v1beta1"}, targetCrd.Status.StoredVersions) + + // Upgrade to 0.3.0 + + require.NoError(cleanCache("k8ssandra", chartName)) + _, err = util.CreateIfNotExistsDir(crdDir) + require.NoError(err) + crdSrc = filepath.Join("..", "..", "testfiles", "crd-upgrader", "multiversion-clientconfig-mockup-v1beta1.yaml") + require.NoError(copyFile(crdSrc, filepath.Join(crdDir, "clientconfig.yaml"))) + + crds, err = u.Upgrade(context.TODO(), "0.3.0") + require.NoError(err) + for _, crd := range crds { + err = runtime.DefaultUnstructuredConverter.FromUnstructured(crd.UnstructuredContent(), targetCrd) + require.NoError(err) + objs = append(objs, targetCrd) + } + require.NotEmpty(objs) + require.NoError(envtest.WaitForCRDs(env.RestConfig(), objs, testOptions)) + require.NoError(kubeClient.Get(context.TODO(), client.ObjectKey{Name: targetCrd.GetName()}, targetCrd)) + require.Equal([]string{"v1beta1"}, targetCrd.Status.StoredVersions) + + // Sanity check, install 0.2.0 and only update to 0.3.0 (there should be no storedVersion of v1alpha1) + require.NoError(kubeClient.Delete(context.TODO(), targetCrd)) + require.Eventually(func() bool { + err = kubeClient.Get(context.TODO(), client.ObjectKey{Name: targetCrd.GetName()}, targetCrd) + return err != nil && client.IgnoreNotFound(err) == nil + }, time.Second*5, time.Millisecond*100) + + // Install 0.2.0 + + require.NoError(cleanCache("k8ssandra", chartName)) + _, err = util.CreateIfNotExistsDir(crdDir) + require.NoError(err) + crdSrc = filepath.Join("..", "..", "testfiles", "crd-upgrader", "multiversion-clientconfig-mockup-both.yaml") + require.NoError(copyFile(crdSrc, filepath.Join(crdDir, "clientconfig.yaml"))) + + crds, err = u.Upgrade(context.TODO(), "0.2.0") + require.NoError(err) + for _, crd := range crds { + err = runtime.DefaultUnstructuredConverter.FromUnstructured(crd.UnstructuredContent(), targetCrd) + require.NoError(err) + objs = append(objs, targetCrd) + } + require.NotEmpty(objs) + require.NoError(envtest.WaitForCRDs(env.RestConfig(), objs, testOptions)) + require.NoError(kubeClient.Get(context.TODO(), client.ObjectKey{Name: targetCrd.GetName()}, targetCrd)) + require.Equal([]string{"v1beta1"}, targetCrd.Status.StoredVersions) + + // Upgrade to 0.3.0 + + require.NoError(cleanCache("k8ssandra", chartName)) + _, err = util.CreateIfNotExistsDir(crdDir) + require.NoError(err) + crdSrc = filepath.Join("..", "..", "testfiles", "crd-upgrader", "multiversion-clientconfig-mockup-v1beta1.yaml") + require.NoError(copyFile(crdSrc, filepath.Join(crdDir, "clientconfig.yaml"))) + + crds, err = u.Upgrade(context.TODO(), "0.3.0") + require.NoError(err) + for _, crd := range crds { + err = runtime.DefaultUnstructuredConverter.FromUnstructured(crd.UnstructuredContent(), targetCrd) + require.NoError(err) + objs = append(objs, targetCrd) + } + require.NotEmpty(objs) + require.NoError(envtest.WaitForCRDs(env.RestConfig(), objs, testOptions)) + require.NoError(kubeClient.Get(context.TODO(), client.ObjectKey{Name: targetCrd.GetName()}, targetCrd)) + require.Equal([]string{"v1beta1"}, targetCrd.Status.StoredVersions) +} + +func copyFile(source, target string) error { + src, err := os.Open(source) + if err != nil { + return errors.Wrap(err, fmt.Sprintf("failed to open %s", source)) + } + defer src.Close() + + dst, err := os.Create(target) + if err != nil { + return errors.Wrap(err, fmt.Sprintf("failed to open %s", target)) + } + defer dst.Close() + + _, err = io.Copy(dst, src) + return err +} diff --git a/testfiles/crd-upgrader/multiversion-clientconfig-mockup-both.yaml b/testfiles/crd-upgrader/multiversion-clientconfig-mockup-both.yaml new file mode 100644 index 0000000..918c12d --- /dev/null +++ b/testfiles/crd-upgrader/multiversion-clientconfig-mockup-both.yaml @@ -0,0 +1,103 @@ +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + annotations: + controller-gen.kubebuilder.io/version: v0.14.0 + name: clientconfigmocks.config.k8ssandra.io +spec: + group: config.k8ssandra.io + names: + kind: ClientConfigMock + listKind: ClientConfigMockList + plural: clientconfigmocks + singular: clientconfigmock + scope: Namespaced + versions: + - name: v1alpha1 + schema: + openAPIV3Schema: + description: ClientConfig is the Schema for the kubeconfigs API + properties: + apiVersion: + description: |- + APIVersion defines the versioned schema of this representation of an object. + Servers should convert recognized schemas to the latest internal value, and + may reject unrecognized values. + More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources + type: string + kind: + description: |- + Kind is a string value representing the REST resource this object represents. + Servers may infer this from the endpoint the client submits requests to. + Cannot be updated. + In CamelCase. + More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds + type: string + metadata: + type: object + spec: + description: ClientConfigSpec defines the desired state of KubeConfig + properties: + kubeConfigSecret: + description: |- + KubeConfigSecret should reference an existing secret; the actual configuration will be read from + this secret's "kubeconfig" key. + properties: + name: + description: |- + Name of the referent. + More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names + TODO: Add other useful fields. apiVersion, kind, uid? + type: string + type: object + x-kubernetes-map-type: atomic + type: object + type: object + served: true + storage: false + - name: v1beta1 + schema: + openAPIV3Schema: + description: ClientConfig is the Schema for the kubeconfigs API + properties: + apiVersion: + description: |- + APIVersion defines the versioned schema of this representation of an object. + Servers should convert recognized schemas to the latest internal value, and + may reject unrecognized values. + More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources + type: string + kind: + description: |- + Kind is a string value representing the REST resource this object represents. + Servers may infer this from the endpoint the client submits requests to. + Cannot be updated. + In CamelCase. + More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds + type: string + metadata: + type: object + spec: + description: ClientConfigSpec defines the desired state of KubeConfig + properties: + contextName: + description: ContextName allows to override the object name for context-name. + If not set, the ClientConfig.Name is used as context name + type: string + kubeConfigSecret: + description: |- + KubeConfigSecret should reference an existing secret; the actual configuration will be read from + this secret's "kubeconfig" key. + properties: + name: + description: |- + Name of the referent. + More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names + TODO: Add other useful fields. apiVersion, kind, uid? + type: string + type: object + x-kubernetes-map-type: atomic + type: object + type: object + served: true + storage: true diff --git a/testfiles/crd-upgrader/multiversion-clientconfig-mockup-v1alpha1.yaml b/testfiles/crd-upgrader/multiversion-clientconfig-mockup-v1alpha1.yaml new file mode 100644 index 0000000..af6d3ca --- /dev/null +++ b/testfiles/crd-upgrader/multiversion-clientconfig-mockup-v1alpha1.yaml @@ -0,0 +1,57 @@ +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + annotations: + controller-gen.kubebuilder.io/version: v0.14.0 + name: clientconfigmocks.config.k8ssandra.io +spec: + group: config.k8ssandra.io + names: + kind: ClientConfigMock + listKind: ClientConfigMockList + plural: clientconfigmocks + singular: clientconfigmock + scope: Namespaced + versions: + - name: v1alpha1 + schema: + openAPIV3Schema: + description: ClientConfig is the Schema for the kubeconfigs API + properties: + apiVersion: + description: |- + APIVersion defines the versioned schema of this representation of an object. + Servers should convert recognized schemas to the latest internal value, and + may reject unrecognized values. + More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources + type: string + kind: + description: |- + Kind is a string value representing the REST resource this object represents. + Servers may infer this from the endpoint the client submits requests to. + Cannot be updated. + In CamelCase. + More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds + type: string + metadata: + type: object + spec: + description: ClientConfigSpec defines the desired state of KubeConfig + properties: + kubeConfigSecret: + description: |- + KubeConfigSecret should reference an existing secret; the actual configuration will be read from + this secret's "kubeconfig" key. + properties: + name: + description: |- + Name of the referent. + More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names + TODO: Add other useful fields. apiVersion, kind, uid? + type: string + type: object + x-kubernetes-map-type: atomic + type: object + type: object + served: true + storage: true diff --git a/testfiles/crd-upgrader/multiversion-clientconfig-mockup-v1beta1.yaml b/testfiles/crd-upgrader/multiversion-clientconfig-mockup-v1beta1.yaml new file mode 100644 index 0000000..d08253e --- /dev/null +++ b/testfiles/crd-upgrader/multiversion-clientconfig-mockup-v1beta1.yaml @@ -0,0 +1,61 @@ +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + annotations: + controller-gen.kubebuilder.io/version: v0.14.0 + name: clientconfigmocks.config.k8ssandra.io +spec: + group: config.k8ssandra.io + names: + kind: ClientConfigMock + listKind: ClientConfigMockList + plural: clientconfigmocks + singular: clientconfigmock + scope: Namespaced + versions: + - name: v1beta1 + schema: + openAPIV3Schema: + description: ClientConfig is the Schema for the kubeconfigs API + properties: + apiVersion: + description: |- + APIVersion defines the versioned schema of this representation of an object. + Servers should convert recognized schemas to the latest internal value, and + may reject unrecognized values. + More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources + type: string + kind: + description: |- + Kind is a string value representing the REST resource this object represents. + Servers may infer this from the endpoint the client submits requests to. + Cannot be updated. + In CamelCase. + More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds + type: string + metadata: + type: object + spec: + description: ClientConfigSpec defines the desired state of KubeConfig + properties: + contextName: + description: ContextName allows to override the object name for context-name. + If not set, the ClientConfig.Name is used as context name + type: string + kubeConfigSecret: + description: |- + KubeConfigSecret should reference an existing secret; the actual configuration will be read from + this secret's "kubeconfig" key. + properties: + name: + description: |- + Name of the referent. + More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names + TODO: Add other useful fields. apiVersion, kind, uid? + type: string + type: object + x-kubernetes-map-type: atomic + type: object + type: object + served: true + storage: true