make package install idempotent

Signed-off-by: Marjan Alavi <malavi@vmware.com>
vmware-tanzu · Oct 26, 2021 · 2abf5e8 · 2abf5e8
1 parent 89947b7
commit 2abf5e8
Show file tree

Hide file tree

Showing 19 changed files with 554 additions and 151 deletions.
diff --git a/cmd/cli/plugin/package/package_install.go b/cmd/cli/plugin/package/package_install.go
@@ -61,6 +61,10 @@ func packageInstall(cmd *cobra.Command, args []string) error {
 
 	initialMsg := fmt.Sprintf("Installing package '%s'", packageInstallOp.PackageName)
 	if err := DisplayProgress(initialMsg, pp); err != nil {
+		if err.Error() == tkgpackagedatamodel.ErrPackageAlreadyExists {
+			log.Infof("Updated installed package '%s'", packageInstallOp.PkgInstallName)
+			return nil
+		}
 		return err
 	}
 

diff --git a/cmd/cli/plugin/package/package_installed_delete.go b/cmd/cli/plugin/package/package_installed_delete.go
@@ -59,12 +59,12 @@ func packageUninstall(cmd *cobra.Command, args []string) error {
 	initialMsg := fmt.Sprintf("Uninstalling package '%s' from namespace '%s'", packageInstalledOp.PkgInstallName, packageInstalledOp.Namespace)
 	if err := DisplayProgress(initialMsg, pp); err != nil {
 		if err.Error() == tkgpackagedatamodel.ErrPackageNotInstalled {
-			log.Warningf("\npackage '%s' is not installed in namespace '%s'. Cleaned up related resources", packageInstalledOp.PkgInstallName, packageInstalledOp.Namespace)
+			log.Warningf("package '%s' is not installed in namespace '%s'.", packageInstalledOp.PkgInstallName, packageInstalledOp.Namespace)
 			return nil
 		}
 		return err
 	}
 
-	log.Infof("\n %s", fmt.Sprintf("Uninstalled package '%s' from namespace '%s'", packageInstalledOp.PkgInstallName, packageInstalledOp.Namespace))
+	log.Infof("%s", fmt.Sprintf("Uninstalled package '%s' from namespace '%s'", packageInstalledOp.PkgInstallName, packageInstalledOp.Namespace))
 	return nil
 }
diff --git a/cmd/cli/plugin/package/package_installed_update.go b/cmd/cli/plugin/package/package_installed_update.go
@@ -63,17 +63,17 @@ func packageUpdate(cmd *cobra.Command, args []string) error {
 		Err:         make(chan error),
 		Done:        make(chan struct{}),
 	}
-	go pkgClient.UpdatePackage(packageInstalledOp, pp)
+	go pkgClient.UpdatePackage(packageInstalledOp, pp, tkgpackagedatamodel.OperationTypeUpdate)
 
-	initialMsg := fmt.Sprintf("Updating package '%s'", packageInstalledOp.PkgInstallName)
+	initialMsg := fmt.Sprintf("Updating installed package '%s'", packageInstalledOp.PkgInstallName)
 	if err := DisplayProgress(initialMsg, pp); err != nil {
 		if err.Error() == tkgpackagedatamodel.ErrPackageNotInstalled {
-			log.Warningf("\npackage '%s' is not among the list of installed packages in namespace '%s'. Consider using the --install flag to install the package", packageInstalledOp.PkgInstallName, packageInstalledOp.Namespace)
+			log.Warningf("package '%s' is not among the list of installed packages in namespace '%s'. Consider using the --install flag to install the package", packageInstalledOp.PkgInstallName, packageInstalledOp.Namespace)
 			return nil
 		}
 		return err
 	}
 
-	log.Infof("\n %s", fmt.Sprintf("Updated package install '%s' in namespace '%s'", packageInstalledOp.PkgInstallName, packageInstalledOp.Namespace))
+	log.Infof("%s", fmt.Sprintf("Updated installed package '%s' in namespace '%s'", packageInstalledOp.PkgInstallName, packageInstalledOp.Namespace))
 	return nil
 }
diff --git a/cmd/cli/plugin/package/repository_add.go b/cmd/cli/plugin/package/repository_add.go
@@ -34,7 +34,7 @@ func init() {
 	repositoryCmd.AddCommand(repositoryAddCmd)
 }
 
-func repositoryAdd(cmd *cobra.Command, args []string) error {
+func repositoryAdd(_ *cobra.Command, args []string) error { //nolint
 	repoOp.RepositoryName = args[0]
 
 	pkgClient, err := tkgpackageclient.NewTKGPackageClient(repoOp.KubeConfig)
@@ -51,9 +51,13 @@ func repositoryAdd(cmd *cobra.Command, args []string) error {
 
 	initialMsg := fmt.Sprintf("Adding package repository '%s'", repoOp.RepositoryName)
 	if err := DisplayProgress(initialMsg, pp); err != nil {
+		if err.Error() == tkgpackagedatamodel.ErrRepoAlreadyExists {
+			log.Infof("Updated package repository '%s' in namespace '%s'", repoOp.RepositoryName, repoOp.Namespace)
+			return nil
+		}
 		return err
 	}
 
-	log.Infof("\n Added package repository '%s'", repoOp.RepositoryName)
+	log.Infof("Added package repository '%s' in namespace '%s'", repoOp.RepositoryName, repoOp.Namespace)
 	return nil
 }
diff --git a/cmd/cli/plugin/package/repository_delete.go b/cmd/cli/plugin/package/repository_delete.go
@@ -65,12 +65,12 @@ func repositoryDelete(cmd *cobra.Command, args []string) error {
 	initialMsg := fmt.Sprintf("Deleting package repository '%s'", repoOp.RepositoryName)
 	if err := DisplayProgress(initialMsg, pp); err != nil {
 		if err.Error() == tkgpackagedatamodel.ErrRepoNotExists {
-			log.Warningf("\npackage repository '%s' does not exist in namespace '%s'", repoOp.RepositoryName, repoOp.Namespace)
+			log.Warningf("package repository '%s' does not exist in namespace '%s'", repoOp.RepositoryName, repoOp.Namespace)
 			return nil
 		}
 		return err
 	}
 
-	log.Infof("\n Deleted package repository '%s' from namespace '%s'", repoOp.RepositoryName, repoOp.Namespace)
+	log.Infof("Deleted package repository '%s' from namespace '%s'", repoOp.RepositoryName, repoOp.Namespace)
 	return nil
 }
diff --git a/cmd/cli/plugin/package/repository_update.go b/cmd/cli/plugin/package/repository_update.go
@@ -35,7 +35,7 @@ func init() {
 	repositoryCmd.AddCommand(repositoryUpdateCmd)
 }
 
-func repositoryUpdate(cmd *cobra.Command, args []string) error {
+func repositoryUpdate(_ *cobra.Command, args []string) error { //nolint
 	repoOp.RepositoryName = args[0]
 
 	pkgClient, err := tkgpackageclient.NewTKGPackageClient(repoOp.KubeConfig)
@@ -48,17 +48,17 @@ func repositoryUpdate(cmd *cobra.Command, args []string) error {
 		Err:         make(chan error),
 		Done:        make(chan struct{}),
 	}
-	go pkgClient.UpdateRepository(repoOp, pp)
+	go pkgClient.UpdateRepository(repoOp, pp, tkgpackagedatamodel.OperationTypeUpdate)
 
 	initialMsg := fmt.Sprintf("Updating package repository '%s'", repoOp.RepositoryName)
 	if err := DisplayProgress(initialMsg, pp); err != nil {
 		if err.Error() == tkgpackagedatamodel.ErrRepoNotExists {
-			log.Warningf("\npackage repository '%s' does not exist in namespace '%s'. Consider using the --create flag to add the package repository", repoOp.RepositoryName, repoOp.Namespace)
+			log.Warningf("package repository '%s' does not exist in namespace '%s'. Consider using the --create flag to add the package repository", repoOp.RepositoryName, repoOp.Namespace)
 			return nil
 		}
 		return err
 	}
 
-	log.Infof("\n Updated package repository '%s' in namespace '%s'", repoOp.RepositoryName, repoOp.Namespace)
+	log.Infof("Updated package repository '%s' in namespace '%s'", repoOp.RepositoryName, repoOp.Namespace)
 	return nil
 }
diff --git a/pkg/v1/providers/config_default.yaml b/pkg/v1/providers/config_default.yaml
@@ -302,7 +302,7 @@ AZURE_ENABLE_ACCELERATED_NETWORKING: false
 
 #! Must be unique to each cluster.
 AZURE_RESOURCE_GROUP: ""
-#! If unset the value of AZURE_RESOURCE_GROUP will be used as the resoure group
+#! If unset the value of AZURE_RESOURCE_GROUP will be used as the resource group
 #! for the VNET
 AZURE_VNET_RESOURCE_GROUP: ""
 

diff --git a/pkg/v1/tkg/tkgpackageclient/interface.go b/pkg/v1/tkg/tkgpackageclient/interface.go
@@ -32,6 +32,6 @@ type TKGPackageClient interface {
 	ListRepositories(o *tkgpackagedatamodel.RepositoryOptions) (*kappipkg.PackageRepositoryList, error)
 	UninstallPackage(o *tkgpackagedatamodel.PackageOptions, packageProgress *tkgpackagedatamodel.PackageProgress)
 	UpdateRegistrySecret(o *tkgpackagedatamodel.RegistrySecretOptions) error
-	UpdatePackage(o *tkgpackagedatamodel.PackageOptions, packageProgress *tkgpackagedatamodel.PackageProgress)
-	UpdateRepository(o *tkgpackagedatamodel.RepositoryOptions, progress *tkgpackagedatamodel.PackageProgress)
+	UpdatePackage(o *tkgpackagedatamodel.PackageOptions, packageProgress *tkgpackagedatamodel.PackageProgress, operationType tkgpackagedatamodel.OperationType)
+	UpdateRepository(o *tkgpackagedatamodel.RepositoryOptions, progress *tkgpackagedatamodel.PackageProgress, operationType tkgpackagedatamodel.OperationType)
 }
diff --git a/pkg/v1/tkg/tkgpackageclient/package_install.go b/pkg/v1/tkg/tkgpackageclient/package_install.go
@@ -27,12 +27,11 @@ import (
 )
 
 const (
-	msgRunPackageInstalledDelete = "\n\nPlease consider using 'tanzu package installed delete' to delete the already created associated resources\n"
 	msgRunPackageInstalledUpdate = "\n\nPlease consider using 'tanzu package installed update' to update the installed package with correct settings\n"
 )
 
 // InstallPackage installs the PackageInstall and its associated resources in the cluster
-func (p *pkgClient) InstallPackage(o *tkgpackagedatamodel.PackageOptions, progress *tkgpackagedatamodel.PackageProgress, operationType tkgpackagedatamodel.OperationType) { //nolint:gocyclo
+func (p *pkgClient) InstallPackage(o *tkgpackagedatamodel.PackageOptions, progress *tkgpackagedatamodel.PackageProgress, operationType tkgpackagedatamodel.OperationType) {
 	var (
 		pkgInstall            *kappipkg.PackageInstall
 		err                   error
@@ -57,8 +56,10 @@ func (p *pkgClient) InstallPackage(o *tkgpackagedatamodel.PackageOptions, progre
 		err = nil
 	}
 
-	if pkgInstall != nil && pkgInstall.Name == o.PkgInstallName {
-		err = errors.New(fmt.Sprintf("package install '%s' already exists in namespace '%s'", o.PkgInstallName, o.Namespace))
+	if pkgInstall != nil {
+		progress.ProgressMsg <- fmt.Sprintf("Updating package '%s'", o.PkgInstallName)
+		p.UpdatePackage(o, progress, tkgpackagedatamodel.OperationTypeInstall)
+		err = &tkgpackagedatamodel.PackagePluginNonCriticalError{Reason: tkgpackagedatamodel.ErrPackageAlreadyExists}
 		return
 	}
 
@@ -76,64 +77,76 @@ func (p *pkgClient) InstallPackage(o *tkgpackagedatamodel.PackageOptions, progre
 		return
 	}
 
+	if serviceAccountCreated, secretCreated, err = p.createRelatedResources(o, progress.ProgressMsg); err != nil {
+		return
+	}
+
+	progress.ProgressMsg <- "Creating package resource"
+	if err = p.createPackageInstall(o, serviceAccountCreated, secretCreated); err != nil {
+		return
+	}
+
+	if o.Wait {
+		if err = p.waitForResourceInstallation(o.PkgInstallName, o.Namespace, o.PollInterval, o.PollTimeout, progress.ProgressMsg, tkgpackagedatamodel.ResourceTypePackageInstall); err != nil {
+			log.Warning(msgRunPackageInstalledUpdate)
+			return
+		}
+	}
+}
+
+func (p *pkgClient) createRelatedResources(o *tkgpackagedatamodel.PackageOptions, progress chan string) (bool, bool, error) {
+	var (
+		secretCreated         bool
+		serviceAccountCreated bool
+		err                   error
+	)
+
 	if o.ServiceAccountName == "" {
 		o.ServiceAccountName = fmt.Sprintf(tkgpackagedatamodel.ServiceAccountName, o.PkgInstallName, o.Namespace)
-		progress.ProgressMsg <- fmt.Sprintf("Creating service account '%s'", o.ServiceAccountName)
-		if serviceAccountCreated, err = p.createServiceAccount(o); err != nil {
-			return
+		progress <- fmt.Sprintf("Creating service account '%s'", o.ServiceAccountName)
+		if serviceAccountCreated, err = p.createOrUpdateServiceAccount(o); err != nil {
+			return serviceAccountCreated, secretCreated, err
 		}
 
 		o.ClusterRoleName = fmt.Sprintf(tkgpackagedatamodel.ClusterRoleName, o.PkgInstallName, o.Namespace)
-		progress.ProgressMsg <- fmt.Sprintf("Creating cluster admin role '%s'", o.ClusterRoleName)
-		if err = p.createClusterAdminRole(o); err != nil {
-			log.Warning(msgRunPackageInstalledDelete)
-			return
+		progress <- fmt.Sprintf("Creating cluster admin role '%s'", o.ClusterRoleName)
+		if err := p.createOrUpdateClusterAdminRole(o); err != nil {
+			return serviceAccountCreated, secretCreated, err
 		}
 
 		o.ClusterRoleBindingName = fmt.Sprintf(tkgpackagedatamodel.ClusterRoleBindingName, o.PkgInstallName, o.Namespace)
-		progress.ProgressMsg <- fmt.Sprintf("Creating cluster role binding '%s'", o.ClusterRoleBindingName)
-		if err = p.createClusterRoleBinding(o); err != nil {
-			log.Warning(msgRunPackageInstalledDelete)
-			return
+		progress <- fmt.Sprintf("Creating cluster role binding '%s'", o.ClusterRoleBindingName)
+		if err := p.createOrUpdateClusterRoleBinding(o); err != nil {
+			return serviceAccountCreated, secretCreated, err
 		}
 	} else {
 		objKey := crtclient.ObjectKey{Name: o.ServiceAccountName, Namespace: o.Namespace}
 		svcAccount := &corev1.ServiceAccount{}
 		if err = p.kappClient.GetClient().Get(context.Background(), objKey, svcAccount); err != nil {
 			err = errors.Wrap(err, fmt.Sprintf("failed to find service account '%s' in namespace '%s'", o.ServiceAccountName, o.Namespace))
-			return
+			return serviceAccountCreated, secretCreated, err
 		}
-		if _, ok := svcAccount.GetAnnotations()[tkgpackagedatamodel.TanzuPkgPluginAnnotation]; ok {
-			err = errors.New(fmt.Sprintf("provided service account '%s' is already used by another package in namespace '%s'", o.ServiceAccountName, o.Namespace))
-			return
+		if svcAccountAnnotation, ok := svcAccount.GetAnnotations()[tkgpackagedatamodel.TanzuPkgPluginAnnotation]; ok {
+			if svcAccountAnnotation != fmt.Sprintf(tkgpackagedatamodel.TanzuPkgPluginResource, o.PkgInstallName, o.Namespace) {
+				err = errors.New(fmt.Sprintf("provided service account '%s' is already used by another package in namespace '%s'", o.ServiceAccountName, o.Namespace))
+				return serviceAccountCreated, secretCreated, err
+			}
 		}
 	}
 
 	if o.ValuesFile != "" {
 		o.SecretName = fmt.Sprintf(tkgpackagedatamodel.SecretName, o.PkgInstallName, o.Namespace)
-		progress.ProgressMsg <- fmt.Sprintf("Creating secret '%s'", o.SecretName)
-		if secretCreated, err = p.createDataValuesSecret(o); err != nil {
-			log.Warning(msgRunPackageInstalledDelete)
-			return
+		progress <- fmt.Sprintf("Creating secret '%s'", o.SecretName)
+		if secretCreated, err = p.createOrUpdateDataValuesSecret(o); err != nil {
+			return serviceAccountCreated, secretCreated, err
 		}
 	}
 
-	progress.ProgressMsg <- "Creating package resource"
-	if err = p.createPackageInstall(o, serviceAccountCreated, secretCreated); err != nil {
-		log.Warning(msgRunPackageInstalledDelete)
-		return
-	}
-
-	if o.Wait {
-		if err = p.waitForResourceInstallation(o.PkgInstallName, o.Namespace, o.PollInterval, o.PollTimeout, progress.ProgressMsg, tkgpackagedatamodel.ResourceTypePackageInstall); err != nil {
-			log.Warning(msgRunPackageInstalledUpdate)
-			return
-		}
-	}
+	return serviceAccountCreated, secretCreated, nil
 }
 
-// createClusterAdminRole creates a ClusterRole resource
-func (p *pkgClient) createClusterAdminRole(o *tkgpackagedatamodel.PackageOptions) error {
+// createOrUpdateClusterAdminRole creates or updates a ClusterRole resource
+func (p *pkgClient) createOrUpdateClusterAdminRole(o *tkgpackagedatamodel.PackageOptions) error {
 	clusterRole := &rbacv1.ClusterRole{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:        o.ClusterRoleName,
@@ -145,14 +158,20 @@ func (p *pkgClient) createClusterAdminRole(o *tkgpackagedatamodel.PackageOptions
 	}
 
 	if err := p.kappClient.GetClient().Create(context.Background(), clusterRole); err != nil {
-		return errors.Wrap(err, "failed to create ClusterRole resource")
+		if k8serror.IsAlreadyExists(err) {
+			if err := p.kappClient.GetClient().Update(context.Background(), clusterRole); err != nil {
+				return err
+			}
+		} else {
+			return err
+		}
 	}
 
 	return nil
 }
 
-// createClusterRoleBinding creates a ClusterRoleBinding resource
-func (p *pkgClient) createClusterRoleBinding(o *tkgpackagedatamodel.PackageOptions) error {
+// createOrUpdateClusterRoleBinding creates or updates a ClusterRoleBinding resource
+func (p *pkgClient) createOrUpdateClusterRoleBinding(o *tkgpackagedatamodel.PackageOptions) error {
 	clusterRoleBinding := &rbacv1.ClusterRoleBinding{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:        o.ClusterRoleBindingName,
@@ -167,14 +186,20 @@ func (p *pkgClient) createClusterRoleBinding(o *tkgpackagedatamodel.PackageOptio
 	}
 
 	if err := p.kappClient.GetClient().Create(context.Background(), clusterRoleBinding); err != nil {
-		return errors.Wrap(err, "failed to create ClusterRoleBinding resource")
+		if k8serror.IsAlreadyExists(err) {
+			if err := p.kappClient.GetClient().Update(context.Background(), clusterRoleBinding); err != nil {
+				return err
+			}
+		} else {
+			return err
+		}
 	}
 
 	return nil
 }
 
-// createDataValuesSecret create a secret object containing the user-provided configuration.
-func (p *pkgClient) createDataValuesSecret(o *tkgpackagedatamodel.PackageOptions) (bool, error) {
+// createOrUpdateDataValuesSecret create or updates a secret object containing the user-provided configuration.
+func (p *pkgClient) createOrUpdateDataValuesSecret(o *tkgpackagedatamodel.PackageOptions) (bool, error) {
 	var err error
 
 	dataValues := make(map[string][]byte)
@@ -192,7 +217,13 @@ func (p *pkgClient) createDataValuesSecret(o *tkgpackagedatamodel.PackageOptions
 	}
 
 	if err := p.kappClient.GetClient().Create(context.Background(), secret); err != nil {
-		return false, errors.Wrap(err, "failed to create Secret resource")
+		if k8serror.IsAlreadyExists(err) {
+			if err := p.kappClient.GetClient().Update(context.Background(), secret); err != nil {
+				return false, err
+			}
+		} else {
+			return false, err
+		}
 	}
 
 	return true, nil
@@ -255,8 +286,8 @@ func (p *pkgClient) createPackageInstall(o *tkgpackagedatamodel.PackageOptions,
 	return nil
 }
 
-// createServiceAccount creates a ServiceAccount resource
-func (p *pkgClient) createServiceAccount(o *tkgpackagedatamodel.PackageOptions) (bool, error) {
+// createOrUpdateServiceAccount creates or updates a ServiceAccount resource
+func (p *pkgClient) createOrUpdateServiceAccount(o *tkgpackagedatamodel.PackageOptions) (bool, error) {
 	serviceAccount := &corev1.ServiceAccount{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:        o.ServiceAccountName,
@@ -265,7 +296,13 @@ func (p *pkgClient) createServiceAccount(o *tkgpackagedatamodel.PackageOptions)
 	}
 
 	if err := p.kappClient.GetClient().Create(context.Background(), serviceAccount); err != nil {
-		return false, errors.Wrap(err, "failed to create ServiceAccount resource")
+		if k8serror.IsAlreadyExists(err) {
+			if err := p.kappClient.GetClient().Update(context.Background(), serviceAccount); err != nil {
+				return false, err
+			}
+		} else {
+			return false, err
+		}
 	}
 
 	return true, nil