From 20fdf7b1f2848e1555b9dc0dae683a3af9e4a9cb Mon Sep 17 00:00:00 2001 From: Diego Alfonso Date: Thu, 25 May 2023 13:34:41 -0500 Subject: [PATCH] fix: Update subpath only if provided when merge Signed-off-by: Diego Alfonso --- .../cartographer/v1alpha1/workload_helpers.go | 22 +++--- .../cartographer/v1alpha1/workload_test.go | 60 +++++++++------- .../testdata/workload-lsp-non-subPath.yaml | 30 ++++++++ pkg/commands/workload_apply_test.go | 69 +++++++++++++++++++ 4 files changed, 146 insertions(+), 35 deletions(-) create mode 100644 pkg/commands/testdata/workload-lsp-non-subPath.yaml diff --git a/pkg/apis/cartographer/v1alpha1/workload_helpers.go b/pkg/apis/cartographer/v1alpha1/workload_helpers.go index 5df482859..dbef827f8 100644 --- a/pkg/apis/cartographer/v1alpha1/workload_helpers.go +++ b/pkg/apis/cartographer/v1alpha1/workload_helpers.go @@ -342,36 +342,36 @@ func (w *WorkloadSpec) MergeGit(git GitSource) { w.Source = &Source{ Git: &git, } - if stash != nil && stash.Git != nil { + if stash != nil && stash.Subpath != "" { w.Source.Subpath = stash.Subpath } } } func (w *WorkloadSpec) MergeSourceImage(image string) { - stash := w.Source.DeepCopy() - w.ResetSource() - - w.Source = &Source{ - Image: image, - } - if stash != nil { - w.Source.Subpath = stash.Subpath + src := &Source{Image: image} + if w.Source != nil { + src.Subpath = w.Source.Subpath } + w.ResetSource() + w.Source = src } func (w *WorkloadSpec) MergeSubPath(subPath string) { if w.Source == nil { w.Source = &Source{} } - w.Source.Subpath = subPath } func (w *WorkloadSpec) MergeImage(image string) { + var src *Source + if w.Source != nil && w.Source.Subpath != "" { + src = &Source{Subpath: w.Source.Subpath} + } w.ResetSource() - w.Image = image + w.Source = src } func (w *WorkloadSpec) MergeEnv(env corev1.EnvVar) { diff --git a/pkg/apis/cartographer/v1alpha1/workload_test.go b/pkg/apis/cartographer/v1alpha1/workload_test.go index ac0aecef1..ee238c0d0 100644 --- a/pkg/apis/cartographer/v1alpha1/workload_test.go +++ b/pkg/apis/cartographer/v1alpha1/workload_test.go @@ -1001,6 +1001,42 @@ func TestWorkload_Merge(t *testing.T) { Image: "ubuntu:bionic", }, }, + }, { + name: "image without source", + seed: &Workload{}, + update: &Workload{ + Spec: WorkloadSpec{ + Image: "ubuntu:bionic", + }, + }, + want: &Workload{ + Spec: WorkloadSpec{ + Image: "ubuntu:bionic", + }, + }, + }, { + name: "image with subpath", + seed: &Workload{ + Spec: WorkloadSpec{ + Image: "alpine:latest", + Source: &Source{ + Subpath: "/sys", + }, + }, + }, + update: &Workload{ + Spec: WorkloadSpec{ + Image: "ubuntu:bionic", + }, + }, + want: &Workload{ + Spec: WorkloadSpec{ + Image: "ubuntu:bionic", + Source: &Source{ + Subpath: "/sys", + }, + }, + }, }, { name: "git", seed: &Workload{ @@ -1767,30 +1803,6 @@ func TestWorkloadSpec_MergeGit(t *testing.T) { Subpath: "my-subpath", }, }, - }, { - name: "update to git source deleting subpath", - seed: &WorkloadSpec{ - Source: &Source{ - Image: "my-registry.nip.io/my-folder/my-image:latest@sha:my-sha1234567890", - Subpath: "my-subpath", - }, - }, - git: GitSource{ - URL: "git@github.com:example/repo.git", - Ref: GitRef{ - Branch: "main", - }, - }, - want: &WorkloadSpec{ - Source: &Source{ - Git: &GitSource{ - URL: "git@github.com:example/repo.git", - Ref: GitRef{ - Branch: "main", - }, - }, - }, - }, }, { name: "delete source when setting repo to empty string", seed: &WorkloadSpec{ diff --git a/pkg/commands/testdata/workload-lsp-non-subPath.yaml b/pkg/commands/testdata/workload-lsp-non-subPath.yaml new file mode 100644 index 000000000..ff03e1c25 --- /dev/null +++ b/pkg/commands/testdata/workload-lsp-non-subPath.yaml @@ -0,0 +1,30 @@ +# Copyright 2023 VMware, Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +apiVersion: carto.run/v1alpha1 +kind: Workload +metadata: + annotations: + local-source-proxy.apps.tanzu.vmware.com: :default-my-workload@sha256:978be33a7f0cbe89bf48fbb438846047a28e1298d6d10d0de2d64bdc102a9e69 + labels: + apps.tanzu.vmware.com/workload-type: web + name: my-workload + namespace: default +spec: + params: + - name: annotations + value: + autoscaling.knative.dev/minScale: "2" + source: + image: :default-my-workload@sha256:978be33a7f0cbe89bf48fbb438846047a28e1298d6d10d0de2d64bdc102a9e69 diff --git a/pkg/commands/workload_apply_test.go b/pkg/commands/workload_apply_test.go index 7620bf232..2d66c12d4 100644 --- a/pkg/commands/workload_apply_test.go +++ b/pkg/commands/workload_apply_test.go @@ -8977,6 +8977,75 @@ No source code is changed To see logs: "tanzu apps workload tail my-workload --timestamp --since 1h" To get status: "tanzu apps workload get my-workload" +`, localSource), + }, + { + Name: "update from local source using lsp from file without subpath", + Skip: runtm.GOOS == "windows", + Args: []string{workloadName, flags.LocalPathFlagName, localSource, flags.FilePathFlagName, "./testdata/workload-lsp-non-subPath.yaml", flags.YesFlagName}, + GivenObjects: []client.Object{ + parent. + MetadataDie(func(d *diemetav1.ObjectMetaDie) { + d.Annotations(map[string]string{apis.LocalSourceProxyAnnotationName: ":default-my-workload@sha256:978be33a7f0cbe89bf48fbb438846047a28e1298d6d10d0de2d64bdc102a9e69"}) + d.Labels(map[string]string{apis.WorkloadTypeLabelName: "web"}) + }).SpecDie(func(d *diecartov1alpha1.WorkloadSpecDie) { + d.Source(&cartov1alpha1.Source{ + Image: ":default-my-workload@sha256:978be33a7f0cbe89bf48fbb438846047a28e1298d6d10d0de2d64bdc102a9e69", + Subpath: "current-subpath", + }) + }), + }, + KubeConfigTransport: clitesting.NewFakeTransportFromResponse(respCreator(http.StatusOK, `{"statuscode": "200", "message": "any ignored message"}`, myWorkloadHeader)), + ExpectUpdates: []client.Object{ + &cartov1alpha1.Workload{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: defaultNamespace, + Name: workloadName, + Labels: map[string]string{ + apis.WorkloadTypeLabelName: "web", + }, + Annotations: map[string]string{ + "local-source-proxy.apps.tanzu.vmware.com": ":default-my-workload@sha256:978be33a7f0cbe89bf48fbb438846047a28e1298d6d10d0de2d64bdc102a9e69", + }, + }, + Spec: cartov1alpha1.WorkloadSpec{ + Source: &cartov1alpha1.Source{ + Image: ":default-my-workload@sha256:978be33a7f0cbe89bf48fbb438846047a28e1298d6d10d0de2d64bdc102a9e69", + Subpath: "current-subpath", + }, + Params: []cartov1alpha1.Param{ + { + Name: "annotations", + Value: apiextensionsv1.JSON{Raw: []byte(`{"autoscaling.knative.dev/minScale":"2"}`)}, + }, + }, + }, + }, + }, + ExpectOutput: fmt.Sprintf(` +❗ WARNING: Configuration file update strategy is changing. By default, provided configuration files will replace rather than merge existing configuration. The change will take place in the January 2024 TAP release (use "--update-strategy" to control strategy explicitly). + +Publishing source in "%s" to "local-source-proxy.tap-local-source-system.svc.cluster.local/source:default-my-workload"... +No source code is changed + +🔎 Update workload: +... + 8, 8 | apps.tanzu.vmware.com/workload-type: web + 9, 9 | name: my-workload + 10, 10 | namespace: default + 11, 11 |spec: + 12 + | params: + 13 + | - name: annotations + 14 + | value: + 15 + | autoscaling.knative.dev/minScale: "2" + 12, 16 | source: + 13, 17 | image: :default-my-workload@sha256:978be33a7f0cbe89bf48fbb438846047a28e1298d6d10d0de2d64bdc102a9e69 + 14, 18 | subPath: current-subpath +👍 Updated workload "my-workload" + +To see logs: "tanzu apps workload tail my-workload --timestamp --since 1h" +To get status: "tanzu apps workload get my-workload" + `, localSource), }, }