From 8eb2eea343413e3dbc80b5c97da939ce416fdd7a Mon Sep 17 00:00:00 2001 From: Fabian Fischer Date: Thu, 27 Jul 2023 17:25:58 +0200 Subject: [PATCH 1/2] Allow upgrading redis to newer versions --- .../functions/vshnredis/release.go | 35 ++-- .../functions/vshnredis/release_test.go | 104 ++++++++++++ .../release/03_version_update.yaml.tmpl | 155 ++++++++++++++++++ 3 files changed, 284 insertions(+), 10 deletions(-) create mode 100644 test/transforms/vshnredis/release/03_version_update.yaml.tmpl diff --git a/pkg/comp-functions/functions/vshnredis/release.go b/pkg/comp-functions/functions/vshnredis/release.go index 420d5f7bc..72e295098 100644 --- a/pkg/comp-functions/functions/vshnredis/release.go +++ b/pkg/comp-functions/functions/vshnredis/release.go @@ -6,6 +6,7 @@ import ( "errors" "fmt" + "github.com/blang/semver/v4" "github.com/crossplane-contrib/provider-helm/apis/release/v1beta1" vshnv1 "github.com/vshn/appcat/apis/vshn/v1" "github.com/vshn/appcat/pkg/comp-functions/runtime" @@ -86,7 +87,7 @@ func updateRelease(ctx context.Context, comp *vshnv1.VSHNRedis, desired *v1beta1 } l.V(1).Info("Setting release version") - err = setReleaseVersion(comp, values, observedValues) + err = setReleaseVersion(ctx, comp, values, observedValues) if err != nil { return nil, fmt.Errorf("cannot set redis version for release %s: %v", releaseName, err) } @@ -194,18 +195,32 @@ func getReleaseValues(r *v1beta1.Release) (map[string]interface{}, error) { } // setReleaseVersion sets the version from the claim if it's a new instance otherwise it is managed by maintenance function -func setReleaseVersion(comp *vshnv1.VSHNRedis, values map[string]interface{}, observed map[string]interface{}) error { - var err error +func setReleaseVersion(ctx context.Context, comp *vshnv1.VSHNRedis, values map[string]interface{}, observed map[string]interface{}) error { + l := controllerruntime.LoggerFrom(ctx) + tag, _, err := unstructured.NestedString(observed, "image", "tag") if err != nil { return fmt.Errorf("cannot get image tag from values in release: %v", err) } - if tag != "" { - // In case the tag is set, keep the observed version - err = unstructured.SetNestedField(values, tag, "image", "tag") - } else { - // In case the tag is not set then this is a new Release therefore we need to set the version from the claim - err = unstructured.SetNestedField(values, comp.Spec.Parameters.Service.Version, "image", "tag") + + desiredVersion, err := semver.ParseTolerant(comp.Spec.Parameters.Service.Version) + if err != nil { + l.Info("failed to parse desired redis version", "version", comp.Spec.Parameters.Service.Version) + // If the desired version is not parsable, just keep the observed one + return unstructured.SetNestedField(values, tag, "image", "tag") + } + + observedVersion, err := semver.ParseTolerant(tag) + if err != nil { + l.Info("failed to parse observed redis version", "version", tag) + // If the observed version is not parsable, e.g. if it's empty, update to the desired version + return unstructured.SetNestedField(values, comp.Spec.Parameters.Service.Version, "image", "tag") + } + + if observedVersion.GTE(desiredVersion) { + // In case the overved tag is valid and greater than the desired version, keep the observed version + return unstructured.SetNestedField(values, tag, "image", "tag") } - return err + // In case the observed tag is smaller than the desired version, then set the version from the claim + return unstructured.SetNestedField(values, comp.Spec.Parameters.Service.Version, "image", "tag") } diff --git a/pkg/comp-functions/functions/vshnredis/release_test.go b/pkg/comp-functions/functions/vshnredis/release_test.go index 7ec4412ca..cc43eaa0c 100644 --- a/pkg/comp-functions/functions/vshnredis/release_test.go +++ b/pkg/comp-functions/functions/vshnredis/release_test.go @@ -1,12 +1,18 @@ package vshnredis import ( + "bytes" "context" "fmt" + "os" + "path/filepath" + "strings" "testing" + "text/template" xhelm "github.com/crossplane-contrib/provider-helm/apis/release/v1beta1" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" vshnv1 "github.com/vshn/appcat/apis/vshn/v1" "github.com/vshn/appcat/pkg/comp-functions/functions/commontest" "github.com/vshn/appcat/pkg/comp-functions/runtime" @@ -88,3 +94,101 @@ func getRedisReleaseComp(t *testing.T, file string) (*runtime.Runtime, *vshnv1.V return iof, comp } + +func TestAllowVersionUpgrade(t *testing.T) { + + tcs := map[string]struct { + CompositeVersion string + ObservedVersion string + Desired string + }{ + "newMinor": { + CompositeVersion: "6.2", + ObservedVersion: "6.0.2", + Desired: "6.2", + }, + "newMajor": { + CompositeVersion: "7.0", + ObservedVersion: "6.2.2", + Desired: "7.0", + }, + "sameMinior": { + CompositeVersion: "6.2", + ObservedVersion: "6.2.2", + Desired: "6.2.2", + }, + "sameVersion": { + CompositeVersion: "6.2", + ObservedVersion: "6.2", + Desired: "6.2", + }, + "noObserved": { + CompositeVersion: "6.2", + ObservedVersion: "", + Desired: "6.2", + }, + "downgrade": { + CompositeVersion: "6.2", + ObservedVersion: "7.0.2", + Desired: "7.0.2", + }, + "invalidObserved": { + CompositeVersion: "6.2", + ObservedVersion: "randomTag", + Desired: "6.2", + }, + "invalidDesired": { + CompositeVersion: "randomTag", + ObservedVersion: "7.0.3", + Desired: "7.0.3", + }, + "allInvalid": { + CompositeVersion: "randomTag", + ObservedVersion: "weirdTag", + Desired: "weirdTag", + }, + } + for name, tc := range tcs { + t.Run(name, func(t *testing.T) { + + ctx := context.TODO() + iof := loadRuntimeFromTemplate(t, fmt.Sprintf("vshnredis/release/03_version_update.yaml.tmpl"), tc) + assert.Equal(t, runtime.NewNormal(), ManageRelease(ctx, iof)) + release := &xhelm.Release{} + assert.NoError(t, iof.Desired.Get(ctx, release, "release")) + + valueMap := map[string]any{} + assert.NoError(t, yaml.Unmarshal(release.Spec.ForProvider.Values.Raw, &valueMap)) + require.NotEmpty(t, valueMap["master"]) + imageMap := valueMap["image"].(map[string]any) + assert.Equal(t, tc.Desired, imageMap["tag"]) + }) + } +} + +func loadRuntimeFromTemplate(t assert.TestingT, file string, data any) *runtime.Runtime { + p, _ := filepath.Abs(".") + before, _, _ := strings.Cut(p, "pkg") + f, err := os.Open(before + "test/transforms/" + file) + assert.NoError(t, err) + b1, err := os.ReadFile(f.Name()) + if err != nil { + assert.FailNow(t, "can't get example") + } + + tmpl, err := template.New("test").Parse(string(b1)) + if err != nil { + assert.FailNow(t, "can't load tempalte") + } + + var buf bytes.Buffer + err = tmpl.Execute(&buf, data) + if err != nil { + assert.FailNow(t, "can't execute tempalte") + } + + funcIO, err := runtime.NewRuntime(context.Background(), buf.Bytes()) + assert.NoError(t, err) + + return funcIO +} diff --git a/test/transforms/vshnredis/release/03_version_update.yaml.tmpl b/test/transforms/vshnredis/release/03_version_update.yaml.tmpl new file mode 100644 index 000000000..b964579a2 --- /dev/null +++ b/test/transforms/vshnredis/release/03_version_update.yaml.tmpl @@ -0,0 +1,155 @@ +apiVersion: apiextensions.crossplane.io/v1alpha1 +kind: FunctionIO +observed: + composite: + resource: + apiVersion: vshn.appcat.vshn.io/v1 + kind: XVSHNRedis + metadata: + annotations: + creationTimestamp: "2023-03-21T16:52:31Z" + finalizers: + - composite.apiextensions.crossplane.io + generateName: redis- + generation: 13 + labels: + appuio.io/organization: vshn + crossplane.io/claim-name: redis + crossplane.io/claim-namespace: unit-test + crossplane.io/composite: redis-gc9x4 + name: redis-gc9x4 + spec: + claimRef: + apiVersion: vshn.appcat.vshn.io/v1 + kind: VSHNRedis + name: redis + namespace: unit-test + compositionRef: + name: vshnredis.vshn.appcat.vshn.io + compositionRevisionRef: + name: vshnredis.vshn.appcat.vshn.io-ce52f13 + compositionUpdatePolicy: Automatic + parameters: + service: + version: "{{ .CompositeVersion }}" + resources: + - name: release + resource: + apiVersion: helm.crossplane.io/v1beta1 + kind: Release + spec: + forProvider: + chart: + name: redis + repository: https://charts.bitnami.com/bitnami + values: + architecture: standalone + commonConfiguration: 'dummy' + fullnameOverride: redis + global: + imageRegistry: 'dummy' + image: + repository: bitnami/redis + tag: "{{ .ObservedVersion }}" + master: + extraVolumes: [] + containerSecurityContext: + enabled: true + persistence: + size: '7Gi' + podSecurityContext: + enabled: true + resources: + limits: + cpu: '10m' + memory: '1Gi' + requests: + cpu: '10m' + memory: '1Gi' + networkPolicy: + allowExternal: false + enabled: true + ingressNSMatchLabels: + kubernetes.io/metadata.name: 'dummy' + tls: + authClients: true + autoGenerated: false + certCAFilename: ca.crt + certFilename: tls.crt + certKeyFilename: tls.key + enabled: true + existingSecret: tls-server-certificate + providerConfigRef: + name: helm +desired: + composite: + connectionDetails: null + resource: + apiVersion: vshn.appcat.vshn.io/v1 + kind: XVSHNRedis + metadata: + creationTimestamp: "2023-03-21T16:52:31Z" + finalizers: + - composite.apiextensions.crossplane.io + generateName: redis- + generation: 13 + labels: + appuio.io/organization: vshn + crossplane.io/claim-name: redis + crossplane.io/claim-namespace: unit-test + crossplane.io/composite: redis-gc9x4 + name: redis-gc9x4 + spec: + parameters: + service: + version: "{{ .CompositeVersion }}" + writeConnectionSecretToRef: {} + status: {} + resources: + - name: release + resource: + apiVersion: helm.crossplane.io/v1beta1 + kind: Release + spec: + forProvider: + chart: + name: redis + repository: https://charts.bitnami.com/bitnami + values: + architecture: standalone + commonConfiguration: 'updated' # check that desired is applied + fullnameOverride: redis + global: + imageRegistry: 'dummy' + image: + repository: bitnami/redis + tag: '7.0' + master: + containerSecurityContext: + enabled: true + persistence: + size: '7Gi' + podSecurityContext: + enabled: true + resources: + limits: + cpu: '10m' + memory: '1Gi' + requests: + cpu: '10m' + memory: '1Gi' + networkPolicy: + allowExternal: false + enabled: true + ingressNSMatchLabels: + kubernetes.io/metadata.name: 'dummy' + tls: + authClients: true + autoGenerated: false + certCAFilename: ca.crt + certFilename: tls.crt + certKeyFilename: tls.key + enabled: true + existingSecret: tls-server-certificate + providerConfigRef: + name: helm From 6a0db3ebc11c9a6db757377b6d405d09c6e7d933 Mon Sep 17 00:00:00 2001 From: Fabian Fischer Date: Mon, 31 Jul 2023 11:01:45 +0200 Subject: [PATCH 2/2] Return an error if desired version is invalid --- pkg/comp-functions/functions/vshnredis/release.go | 3 +-- .../functions/vshnredis/release_test.go | 13 ++++++++++++- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/pkg/comp-functions/functions/vshnredis/release.go b/pkg/comp-functions/functions/vshnredis/release.go index 72e295098..ca8179dae 100644 --- a/pkg/comp-functions/functions/vshnredis/release.go +++ b/pkg/comp-functions/functions/vshnredis/release.go @@ -206,8 +206,7 @@ func setReleaseVersion(ctx context.Context, comp *vshnv1.VSHNRedis, values map[s desiredVersion, err := semver.ParseTolerant(comp.Spec.Parameters.Service.Version) if err != nil { l.Info("failed to parse desired redis version", "version", comp.Spec.Parameters.Service.Version) - // If the desired version is not parsable, just keep the observed one - return unstructured.SetNestedField(values, tag, "image", "tag") + return fmt.Errorf("invalid redis version %q", comp.Spec.Parameters.Service.Version) } observedVersion, err := semver.ParseTolerant(tag) diff --git a/pkg/comp-functions/functions/vshnredis/release_test.go b/pkg/comp-functions/functions/vshnredis/release_test.go index cc43eaa0c..409df3cc4 100644 --- a/pkg/comp-functions/functions/vshnredis/release_test.go +++ b/pkg/comp-functions/functions/vshnredis/release_test.go @@ -11,6 +11,7 @@ import ( "text/template" xhelm "github.com/crossplane-contrib/provider-helm/apis/release/v1beta1" + crossfnv1alpha1 "github.com/crossplane/crossplane/apis/apiextensions/fn/io/v1alpha1" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" vshnv1 "github.com/vshn/appcat/apis/vshn/v1" @@ -101,6 +102,7 @@ func TestAllowVersionUpgrade(t *testing.T) { CompositeVersion string ObservedVersion string Desired string + Fail bool }{ "newMinor": { CompositeVersion: "6.2", @@ -141,11 +143,13 @@ func TestAllowVersionUpgrade(t *testing.T) { CompositeVersion: "randomTag", ObservedVersion: "7.0.3", Desired: "7.0.3", + Fail: true, }, "allInvalid": { CompositeVersion: "randomTag", ObservedVersion: "weirdTag", Desired: "weirdTag", + Fail: true, }, } for name, tc := range tcs { @@ -153,7 +157,14 @@ func TestAllowVersionUpgrade(t *testing.T) { ctx := context.TODO() iof := loadRuntimeFromTemplate(t, fmt.Sprintf("vshnredis/release/03_version_update.yaml.tmpl"), tc) - assert.Equal(t, runtime.NewNormal(), ManageRelease(ctx, iof)) + + res := ManageRelease(ctx, iof) + if tc.Fail { + assert.Equal(t, crossfnv1alpha1.SeverityFatal, res.Resolve().Severity) + return + } + assert.Equal(t, runtime.NewNormal(), res) + release := &xhelm.Release{} assert.NoError(t, iof.Desired.Get(ctx, release, "release"))