From 5f595d21f9acf9a800d3e844f43a13d2a9472e2f Mon Sep 17 00:00:00 2001 From: michaelvl Date: Sat, 26 Aug 2023 16:09:22 +0200 Subject: [PATCH 1/5] Add test-case for kpt pkg update not updating Kptfile bug --- commands/pkg/update/cmdupdate_test.go | 95 +++++++++++++++++++ .../testutil/testdata/dataset1/mysql/Kptfile | 16 ++++ .../testutil/testdata/dataset2/mysql/Kptfile | 20 ++++ .../testutil/testdata/dataset4/mysql/Kptfile | 20 ++++ .../testutil/testdata/dataset5/mysql/Kptfile | 20 ++++ 5 files changed, 171 insertions(+) create mode 100644 internal/testutil/testdata/dataset1/mysql/Kptfile create mode 100644 internal/testutil/testdata/dataset2/mysql/Kptfile create mode 100644 internal/testutil/testdata/dataset4/mysql/Kptfile create mode 100644 internal/testutil/testdata/dataset5/mysql/Kptfile diff --git a/commands/pkg/update/cmdupdate_test.go b/commands/pkg/update/cmdupdate_test.go index a788e4ca8e..e5b8a8ac87 100644 --- a/commands/pkg/update/cmdupdate_test.go +++ b/commands/pkg/update/cmdupdate_test.go @@ -27,12 +27,14 @@ import ( "github.com/GoogleContainerTools/kpt/commands/pkg/get" "github.com/GoogleContainerTools/kpt/commands/pkg/update" "github.com/GoogleContainerTools/kpt/internal/gitutil" + "github.com/GoogleContainerTools/kpt/internal/pkg" "github.com/GoogleContainerTools/kpt/internal/testutil" "github.com/GoogleContainerTools/kpt/internal/testutil/pkgbuilder" kptfilev1 "github.com/GoogleContainerTools/kpt/pkg/api/kptfile/v1" "github.com/GoogleContainerTools/kpt/pkg/printer/fake" "github.com/spf13/cobra" "github.com/stretchr/testify/assert" + "sigs.k8s.io/kustomize/kyaml/filesys" "sigs.k8s.io/kustomize/kyaml/yaml" ) @@ -132,6 +134,99 @@ func TestCmd_execute(t *testing.T) { } } +// TestCmd_execute verifies that update is correctly invoked with an upstream 'mysql' package with multiple versions +func TestCmd_subpkgVersions(t *testing.T) { + // Setup version v1 of upstream package + g, w, clean := testutil.SetupRepoAndWorkspace(t, testutil.Content{ + Data: testutil.Dataset1, + Branch: "master", + }) + defer clean() + err := g.Tag("v1") + // update the master branch + if !assert.NoError(t, g.ReplaceData(testutil.Dataset2)) { + return + } + _, err = g.Commit("modify upstream package -- ds2") + if !assert.NoError(t, err) { + return + } + err = g.Tag("v2") + + defer testutil.Chdir(t, w.WorkspaceDirectory)() + + dest := filepath.Join(w.WorkspaceDirectory, "mysql") + + // Initial clone of package v1 + getCmd := get.NewRunner(fake.CtxWithDefaultPrinter(), "kpt") + getCmd.Command.SetArgs([]string{"file://" + g.RepoDirectory + ".git/mysql@v1", w.WorkspaceDirectory}) + err = getCmd.Command.Execute() + if !assert.NoError(t, err) { + return + } + if !g.AssertEqual(t, filepath.Join(g.DatasetDirectory, testutil.Dataset1, "mysql"), dest, true) { + return + } + + // update the cloned package to v2 + updateCmd := update.NewRunner(fake.CtxWithDefaultPrinter(), "kpt") + updateCmd.Command.SetArgs([]string{"mysql@v2", "--strategy", "fast-forward"}) + if !assert.NoError(t, updateCmd.Command.Execute()) { + return + } + if !g.AssertEqual(t, filepath.Join(g.DatasetDirectory, testutil.Dataset2, "mysql"), dest, true) { + return + } + + commit, err := g.GetCommit() + if !assert.NoError(t, err) { + return + } + + // Reference Kptfile for package v2 + pkgV2Kptfile, err := pkg.ReadKptfile(filesys.FileSystemOrOnDisk{}, filepath.Join(g.DatasetDirectory, testutil.Dataset2, "mysql")) + if !assert.NoError(t, err) { + return + } + + if !g.AssertKptfile(t, dest, kptfilev1.KptFile{ + ResourceMeta: yaml.ResourceMeta{ + ObjectMeta: yaml.ObjectMeta{ + NameMeta: yaml.NameMeta{ + Name: "mysql", + }, + }, + TypeMeta: yaml.TypeMeta{ + APIVersion: kptfilev1.TypeMeta.APIVersion, + Kind: kptfilev1.TypeMeta.Kind}, + }, + Upstream: &kptfilev1.Upstream{ + Type: kptfilev1.GitOrigin, + Git: &kptfilev1.Git{ + Repo: "file://" + g.RepoDirectory, + Ref: "v2", + Directory: "/mysql", + }, + UpdateStrategy: kptfilev1.FastForward, + }, + UpstreamLock: &kptfilev1.UpstreamLock{ + Type: kptfilev1.GitOrigin, + Git: &kptfilev1.GitLock{ + Repo: "file://" + g.RepoDirectory, + Ref: "v2", + Directory: "/mysql", + Commit: commit, + }, + }, + Info: &kptfilev1.PackageInfo{ + Description: pkgV2Kptfile.Info.Description, + }, + Pipeline: pkgV2Kptfile.Pipeline, + }) { + return + } +} + func TestCmd_successUnCommitted(t *testing.T) { g, w, clean := testutil.SetupRepoAndWorkspace(t, testutil.Content{ Data: testutil.Dataset1, diff --git a/internal/testutil/testdata/dataset1/mysql/Kptfile b/internal/testutil/testdata/dataset1/mysql/Kptfile new file mode 100644 index 0000000000..13b6ed5ca6 --- /dev/null +++ b/internal/testutil/testdata/dataset1/mysql/Kptfile @@ -0,0 +1,16 @@ +apiVersion: kpt.dev/v1 +kind: Kptfile +metadata: + name: mysql +info: + description: kpt package for mysql +pipeline: + mutators: + - image: gcr.io/kpt-fn/set-namespace:v0.4.1 + configMap: + namespace: example-ns + name: set namespace + - image: gcr.io/kpt-fn/set-labels:v0.2.0 + configMap: + color: orange + name: set color label diff --git a/internal/testutil/testdata/dataset2/mysql/Kptfile b/internal/testutil/testdata/dataset2/mysql/Kptfile new file mode 100644 index 0000000000..cfd69cdac1 --- /dev/null +++ b/internal/testutil/testdata/dataset2/mysql/Kptfile @@ -0,0 +1,20 @@ +apiVersion: kpt.dev/v1 +kind: Kptfile +metadata: + name: mysql +info: + description: mysql package +pipeline: + mutators: + - image: gcr.io/kpt-fn/set-namespace:v0.4.1 + configMap: + namespace: example-ns + name: set namespace + - image: gcr.io/kpt-fn/set-labels:v0.2.0 + configMap: + fruit: apple + name: set fruit label + - image: gcr.io/kpt-fn/set-labels:v0.2.0 + configMap: + color: orange + name: set color label diff --git a/internal/testutil/testdata/dataset4/mysql/Kptfile b/internal/testutil/testdata/dataset4/mysql/Kptfile new file mode 100644 index 0000000000..cfd69cdac1 --- /dev/null +++ b/internal/testutil/testdata/dataset4/mysql/Kptfile @@ -0,0 +1,20 @@ +apiVersion: kpt.dev/v1 +kind: Kptfile +metadata: + name: mysql +info: + description: mysql package +pipeline: + mutators: + - image: gcr.io/kpt-fn/set-namespace:v0.4.1 + configMap: + namespace: example-ns + name: set namespace + - image: gcr.io/kpt-fn/set-labels:v0.2.0 + configMap: + fruit: apple + name: set fruit label + - image: gcr.io/kpt-fn/set-labels:v0.2.0 + configMap: + color: orange + name: set color label diff --git a/internal/testutil/testdata/dataset5/mysql/Kptfile b/internal/testutil/testdata/dataset5/mysql/Kptfile new file mode 100644 index 0000000000..cfd69cdac1 --- /dev/null +++ b/internal/testutil/testdata/dataset5/mysql/Kptfile @@ -0,0 +1,20 @@ +apiVersion: kpt.dev/v1 +kind: Kptfile +metadata: + name: mysql +info: + description: mysql package +pipeline: + mutators: + - image: gcr.io/kpt-fn/set-namespace:v0.4.1 + configMap: + namespace: example-ns + name: set namespace + - image: gcr.io/kpt-fn/set-labels:v0.2.0 + configMap: + fruit: apple + name: set fruit label + - image: gcr.io/kpt-fn/set-labels:v0.2.0 + configMap: + color: orange + name: set color label From 185e2204246fdf5d8ced0ffa3d3c565383f603be Mon Sep 17 00:00:00 2001 From: michaelvl Date: Sat, 26 Aug 2023 16:18:10 +0200 Subject: [PATCH 2/5] Fix comment --- commands/pkg/update/cmdupdate_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/commands/pkg/update/cmdupdate_test.go b/commands/pkg/update/cmdupdate_test.go index e5b8a8ac87..35148b59a5 100644 --- a/commands/pkg/update/cmdupdate_test.go +++ b/commands/pkg/update/cmdupdate_test.go @@ -134,7 +134,7 @@ func TestCmd_execute(t *testing.T) { } } -// TestCmd_execute verifies that update is correctly invoked with an upstream 'mysql' package with multiple versions +// TestCmd_subpkgVersions verifies that update is correctly invoked with an upstream 'mysql' package with multiple versions func TestCmd_subpkgVersions(t *testing.T) { // Setup version v1 of upstream package g, w, clean := testutil.SetupRepoAndWorkspace(t, testutil.Content{ From 222f000f56601ad6caf1b48098e8e73d5237c4f9 Mon Sep 17 00:00:00 2001 From: michaelvl Date: Sat, 26 Aug 2023 16:21:27 +0200 Subject: [PATCH 3/5] Add error checking in test case --- commands/pkg/update/cmdupdate_test.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/commands/pkg/update/cmdupdate_test.go b/commands/pkg/update/cmdupdate_test.go index 35148b59a5..e3d9bc0a1b 100644 --- a/commands/pkg/update/cmdupdate_test.go +++ b/commands/pkg/update/cmdupdate_test.go @@ -143,6 +143,9 @@ func TestCmd_subpkgVersions(t *testing.T) { }) defer clean() err := g.Tag("v1") + if !assert.NoError(t, err) { + t.FailNow() + } // update the master branch if !assert.NoError(t, g.ReplaceData(testutil.Dataset2)) { return @@ -152,6 +155,9 @@ func TestCmd_subpkgVersions(t *testing.T) { return } err = g.Tag("v2") + if !assert.NoError(t, err) { + t.FailNow() + } defer testutil.Chdir(t, w.WorkspaceDirectory)() From ef918cf5ff3babe66763299fc8440e808a06c9b6 Mon Sep 17 00:00:00 2001 From: michaelvl Date: Sat, 26 Aug 2023 21:40:01 +0200 Subject: [PATCH 4/5] Editorial changes --- commands/pkg/update/cmdupdate_test.go | 24 ++++++++++++------------ internal/testutil/testutil.go | 6 +++--- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/commands/pkg/update/cmdupdate_test.go b/commands/pkg/update/cmdupdate_test.go index e3d9bc0a1b..367cc1606d 100644 --- a/commands/pkg/update/cmdupdate_test.go +++ b/commands/pkg/update/cmdupdate_test.go @@ -142,7 +142,7 @@ func TestCmd_subpkgVersions(t *testing.T) { Branch: "master", }) defer clean() - err := g.Tag("v1") + err := g.Tag("dataset1") if !assert.NoError(t, err) { t.FailNow() } @@ -154,7 +154,7 @@ func TestCmd_subpkgVersions(t *testing.T) { if !assert.NoError(t, err) { return } - err = g.Tag("v2") + err = g.Tag("dataset2") if !assert.NoError(t, err) { t.FailNow() } @@ -163,9 +163,9 @@ func TestCmd_subpkgVersions(t *testing.T) { dest := filepath.Join(w.WorkspaceDirectory, "mysql") - // Initial clone of package v1 + // Initial clone of package version 'dataset1' getCmd := get.NewRunner(fake.CtxWithDefaultPrinter(), "kpt") - getCmd.Command.SetArgs([]string{"file://" + g.RepoDirectory + ".git/mysql@v1", w.WorkspaceDirectory}) + getCmd.Command.SetArgs([]string{"file://" + g.RepoDirectory + ".git/mysql@dataset1", w.WorkspaceDirectory}) err = getCmd.Command.Execute() if !assert.NoError(t, err) { return @@ -174,9 +174,9 @@ func TestCmd_subpkgVersions(t *testing.T) { return } - // update the cloned package to v2 + // update the cloned package to dataset2 updateCmd := update.NewRunner(fake.CtxWithDefaultPrinter(), "kpt") - updateCmd.Command.SetArgs([]string{"mysql@v2", "--strategy", "fast-forward"}) + updateCmd.Command.SetArgs([]string{"mysql@dataset2", "--strategy", "fast-forward"}) if !assert.NoError(t, updateCmd.Command.Execute()) { return } @@ -189,8 +189,8 @@ func TestCmd_subpkgVersions(t *testing.T) { return } - // Reference Kptfile for package v2 - pkgV2Kptfile, err := pkg.ReadKptfile(filesys.FileSystemOrOnDisk{}, filepath.Join(g.DatasetDirectory, testutil.Dataset2, "mysql")) + // Reference Kptfile for package version 'dataset2' + pkgDs2Kptfile, err := pkg.ReadKptfile(filesys.FileSystemOrOnDisk{}, filepath.Join(g.DatasetDirectory, testutil.Dataset2, "mysql")) if !assert.NoError(t, err) { return } @@ -210,7 +210,7 @@ func TestCmd_subpkgVersions(t *testing.T) { Type: kptfilev1.GitOrigin, Git: &kptfilev1.Git{ Repo: "file://" + g.RepoDirectory, - Ref: "v2", + Ref: "dataset2", Directory: "/mysql", }, UpdateStrategy: kptfilev1.FastForward, @@ -219,15 +219,15 @@ func TestCmd_subpkgVersions(t *testing.T) { Type: kptfilev1.GitOrigin, Git: &kptfilev1.GitLock{ Repo: "file://" + g.RepoDirectory, - Ref: "v2", + Ref: "dataset2", Directory: "/mysql", Commit: commit, }, }, Info: &kptfilev1.PackageInfo{ - Description: pkgV2Kptfile.Info.Description, + Description: pkgDs2Kptfile.Info.Description, }, - Pipeline: pkgV2Kptfile.Pipeline, + Pipeline: pkgDs2Kptfile.Pipeline, }) { return } diff --git a/internal/testutil/testutil.go b/internal/testutil/testutil.go index 4a0c156391..69c948b718 100644 --- a/internal/testutil/testutil.go +++ b/internal/testutil/testutil.go @@ -46,10 +46,10 @@ const TmpDirPrefix = "test-kpt" const ( Dataset1 = "dataset1" - Dataset2 = "dataset2" + Dataset2 = "dataset2" // Dataset2 is a replica of Dataset1 with workload spec changes (ports, replica count etc.) and a Kptfile mod Dataset3 = "dataset3" - Dataset4 = "dataset4" // Dataset4 is replica of Dataset2 with different setter values - Dataset5 = "dataset5" // Dataset5 is replica of Dataset2 with additional non KRM files + Dataset4 = "dataset4" // Dataset4 is a replica of Dataset2 with different setter values + Dataset5 = "dataset5" // Dataset5 is a replica of Dataset2 with additional non KRM files Dataset6 = "dataset6" // Dataset6 contains symlinks DatasetMerged = "datasetmerged" DiffOutput = "diff_output" From a779c1dfd6a30e410c9c270252a0be002335e7c7 Mon Sep 17 00:00:00 2001 From: michaelvl Date: Sat, 26 Aug 2023 22:31:47 +0200 Subject: [PATCH 5/5] Fix diff of Kptfile. Add additional kptfile validation --- commands/pkg/update/cmdupdate_test.go | 57 ++++++++++++++++++++++++--- internal/testutil/testutil.go | 9 ++--- 2 files changed, 56 insertions(+), 10 deletions(-) diff --git a/commands/pkg/update/cmdupdate_test.go b/commands/pkg/update/cmdupdate_test.go index 367cc1606d..fa893c8a7f 100644 --- a/commands/pkg/update/cmdupdate_test.go +++ b/commands/pkg/update/cmdupdate_test.go @@ -142,7 +142,12 @@ func TestCmd_subpkgVersions(t *testing.T) { Branch: "master", }) defer clean() - err := g.Tag("dataset1") + + commitDs1, err := g.GetCommit() + if !assert.NoError(t, err) { + return + } + err = g.Tag("dataset1") if !assert.NoError(t, err) { t.FailNow() } @@ -163,7 +168,7 @@ func TestCmd_subpkgVersions(t *testing.T) { dest := filepath.Join(w.WorkspaceDirectory, "mysql") - // Initial clone of package version 'dataset1' + // pkg get package version 'dataset1' getCmd := get.NewRunner(fake.CtxWithDefaultPrinter(), "kpt") getCmd.Command.SetArgs([]string{"file://" + g.RepoDirectory + ".git/mysql@dataset1", w.WorkspaceDirectory}) err = getCmd.Command.Execute() @@ -174,7 +179,49 @@ func TestCmd_subpkgVersions(t *testing.T) { return } - // update the cloned package to dataset2 + // Reference Kptfile for package version 'dataset1' + pkgDs1Kptfile, err := pkg.ReadKptfile(filesys.FileSystemOrOnDisk{}, filepath.Join(g.DatasetDirectory, testutil.Dataset1, "mysql")) + if !assert.NoError(t, err) { + return + } + if !g.AssertKptfile(t, dest, kptfilev1.KptFile{ + ResourceMeta: yaml.ResourceMeta{ + ObjectMeta: yaml.ObjectMeta{ + NameMeta: yaml.NameMeta{ + Name: "mysql", + }, + }, + TypeMeta: yaml.TypeMeta{ + APIVersion: kptfilev1.TypeMeta.APIVersion, + Kind: kptfilev1.TypeMeta.Kind}, + }, + Upstream: &kptfilev1.Upstream{ + Type: kptfilev1.GitOrigin, + Git: &kptfilev1.Git{ + Repo: "file://" + g.RepoDirectory, + Ref: "dataset1", + Directory: "/mysql", + }, + UpdateStrategy: kptfilev1.ResourceMerge, // Defaulted + }, + UpstreamLock: &kptfilev1.UpstreamLock{ + Type: kptfilev1.GitOrigin, + Git: &kptfilev1.GitLock{ + Repo: "file://" + g.RepoDirectory, + Ref: "dataset1", + Directory: "/mysql", + Commit: commitDs1, + }, + }, + Info: &kptfilev1.PackageInfo{ + Description: pkgDs1Kptfile.Info.Description, + }, + Pipeline: pkgDs1Kptfile.Pipeline, + }) { + return + } + + // pkg update to version 'dataset2' updateCmd := update.NewRunner(fake.CtxWithDefaultPrinter(), "kpt") updateCmd.Command.SetArgs([]string{"mysql@dataset2", "--strategy", "fast-forward"}) if !assert.NoError(t, updateCmd.Command.Execute()) { @@ -184,7 +231,7 @@ func TestCmd_subpkgVersions(t *testing.T) { return } - commit, err := g.GetCommit() + commitDs2, err := g.GetCommit() if !assert.NoError(t, err) { return } @@ -221,7 +268,7 @@ func TestCmd_subpkgVersions(t *testing.T) { Repo: "file://" + g.RepoDirectory, Ref: "dataset2", Directory: "/mysql", - Commit: commit, + Commit: commitDs2, }, }, Info: &kptfilev1.PackageInfo{ diff --git a/internal/testutil/testutil.go b/internal/testutil/testutil.go index 69c948b718..6094b39b1b 100644 --- a/internal/testutil/testutil.go +++ b/internal/testutil/testutil.go @@ -15,7 +15,6 @@ package testutil import ( - "bytes" "fmt" "os" "os/exec" @@ -272,12 +271,12 @@ func (g *TestGitRepo) AssertKptfile(t *testing.T, cloned string, kpkg kptfilev1. if !assert.NoError(t, err) { return false } - var res bytes.Buffer - d := yaml.NewEncoder(&res) - if !assert.NoError(t, d.Encode(kpkg)) { + // This mirrors 'WriteFile' in pkg/kptfile/kptfileutil/util.go + res, err := yaml.MarshalWithOptions(kpkg, &yaml.EncoderOptions{SeqIndent: yaml.WideSequenceStyle}) + if !assert.NoError(t, err) { return false } - return assert.Equal(t, res.String(), string(b)) + return assert.Equal(t, string(res), string(b)) } // CheckoutBranch checks out the git branch in the repo