From 7b0ed3562d8d68e57589ad652659c3baf5646268 Mon Sep 17 00:00:00 2001 From: Thomas Vitale Date: Sun, 26 Feb 2023 17:10:35 +0100 Subject: [PATCH 1/7] kctrl: Flag to create namespace when adding repo When adding a new package repository to a cluster, it's now possible to create the installation namespace automatically by specifying the "--create-namespace" flag. Fixes gh-1001 Signed-off-by: Thomas Vitale --- .../cmd/package/repository/add_or_update.go | 22 +++++++++++++++++++ cli/test/e2e/package_repository_test.go | 17 ++++++++++++++ 2 files changed, 39 insertions(+) diff --git a/cli/pkg/kctrl/cmd/package/repository/add_or_update.go b/cli/pkg/kctrl/cmd/package/repository/add_or_update.go index 672500b52..cf18fc732 100644 --- a/cli/pkg/kctrl/cmd/package/repository/add_or_update.go +++ b/cli/pkg/kctrl/cmd/package/repository/add_or_update.go @@ -19,6 +19,7 @@ import ( kcpkg "github.com/vmware-tanzu/carvel-kapp-controller/pkg/apis/packaging/v1alpha1" kcclient "github.com/vmware-tanzu/carvel-kapp-controller/pkg/client/clientset/versioned" versions "github.com/vmware-tanzu/carvel-vendir/pkg/vendir/versions/v1alpha1" + v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -33,6 +34,7 @@ type AddOrUpdateOptions struct { SecureNamespaceFlags cmdcore.SecureNamespaceFlags Name string URL string + CreateNamespace bool CreateRepository bool @@ -72,6 +74,8 @@ func NewAddCmd(o *AddOrUpdateOptions, flagsFactory cmdcore.FlagsFactory) *cobra. // TODO consider how to support other repository types cmd.Flags().StringVar(&o.URL, "url", "", "OCI registry url for package repository bundle (required)") + cmd.Flags().BoolVar(&o.CreateNamespace, "create-namespace", false, "Create the package repository namespace if not present (default false)") + o.WaitFlags.Set(cmd, flagsFactory, &cmdcore.WaitFlagsOpts{ AllowDisableWait: true, DefaultInterval: 1 * time.Second, @@ -141,6 +145,24 @@ func (o *AddOrUpdateOptions) Run(args []string) error { return err } + if o.CreateNamespace { + coreClient, err := o.depsFactory.CoreClient() + if err != nil { + return err + } + + namespace := &v1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: o.NamespaceFlags.Name, + }, + } + + _, err = coreClient.CoreV1().Namespaces().Create(context.Background(), namespace, metav1.CreateOptions{}) + if err != nil { + o.ui.PrintLinef("The namespace %s already exists", o.NamespaceFlags.Name) + } + } + existingRepository, err := client.PackagingV1alpha1().PackageRepositories(o.NamespaceFlags.Name).Get( context.Background(), o.Name, metav1.GetOptions{}) if err != nil { diff --git a/cli/test/e2e/package_repository_test.go b/cli/test/e2e/package_repository_test.go index b2a05a4fb..3bab8e18b 100644 --- a/cli/test/e2e/package_repository_test.go +++ b/cli/test/e2e/package_repository_test.go @@ -127,4 +127,21 @@ func TestPackageRepository(t *testing.T) { require.Exactly(t, expectedOutputRows, output.Tables[0].Rows) }) + logger.Section("creating a repository in a new namespace that doesn't exist", func() { + repoNamespace := "carvel-test-repos-a" + + kappCtrl.Run([]string{"package", "repository", "add", "-r", pkgrName, "--url", pkgrURL, "-n", repoNamespace, "--create-namespace"}) + + kubectl.Run([]string{"get", kind, pkgrName, "-n", repoNamespace}) + }) + + logger.Section("creating a repository in a namespace that already exists", func() { + repoNamespace := "carvel-test-repos-b" + kubectl.Run([]string{"create", "namespace", repoNamespace}) + + kappCtrl.Run([]string{"package", "repository", "add", "-r", pkgrName, "--url", pkgrURL, "-n", repoNamespace, "--create-namespace"}) + + kubectl.Run([]string{"get", kind, pkgrName, "-n", repoNamespace}) + }) + } From 4bf25d2b63d26730dfffa87d2bea05d5e8b4f295 Mon Sep 17 00:00:00 2001 From: Thomas Vitale Date: Wed, 1 Mar 2023 21:30:34 +0100 Subject: [PATCH 2/7] Improve error handling Signed-off-by: Thomas Vitale --- .../kctrl/cmd/package/repository/add_or_update.go | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/cli/pkg/kctrl/cmd/package/repository/add_or_update.go b/cli/pkg/kctrl/cmd/package/repository/add_or_update.go index cf18fc732..957741be0 100644 --- a/cli/pkg/kctrl/cmd/package/repository/add_or_update.go +++ b/cli/pkg/kctrl/cmd/package/repository/add_or_update.go @@ -19,7 +19,7 @@ import ( kcpkg "github.com/vmware-tanzu/carvel-kapp-controller/pkg/apis/packaging/v1alpha1" kcclient "github.com/vmware-tanzu/carvel-kapp-controller/pkg/client/clientset/versioned" versions "github.com/vmware-tanzu/carvel-vendir/pkg/vendir/versions/v1alpha1" - v1 "k8s.io/api/core/v1" + corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -146,12 +146,14 @@ func (o *AddOrUpdateOptions) Run(args []string) error { } if o.CreateNamespace { + o.statusUI.PrintMessagef("Creating namespace '%s'", o.NamespaceFlags.Name) + coreClient, err := o.depsFactory.CoreClient() if err != nil { return err } - namespace := &v1.Namespace{ + namespace := &corev1.Namespace{ ObjectMeta: metav1.ObjectMeta{ Name: o.NamespaceFlags.Name, }, @@ -159,7 +161,11 @@ func (o *AddOrUpdateOptions) Run(args []string) error { _, err = coreClient.CoreV1().Namespaces().Create(context.Background(), namespace, metav1.CreateOptions{}) if err != nil { - o.ui.PrintLinef("The namespace %s already exists", o.NamespaceFlags.Name) + if errors.IsAlreadyExists(err) { + o.statusUI.PrintMessagef("The namespace '%s' already exists", o.NamespaceFlags.Name) + } else { + return err + } } } @@ -184,7 +190,6 @@ func (o *AddOrUpdateOptions) Run(args []string) error { } if o.WaitFlags.Enabled { - o.ui.PrintLinef("Waiting for package repository to be updated") err = o.waitForPackageRepositoryInstallation(client) } From abc777ae3c67c2d88b3953824f32904d6409504a Mon Sep 17 00:00:00 2001 From: Thomas Vitale Date: Tue, 7 Mar 2023 17:30:20 +0100 Subject: [PATCH 3/7] Optimize status messages for namespace creation Signed-off-by: Thomas Vitale --- cli/pkg/kctrl/cmd/package/repository/add_or_update.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cli/pkg/kctrl/cmd/package/repository/add_or_update.go b/cli/pkg/kctrl/cmd/package/repository/add_or_update.go index 957741be0..2ca2eddf2 100644 --- a/cli/pkg/kctrl/cmd/package/repository/add_or_update.go +++ b/cli/pkg/kctrl/cmd/package/repository/add_or_update.go @@ -161,11 +161,11 @@ func (o *AddOrUpdateOptions) Run(args []string) error { _, err = coreClient.CoreV1().Namespaces().Create(context.Background(), namespace, metav1.CreateOptions{}) if err != nil { - if errors.IsAlreadyExists(err) { - o.statusUI.PrintMessagef("The namespace '%s' already exists", o.NamespaceFlags.Name) - } else { + if !errors.IsAlreadyExists(err) { return err } + } else { + o.statusUI.PrintMessagef("Created namespace '%s'", o.NamespaceFlags.Name) } } From 057bbdba0e8bf71facbd8a7a10812aaae7816124 Mon Sep 17 00:00:00 2001 From: Thomas Vitale Date: Thu, 30 Mar 2023 17:27:42 +0200 Subject: [PATCH 4/7] Add cleanup after tests Signed-off-by: Thomas Vitale --- .../cmd/package/repository/add_or_update.go | 2 -- cli/test/e2e/package_repository_test.go | 18 ++++++++++-------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/cli/pkg/kctrl/cmd/package/repository/add_or_update.go b/cli/pkg/kctrl/cmd/package/repository/add_or_update.go index 2ca2eddf2..559097ee7 100644 --- a/cli/pkg/kctrl/cmd/package/repository/add_or_update.go +++ b/cli/pkg/kctrl/cmd/package/repository/add_or_update.go @@ -146,8 +146,6 @@ func (o *AddOrUpdateOptions) Run(args []string) error { } if o.CreateNamespace { - o.statusUI.PrintMessagef("Creating namespace '%s'", o.NamespaceFlags.Name) - coreClient, err := o.depsFactory.CoreClient() if err != nil { return err diff --git a/cli/test/e2e/package_repository_test.go b/cli/test/e2e/package_repository_test.go index 3bab8e18b..1fc53b034 100644 --- a/cli/test/e2e/package_repository_test.go +++ b/cli/test/e2e/package_repository_test.go @@ -20,10 +20,15 @@ func TestPackageRepository(t *testing.T) { pkgrName := "test-package-repository" pkgrURL := `index.docker.io/k8slt/kc-e2e-test-repo:latest` + existingRepoNamespace := "carvel-test-repos-a" + newRepoNamespace := "carvel-test-repos-b" + kind := "PackageRepository" cleanUp := func() { RemoveClusterResource(t, kind, pkgrName, env.Namespace, kubectl) + kubectl.Run([]string{"delete", "namespace", existingRepoNamespace}) + kubectl.Run([]string{"delete", "namespace", newRepoNamespace}) } cleanUp() @@ -128,20 +133,17 @@ func TestPackageRepository(t *testing.T) { }) logger.Section("creating a repository in a new namespace that doesn't exist", func() { - repoNamespace := "carvel-test-repos-a" - - kappCtrl.Run([]string{"package", "repository", "add", "-r", pkgrName, "--url", pkgrURL, "-n", repoNamespace, "--create-namespace"}) + kappCtrl.Run([]string{"package", "repository", "add", "-r", pkgrName, "--url", pkgrURL, "-n", newRepoNamespace, "--create-namespace"}) - kubectl.Run([]string{"get", kind, pkgrName, "-n", repoNamespace}) + kubectl.Run([]string{"get", kind, pkgrName, "-n", newRepoNamespace}) }) logger.Section("creating a repository in a namespace that already exists", func() { - repoNamespace := "carvel-test-repos-b" - kubectl.Run([]string{"create", "namespace", repoNamespace}) + kubectl.Run([]string{"create", "namespace", existingRepoNamespace}) - kappCtrl.Run([]string{"package", "repository", "add", "-r", pkgrName, "--url", pkgrURL, "-n", repoNamespace, "--create-namespace"}) + kappCtrl.Run([]string{"package", "repository", "add", "-r", pkgrName, "--url", pkgrURL, "-n", existingRepoNamespace, "--create-namespace"}) - kubectl.Run([]string{"get", kind, pkgrName, "-n", repoNamespace}) + kubectl.Run([]string{"get", kind, pkgrName, "-n", existingRepoNamespace}) }) } From 82dac38b2d06fbb4fadca77bb169dfa60a992440 Mon Sep 17 00:00:00 2001 From: Thomas Vitale Date: Thu, 30 Mar 2023 17:58:45 +0200 Subject: [PATCH 5/7] Update test cleanup Signed-off-by: Thomas Vitale --- cli/test/e2e/package_repository_test.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/cli/test/e2e/package_repository_test.go b/cli/test/e2e/package_repository_test.go index 1fc53b034..7b08f29f2 100644 --- a/cli/test/e2e/package_repository_test.go +++ b/cli/test/e2e/package_repository_test.go @@ -27,8 +27,6 @@ func TestPackageRepository(t *testing.T) { cleanUp := func() { RemoveClusterResource(t, kind, pkgrName, env.Namespace, kubectl) - kubectl.Run([]string{"delete", "namespace", existingRepoNamespace}) - kubectl.Run([]string{"delete", "namespace", newRepoNamespace}) } cleanUp() @@ -136,6 +134,8 @@ func TestPackageRepository(t *testing.T) { kappCtrl.Run([]string{"package", "repository", "add", "-r", pkgrName, "--url", pkgrURL, "-n", newRepoNamespace, "--create-namespace"}) kubectl.Run([]string{"get", kind, pkgrName, "-n", newRepoNamespace}) + + RemoveClusterResource(t, kind, pkgrName, newRepoNamespace, kubectl) }) logger.Section("creating a repository in a namespace that already exists", func() { @@ -144,6 +144,8 @@ func TestPackageRepository(t *testing.T) { kappCtrl.Run([]string{"package", "repository", "add", "-r", pkgrName, "--url", pkgrURL, "-n", existingRepoNamespace, "--create-namespace"}) kubectl.Run([]string{"get", kind, pkgrName, "-n", existingRepoNamespace}) + + RemoveClusterResource(t, kind, pkgrName, existingRepoNamespace, kubectl) }) } From 94e6bdcfe669bbcb2a52d99ec831c25437f79306 Mon Sep 17 00:00:00 2001 From: Thomas Vitale Date: Wed, 5 Apr 2023 12:06:38 +0200 Subject: [PATCH 6/7] Use cleanup function for new tests Signed-off-by: Thomas Vitale --- cli/test/e2e/package_repository_test.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/cli/test/e2e/package_repository_test.go b/cli/test/e2e/package_repository_test.go index 7b08f29f2..79cf1ed0b 100644 --- a/cli/test/e2e/package_repository_test.go +++ b/cli/test/e2e/package_repository_test.go @@ -27,6 +27,8 @@ func TestPackageRepository(t *testing.T) { cleanUp := func() { RemoveClusterResource(t, kind, pkgrName, env.Namespace, kubectl) + RemoveClusterResource(t, kind, pkgrName, existingRepoNamespace, kubectl) + RemoveClusterResource(t, kind, pkgrName, newRepoNamespace, kubectl) } cleanUp() @@ -134,8 +136,6 @@ func TestPackageRepository(t *testing.T) { kappCtrl.Run([]string{"package", "repository", "add", "-r", pkgrName, "--url", pkgrURL, "-n", newRepoNamespace, "--create-namespace"}) kubectl.Run([]string{"get", kind, pkgrName, "-n", newRepoNamespace}) - - RemoveClusterResource(t, kind, pkgrName, newRepoNamespace, kubectl) }) logger.Section("creating a repository in a namespace that already exists", func() { @@ -144,8 +144,6 @@ func TestPackageRepository(t *testing.T) { kappCtrl.Run([]string{"package", "repository", "add", "-r", pkgrName, "--url", pkgrURL, "-n", existingRepoNamespace, "--create-namespace"}) kubectl.Run([]string{"get", kind, pkgrName, "-n", existingRepoNamespace}) - - RemoveClusterResource(t, kind, pkgrName, existingRepoNamespace, kubectl) }) } From 893f9e7beb0e8faa1892be52c9a07e706e7e8002 Mon Sep 17 00:00:00 2001 From: Thomas Vitale Date: Wed, 5 Apr 2023 12:57:09 +0200 Subject: [PATCH 7/7] Re-use existing namespace in test Signed-off-by: Thomas Vitale --- cli/test/e2e/package_repository_test.go | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/cli/test/e2e/package_repository_test.go b/cli/test/e2e/package_repository_test.go index 79cf1ed0b..75d5b5c61 100644 --- a/cli/test/e2e/package_repository_test.go +++ b/cli/test/e2e/package_repository_test.go @@ -20,14 +20,12 @@ func TestPackageRepository(t *testing.T) { pkgrName := "test-package-repository" pkgrURL := `index.docker.io/k8slt/kc-e2e-test-repo:latest` - existingRepoNamespace := "carvel-test-repos-a" - newRepoNamespace := "carvel-test-repos-b" + newRepoNamespace := "carvel-test-repo-a" kind := "PackageRepository" cleanUp := func() { RemoveClusterResource(t, kind, pkgrName, env.Namespace, kubectl) - RemoveClusterResource(t, kind, pkgrName, existingRepoNamespace, kubectl) RemoveClusterResource(t, kind, pkgrName, newRepoNamespace, kubectl) } @@ -139,11 +137,9 @@ func TestPackageRepository(t *testing.T) { }) logger.Section("creating a repository in a namespace that already exists", func() { - kubectl.Run([]string{"create", "namespace", existingRepoNamespace}) + kappCtrl.Run([]string{"package", "repository", "add", "-r", pkgrName, "--url", pkgrURL, "-n", env.Namespace, "--create-namespace"}) - kappCtrl.Run([]string{"package", "repository", "add", "-r", pkgrName, "--url", pkgrURL, "-n", existingRepoNamespace, "--create-namespace"}) - - kubectl.Run([]string{"get", kind, pkgrName, "-n", existingRepoNamespace}) + kubectl.Run([]string{"get", kind, pkgrName, "-n", env.Namespace}) }) }