diff --git a/cmd/cli/plugin/package/package_install.go b/cmd/cli/plugin/package/package_install.go index 4a715fdb62..dbe491f590 100644 --- 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 index 2fa9223b90..edbfdaa690 100644 --- 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 index eb889abc6c..60469f3638 100644 --- 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 index fad0db1055..fa21fb210a 100644 --- 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 index a75fc85eed..9ef8f25085 100644 --- 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 index 4e7d826abf..2c3337f6ae 100644 --- 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 index 2a9b794605..2c5c80ecd0 100644 --- 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/fakes/kappclient.go b/pkg/v1/tkg/fakes/kappclient.go index 341b6c927a..db7b1d07e6 100644 --- a/pkg/v1/tkg/fakes/kappclient.go +++ b/pkg/v1/tkg/fakes/kappclient.go @@ -12,15 +12,15 @@ import ( v1alpha1b "github.com/vmware-tanzu/carvel-kapp-controller/pkg/apiserver/apis/datapackaging/v1alpha1" v1alpha1c "github.com/vmware-tanzu/carvel-secretgen-controller/pkg/apis/secretgen2/v1alpha1" "github.com/vmware-tanzu/tanzu-framework/pkg/v1/tkg/kappclient" + "github.com/vmware-tanzu/tanzu-framework/pkg/v1/tkg/tkgpackagedatamodel" ) type KappClient struct { - CreatePackageInstallStub func(*v1alpha1.PackageInstall, bool, bool) error + CreatePackageInstallStub func(*v1alpha1.PackageInstall, *tkgpackagedatamodel.PkgPluginResourceCreationStatus) error createPackageInstallMutex sync.RWMutex createPackageInstallArgsForCall []struct { arg1 *v1alpha1.PackageInstall - arg2 bool - arg3 bool + arg2 *tkgpackagedatamodel.PkgPluginResourceCreationStatus } createPackageInstallReturns struct { result1 error @@ -223,11 +223,11 @@ type KappClient struct { result1 *v1alpha1c.SecretExportList result2 error } - UpdatePackageInstallStub func(*v1alpha1.PackageInstall, bool) error + UpdatePackageInstallStub func(*v1alpha1.PackageInstall, *tkgpackagedatamodel.PkgPluginResourceCreationStatus) error updatePackageInstallMutex sync.RWMutex updatePackageInstallArgsForCall []struct { arg1 *v1alpha1.PackageInstall - arg2 bool + arg2 *tkgpackagedatamodel.PkgPluginResourceCreationStatus } updatePackageInstallReturns struct { result1 error @@ -250,18 +250,17 @@ type KappClient struct { invocationsMutex sync.RWMutex } -func (fake *KappClient) CreatePackageInstall(arg1 *v1alpha1.PackageInstall, arg2 bool, arg3 bool) error { +func (fake *KappClient) CreatePackageInstall(arg1 *v1alpha1.PackageInstall, arg2 *tkgpackagedatamodel.PkgPluginResourceCreationStatus) error { fake.createPackageInstallMutex.Lock() ret, specificReturn := fake.createPackageInstallReturnsOnCall[len(fake.createPackageInstallArgsForCall)] fake.createPackageInstallArgsForCall = append(fake.createPackageInstallArgsForCall, struct { arg1 *v1alpha1.PackageInstall - arg2 bool - arg3 bool - }{arg1, arg2, arg3}) - fake.recordInvocation("CreatePackageInstall", []interface{}{arg1, arg2, arg3}) + arg2 *tkgpackagedatamodel.PkgPluginResourceCreationStatus + }{arg1, arg2}) + fake.recordInvocation("CreatePackageInstall", []interface{}{arg1, arg2}) fake.createPackageInstallMutex.Unlock() if fake.CreatePackageInstallStub != nil { - return fake.CreatePackageInstallStub(arg1, arg2, arg3) + return fake.CreatePackageInstallStub(arg1, arg2) } if specificReturn { return ret.result1 @@ -276,17 +275,17 @@ func (fake *KappClient) CreatePackageInstallCallCount() int { return len(fake.createPackageInstallArgsForCall) } -func (fake *KappClient) CreatePackageInstallCalls(stub func(*v1alpha1.PackageInstall, bool, bool) error) { +func (fake *KappClient) CreatePackageInstallCalls(stub func(*v1alpha1.PackageInstall, *tkgpackagedatamodel.PkgPluginResourceCreationStatus) error) { fake.createPackageInstallMutex.Lock() defer fake.createPackageInstallMutex.Unlock() fake.CreatePackageInstallStub = stub } -func (fake *KappClient) CreatePackageInstallArgsForCall(i int) (*v1alpha1.PackageInstall, bool, bool) { +func (fake *KappClient) CreatePackageInstallArgsForCall(i int) (*v1alpha1.PackageInstall, *tkgpackagedatamodel.PkgPluginResourceCreationStatus) { fake.createPackageInstallMutex.RLock() defer fake.createPackageInstallMutex.RUnlock() argsForCall := fake.createPackageInstallArgsForCall[i] - return argsForCall.arg1, argsForCall.arg2, argsForCall.arg3 + return argsForCall.arg1, argsForCall.arg2 } func (fake *KappClient) CreatePackageInstallReturns(result1 error) { @@ -1247,12 +1246,12 @@ func (fake *KappClient) ListSecretExportsReturnsOnCall(i int, result1 *v1alpha1c }{result1, result2} } -func (fake *KappClient) UpdatePackageInstall(arg1 *v1alpha1.PackageInstall, arg2 bool) error { +func (fake *KappClient) UpdatePackageInstall(arg1 *v1alpha1.PackageInstall, arg2 *tkgpackagedatamodel.PkgPluginResourceCreationStatus) error { fake.updatePackageInstallMutex.Lock() ret, specificReturn := fake.updatePackageInstallReturnsOnCall[len(fake.updatePackageInstallArgsForCall)] fake.updatePackageInstallArgsForCall = append(fake.updatePackageInstallArgsForCall, struct { arg1 *v1alpha1.PackageInstall - arg2 bool + arg2 *tkgpackagedatamodel.PkgPluginResourceCreationStatus }{arg1, arg2}) fake.recordInvocation("UpdatePackageInstall", []interface{}{arg1, arg2}) fake.updatePackageInstallMutex.Unlock() @@ -1272,13 +1271,13 @@ func (fake *KappClient) UpdatePackageInstallCallCount() int { return len(fake.updatePackageInstallArgsForCall) } -func (fake *KappClient) UpdatePackageInstallCalls(stub func(*v1alpha1.PackageInstall, bool) error) { +func (fake *KappClient) UpdatePackageInstallCalls(stub func(*v1alpha1.PackageInstall, *tkgpackagedatamodel.PkgPluginResourceCreationStatus) error) { fake.updatePackageInstallMutex.Lock() defer fake.updatePackageInstallMutex.Unlock() fake.UpdatePackageInstallStub = stub } -func (fake *KappClient) UpdatePackageInstallArgsForCall(i int) (*v1alpha1.PackageInstall, bool) { +func (fake *KappClient) UpdatePackageInstallArgsForCall(i int) (*v1alpha1.PackageInstall, *tkgpackagedatamodel.PkgPluginResourceCreationStatus) { fake.updatePackageInstallMutex.RLock() defer fake.updatePackageInstallMutex.RUnlock() argsForCall := fake.updatePackageInstallArgsForCall[i] diff --git a/pkg/v1/tkg/kappclient/client.go b/pkg/v1/tkg/kappclient/client.go index da06f548f1..ccbf625803 100644 --- a/pkg/v1/tkg/kappclient/client.go +++ b/pkg/v1/tkg/kappclient/client.go @@ -105,16 +105,16 @@ func GetKubeConfig(kubeCfgPath string) (*rest.Config, error) { return restConfig, nil } -func (c *client) addAnnotations(meta *v1.ObjectMeta, isPkgPluginCreatedSvcAccount, isPkgPluginCreatedSecret bool) { +func (c *client) addAnnotations(meta *v1.ObjectMeta, pkgPluginResourceCreationStatus *tkgpackagedatamodel.PkgPluginResourceCreationStatus) { if meta.Annotations == nil { meta.Annotations = make(map[string]string) } - if isPkgPluginCreatedSvcAccount { + if pkgPluginResourceCreationStatus.IsServiceAccountCreated { meta.Annotations[tkgpackagedatamodel.TanzuPkgPluginAnnotation+"-"+tkgpackagedatamodel.KindClusterRole] = fmt.Sprintf(tkgpackagedatamodel.ClusterRoleName, meta.Name, meta.Namespace) meta.Annotations[tkgpackagedatamodel.TanzuPkgPluginAnnotation+"-"+tkgpackagedatamodel.KindClusterRoleBinding] = fmt.Sprintf(tkgpackagedatamodel.ClusterRoleBindingName, meta.Name, meta.Namespace) meta.Annotations[tkgpackagedatamodel.TanzuPkgPluginAnnotation+"-"+tkgpackagedatamodel.KindServiceAccount] = fmt.Sprintf(tkgpackagedatamodel.ServiceAccountName, meta.Name, meta.Namespace) } - if isPkgPluginCreatedSecret { + if pkgPluginResourceCreationStatus.IsSecretCreated { meta.Annotations[tkgpackagedatamodel.TanzuPkgPluginAnnotation+"-"+tkgpackagedatamodel.KindSecret] = fmt.Sprintf(tkgpackagedatamodel.SecretName, meta.Name, meta.Namespace) } } @@ -138,9 +138,9 @@ func (c *client) DeletePackageRepository(repository *kappipkg.PackageRepository) } // CreatePackageInstall creates a PackageInstall CR -func (c *client) CreatePackageInstall(packageInstall *kappipkg.PackageInstall, isPkgPluginCreatedSvcAccount, isPkgPluginCreatedSecret bool) error { +func (c *client) CreatePackageInstall(packageInstall *kappipkg.PackageInstall, pkgPluginResourceCreationStatus *tkgpackagedatamodel.PkgPluginResourceCreationStatus) error { installedPkg := packageInstall.DeepCopy() - c.addAnnotations(&installedPkg.ObjectMeta, isPkgPluginCreatedSvcAccount, isPkgPluginCreatedSecret) + c.addAnnotations(&installedPkg.ObjectMeta, pkgPluginResourceCreationStatus) if err := c.client.Create(context.Background(), installedPkg); err != nil { return err @@ -280,8 +280,8 @@ func (c *client) ListPackages(packageName, namespace string) (*kapppkg.PackageLi } // UpdatePackageInstall updates the PackageInstall CR -func (c *client) UpdatePackageInstall(packageInstall *kappipkg.PackageInstall, isPkgPluginCreatedSecret bool) error { - c.addAnnotations(&packageInstall.ObjectMeta, false, isPkgPluginCreatedSecret) +func (c *client) UpdatePackageInstall(packageInstall *kappipkg.PackageInstall, pkgPluginResourceCreationStatus *tkgpackagedatamodel.PkgPluginResourceCreationStatus) error { + c.addAnnotations(&packageInstall.ObjectMeta, pkgPluginResourceCreationStatus) if err := c.client.Update(context.Background(), packageInstall); err != nil { return err diff --git a/pkg/v1/tkg/kappclient/interface.go b/pkg/v1/tkg/kappclient/interface.go index aa7abb8ed1..2cc3e40b41 100644 --- a/pkg/v1/tkg/kappclient/interface.go +++ b/pkg/v1/tkg/kappclient/interface.go @@ -12,13 +12,14 @@ import ( kappipkg "github.com/vmware-tanzu/carvel-kapp-controller/pkg/apis/packaging/v1alpha1" kapppkg "github.com/vmware-tanzu/carvel-kapp-controller/pkg/apiserver/apis/datapackaging/v1alpha1" secretgen "github.com/vmware-tanzu/carvel-secretgen-controller/pkg/apis/secretgen2/v1alpha1" + "github.com/vmware-tanzu/tanzu-framework/pkg/v1/tkg/tkgpackagedatamodel" ) //go:generate counterfeiter -o ../fakes/kappclient.go --fake-name KappClient . Client // Client is the kapp client interface type Client interface { - CreatePackageInstall(packageInstall *kappipkg.PackageInstall, isPkgPluginCreatedSvcAccount bool, isPkgPluginCreatedSecret bool) error + CreatePackageInstall(packageInstall *kappipkg.PackageInstall, pkgPluginResourceCreationStatus *tkgpackagedatamodel.PkgPluginResourceCreationStatus) error CreatePackageRepository(repository *kappipkg.PackageRepository) error DeletePackageRepository(repository *kappipkg.PackageRepository) error GetAppCR(appName string, namespace string) (*kappctrl.App, error) @@ -34,6 +35,6 @@ type Client interface { ListPackageRepositories(namespace string) (*kappipkg.PackageRepositoryList, error) ListRegistrySecrets(namespace string) (*corev1.SecretList, error) ListSecretExports(namespace string) (*secretgen.SecretExportList, error) - UpdatePackageInstall(packageInstall *kappipkg.PackageInstall, isPkgPluginCreatedSecret bool) error + UpdatePackageInstall(packageInstall *kappipkg.PackageInstall, pkgPluginResourceCreationStatus *tkgpackagedatamodel.PkgPluginResourceCreationStatus) error UpdatePackageRepository(repository *kappipkg.PackageRepository) error } diff --git a/pkg/v1/tkg/tkgpackageclient/interface.go b/pkg/v1/tkg/tkgpackageclient/interface.go index e1f81d1710..4c6b0bd5cb 100644 --- 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 index 3ca09e7b49..0241a56ec5 100644 --- a/pkg/v1/tkg/tkgpackageclient/package_install.go +++ b/pkg/v1/tkg/tkgpackageclient/package_install.go @@ -27,17 +27,15 @@ 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 - secretCreated bool - serviceAccountCreated bool + pkgInstall *kappipkg.PackageInstall + pkgPluginResourceCreationStatus *tkgpackagedatamodel.PkgPluginResourceCreationStatus + err error ) defer func() { @@ -57,8 +55,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 +76,75 @@ func (p *pkgClient) InstallPackage(o *tkgpackagedatamodel.PackageOptions, progre return } + if pkgPluginResourceCreationStatus, err = p.createRelatedResources(o, progress.ProgressMsg); err != nil { + return + } + + progress.ProgressMsg <- "Creating package resource" + if err = p.createPackageInstall(o, pkgPluginResourceCreationStatus); 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) (*tkgpackagedatamodel.PkgPluginResourceCreationStatus, error) { + var ( + pkgPluginResourceCreationStatus tkgpackagedatamodel.PkgPluginResourceCreationStatus + 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 pkgPluginResourceCreationStatus.IsServiceAccountCreated, err = p.createOrUpdateServiceAccount(o); err != nil { + return &pkgPluginResourceCreationStatus, 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 &pkgPluginResourceCreationStatus, 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 &pkgPluginResourceCreationStatus, 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 &pkgPluginResourceCreationStatus, 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 &pkgPluginResourceCreationStatus, 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 pkgPluginResourceCreationStatus.IsSecretCreated, err = p.createOrUpdateDataValuesSecret(o); err != nil { + return &pkgPluginResourceCreationStatus, 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 &pkgPluginResourceCreationStatus, 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 +156,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 +184,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 +215,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 @@ -221,7 +250,7 @@ func (p *pkgClient) createNamespace(namespace string) error { } // createPackageInstall creates the PackageInstall CR -func (p *pkgClient) createPackageInstall(o *tkgpackagedatamodel.PackageOptions, serviceAccountCreated, secretCreated bool) error { +func (p *pkgClient) createPackageInstall(o *tkgpackagedatamodel.PackageOptions, pkgPluginResourceCreationStatus *tkgpackagedatamodel.PkgPluginResourceCreationStatus) error { // construct the PackageInstall CR packageInstall := &kappipkg.PackageInstall{ ObjectMeta: metav1.ObjectMeta{Name: o.PkgInstallName, Namespace: o.Namespace}, @@ -238,7 +267,7 @@ func (p *pkgClient) createPackageInstall(o *tkgpackagedatamodel.PackageOptions, } // if configuration data file was provided, reference the secret name in the PackageInstall - if secretCreated { + if pkgPluginResourceCreationStatus.IsSecretCreated { packageInstall.Spec.Values = []kappipkg.PackageInstallValues{ { SecretRef: &kappipkg.PackageInstallValuesSecretRef{ @@ -248,15 +277,15 @@ func (p *pkgClient) createPackageInstall(o *tkgpackagedatamodel.PackageOptions, } } - if err := p.kappClient.CreatePackageInstall(packageInstall, serviceAccountCreated, secretCreated); err != nil { + if err := p.kappClient.CreatePackageInstall(packageInstall, pkgPluginResourceCreationStatus); err != nil { return errors.Wrap(err, "failed to create PackageInstall resource") } 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 +294,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 diff --git a/pkg/v1/tkg/tkgpackageclient/package_install_test.go b/pkg/v1/tkg/tkgpackageclient/package_install_test.go index 67c0f1f5bb..4e6638d218 100644 --- a/pkg/v1/tkg/tkgpackageclient/package_install_test.go +++ b/pkg/v1/tkg/tkgpackageclient/package_install_test.go @@ -51,7 +51,7 @@ var ( Spec: kappipkg.PackageInstallSpec{ ServiceAccountName: testServiceAccountName, PackageRef: &kappipkg.PackageRef{ - RefName: testPkgInstallName, + RefName: testPkgName, VersionSelection: testVersionSelection, }, }, @@ -76,8 +76,6 @@ var ( } testVersionSelection = &versions.VersionSelectionSemver{Constraints: "1.0.0"} - - testPackageInstallName = "test-package" ) var _ = Describe("Install Package", func() { @@ -110,32 +108,78 @@ var _ = Describe("Install Package", func() { err = testReceive(progress) }) - Context("failure in listing package versions due to ListPackages API error", func() { + Context("failure in getting installed package due to GetPackageInstall API error", func() { BeforeEach(func() { kappCtl = &fakes.KappClient{} crtCtl = &fakes.CRTClusterClient{} kappCtl.GetClientReturns(crtCtl) - kappCtl.ListPackagesReturns(nil, errors.New("failure in ListPackages")) + kappCtl.ListPackagesReturns(testPkgVersionList, nil) + kappCtl.GetPackageInstallReturnsOnCall(0, nil, errors.New("failure in GetPackageInstall")) }) It(testFailureMsg, func() { Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring("failure in ListPackages")) + Expect(err.Error()).To(ContainSubstring("failure in GetPackageInstall")) }) AfterEach(func() { options = opts }) }) - Context("failure in finding the provided service account", func() { + Context("falling back to update when trying to install an existing package install (with reconciliation failure)", func() { BeforeEach(func() { - options.ServiceAccountName = testServiceAccountName + options.Wait = true + options.ValuesFile = testValuesFile kappCtl = &fakes.KappClient{} crtCtl = &fakes.CRTClusterClient{} kappCtl.GetClientReturns(crtCtl) kappCtl.ListPackagesReturns(testPkgVersionList, nil) - crtCtl.GetReturns(apierrors.NewNotFound(schema.GroupResource{Resource: tkgpackagedatamodel.KindServiceAccount}, testServiceAccountName)) + kappCtl.GetPackageInstallReturns(testPkgInstall, nil) + err = os.WriteFile(testValuesFile, []byte("test"), 0644) + Expect(err).ToNot(HaveOccurred()) + Expect(testPkgInstall.Status.ObservedGeneration).To(Equal(testPkgInstall.Generation)) + Expect(len(testPkgInstall.Status.Conditions)).To(BeNumerically("==", 2)) + testPkgInstall.Status.Conditions[1] = kappctrl.AppCondition{Type: kappctrl.ReconcileFailed, Status: corev1.ConditionTrue} + testPkgInstall.Status.UsefulErrorMessage = testUsefulErrMsg }) It(testFailureMsg, func() { Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring("ServiceAccount \"test-pkg-test-ns-sa\" not found")) + Expect(err.Error()).To(ContainSubstring(testUsefulErrMsg)) + }) + AfterEach(func() { + options = opts + testPkgInstall.Status.Conditions[1].Type = kappctrl.ReconcileSucceeded + err = os.Remove(testValuesFile) + Expect(err).ToNot(HaveOccurred()) + }) + }) + + Context("falling back to update when trying to install an existing package install (throwing non-critical error)", func() { + BeforeEach(func() { + options.Wait = true + options.PackageName = testPkgName + kappCtl = &fakes.KappClient{} + crtCtl = &fakes.CRTClusterClient{} + Expect(options.PackageName).To(Equal(testPkgInstall.Spec.PackageRef.RefName)) + kappCtl.GetPackageInstallReturns(testPkgInstall, nil) + kappCtl.ListPackagesReturns(testPkgVersionList, nil) + }) + It(testFailureMsg, func() { + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring(tkgpackagedatamodel.ErrPackageAlreadyExists)) + }) + AfterEach(func() { + options = opts + }) + }) + + Context("failure in listing package versions due to ListPackages API error (in GetPackage())", func() { + BeforeEach(func() { + kappCtl = &fakes.KappClient{} + crtCtl = &fakes.CRTClusterClient{} + kappCtl.GetClientReturns(crtCtl) + kappCtl.ListPackagesReturns(nil, errors.New("failure in ListPackages")) + }) + It(testFailureMsg, func() { + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("failure in ListPackages")) }) AfterEach(func() { options = opts }) }) @@ -171,6 +215,173 @@ var _ = Describe("Install Package", func() { AfterEach(func() { options = opts }) }) + Context("failure in namespace creation due to namespace Get API error", func() { + BeforeEach(func() { + options.CreateNamespace = true + kappCtl = &fakes.KappClient{} + crtCtl = &fakes.CRTClusterClient{} + kappCtl.GetClientReturns(crtCtl) + kappCtl.ListPackagesReturns(testPkgVersionList, nil) + crtCtl.GetReturns(errors.New("failure in Get namespace")) + }) + It(testFailureMsg, func() { + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("failure in Get namespace")) + }) + AfterEach(func() { options = opts }) + }) + + Context("failure in namespace creation due to namespace Create API error", func() { + BeforeEach(func() { + options.CreateNamespace = true + kappCtl = &fakes.KappClient{} + crtCtl = &fakes.CRTClusterClient{} + kappCtl.GetClientReturns(crtCtl) + kappCtl.ListPackagesReturns(testPkgVersionList, nil) + crtCtl.GetReturns(apierrors.NewNotFound(schema.GroupResource{Resource: tkgpackagedatamodel.KindNamespace}, testNamespaceName)) + crtCtl.CreateReturns(errors.New("failure in Create namespace")) + }) + It(testFailureMsg, func() { + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("failure in Create namespace")) + }) + AfterEach(func() { options = opts }) + }) + + Context("failure in getting an existing namespace (namespace NotFound error)", func() { + BeforeEach(func() { + options.CreateNamespace = false + kappCtl = &fakes.KappClient{} + crtCtl = &fakes.CRTClusterClient{} + kappCtl.GetClientReturns(crtCtl) + kappCtl.ListPackagesReturns(testPkgVersionList, nil) + crtCtl.GetReturns(apierrors.NewNotFound(schema.GroupResource{Resource: tkgpackagedatamodel.KindNamespace}, testNamespaceName)) + }) + It(testFailureMsg, func() { + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring(fmt.Sprintf("Namespace \"%s\" not found", testNamespaceName))) + }) + AfterEach(func() { options = opts }) + }) + + Context("failure in creating service account", func() { + BeforeEach(func() { + kappCtl = &fakes.KappClient{} + crtCtl = &fakes.CRTClusterClient{} + kappCtl.GetClientReturns(crtCtl) + kappCtl.ListPackagesReturns(testPkgVersionList, nil) + crtCtl.CreateReturnsOnCall(0, errors.New("failure in Create ServiceAccount")) + }) + It(testFailureMsg, func() { + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("failure in Create ServiceAccount")) + }) + AfterEach(func() { options = opts }) + }) + + Context("failure in updating service account", func() { + BeforeEach(func() { + kappCtl = &fakes.KappClient{} + crtCtl = &fakes.CRTClusterClient{} + kappCtl.GetClientReturns(crtCtl) + kappCtl.ListPackagesReturns(testPkgVersionList, nil) + crtCtl.CreateReturnsOnCall(0, apierrors.NewAlreadyExists(schema.GroupResource{Resource: tkgpackagedatamodel.KindServiceAccount}, testServiceAccountName)) + crtCtl.UpdateReturnsOnCall(0, errors.New("failure in Update ServiceAccount")) + }) + It(testFailureMsg, func() { + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("failure in Update ServiceAccount")) + }) + AfterEach(func() { options = opts }) + }) + + Context("failure in creating cluster admin role", func() { + BeforeEach(func() { + kappCtl = &fakes.KappClient{} + crtCtl = &fakes.CRTClusterClient{} + kappCtl.GetClientReturns(crtCtl) + kappCtl.ListPackagesReturns(testPkgVersionList, nil) + crtCtl.CreateReturnsOnCall(0, nil) + crtCtl.CreateReturnsOnCall(1, errors.New("failure in Create ClusterRole")) + }) + It(testFailureMsg, func() { + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("failure in Create ClusterRole")) + }) + AfterEach(func() { options = opts }) + }) + + Context("failure in updating cluster admin role", func() { + BeforeEach(func() { + kappCtl = &fakes.KappClient{} + crtCtl = &fakes.CRTClusterClient{} + kappCtl.GetClientReturns(crtCtl) + kappCtl.ListPackagesReturns(testPkgVersionList, nil) + crtCtl.CreateReturnsOnCall(0, apierrors.NewAlreadyExists(schema.GroupResource{Resource: tkgpackagedatamodel.KindServiceAccount}, testServiceAccountName)) + crtCtl.UpdateReturnsOnCall(0, nil) + crtCtl.CreateReturnsOnCall(1, apierrors.NewAlreadyExists(schema.GroupResource{Resource: tkgpackagedatamodel.KindClusterRole}, testClusterRoleName)) + crtCtl.UpdateReturnsOnCall(1, errors.New("failure in Update ClusterRole")) + }) + It(testFailureMsg, func() { + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("failure in Update ClusterRole")) + }) + AfterEach(func() { options = opts }) + }) + + Context("failure in creating cluster role binding", func() { + BeforeEach(func() { + kappCtl = &fakes.KappClient{} + crtCtl = &fakes.CRTClusterClient{} + kappCtl.GetClientReturns(crtCtl) + kappCtl.ListPackagesReturns(testPkgVersionList, nil) + crtCtl.CreateReturnsOnCall(0, nil) + crtCtl.CreateReturnsOnCall(1, nil) + crtCtl.CreateReturnsOnCall(2, errors.New("failure in Create ClusterRoleBinding")) + }) + It(testFailureMsg, func() { + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("failure in Create ClusterRoleBinding")) + }) + AfterEach(func() { options = opts }) + }) + + Context("failure in updating cluster role binding", func() { + BeforeEach(func() { + kappCtl = &fakes.KappClient{} + crtCtl = &fakes.CRTClusterClient{} + kappCtl.GetClientReturns(crtCtl) + kappCtl.ListPackagesReturns(testPkgVersionList, nil) + crtCtl.CreateReturnsOnCall(0, apierrors.NewAlreadyExists(schema.GroupResource{Resource: tkgpackagedatamodel.KindServiceAccount}, testServiceAccountName)) + crtCtl.UpdateReturnsOnCall(0, nil) + crtCtl.CreateReturnsOnCall(1, apierrors.NewAlreadyExists(schema.GroupResource{Resource: tkgpackagedatamodel.KindClusterRole}, testClusterRoleName)) + crtCtl.UpdateReturnsOnCall(1, nil) + crtCtl.CreateReturnsOnCall(2, apierrors.NewAlreadyExists(schema.GroupResource{Resource: tkgpackagedatamodel.KindClusterRoleBinding}, testClusterRoleBindingName)) + crtCtl.UpdateReturnsOnCall(2, errors.New("failure in Update ClusterRoleBinding")) + }) + It(testFailureMsg, func() { + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("failure in Update ClusterRoleBinding")) + }) + AfterEach(func() { options = opts }) + }) + + Context("failure in finding the provided service account", func() { + BeforeEach(func() { + options.ServiceAccountName = testServiceAccountName + kappCtl = &fakes.KappClient{} + crtCtl = &fakes.CRTClusterClient{} + kappCtl.GetClientReturns(crtCtl) + kappCtl.ListPackagesReturns(testPkgVersionList, nil) + crtCtl.GetReturns(apierrors.NewNotFound(schema.GroupResource{Resource: tkgpackagedatamodel.KindServiceAccount}, testServiceAccountName)) + }) + It(testFailureMsg, func() { + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("ServiceAccount \"test-pkg-test-ns-sa\" not found")) + }) + AfterEach(func() { options = opts }) + }) + Context("failure in finding the provided secret value file", func() { BeforeEach(func() { options.ValuesFile = testValuesFile @@ -186,31 +397,95 @@ var _ = Describe("Install Package", func() { AfterEach(func() { options = opts }) }) - Context("failure in getting installed package due to GetPackageInstall API error in waitForResourceInstallation", func() { + Context("failure in creating secret", func() { BeforeEach(func() { - options.Wait = true + options.ValuesFile = testValuesFile kappCtl = &fakes.KappClient{} crtCtl = &fakes.CRTClusterClient{} kappCtl.GetClientReturns(crtCtl) kappCtl.ListPackagesReturns(testPkgVersionList, nil) - kappCtl.GetPackageInstallReturns(nil, errors.New("failure in GetPackageInstall")) + err = os.WriteFile(testValuesFile, []byte("test"), 0644) + Expect(err).ToNot(HaveOccurred()) + crtCtl.CreateReturnsOnCall(0, nil) + crtCtl.CreateReturnsOnCall(1, nil) + crtCtl.CreateReturnsOnCall(2, nil) + crtCtl.CreateReturnsOnCall(3, errors.New("failure in Create Secret")) }) It(testFailureMsg, func() { Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring("failure in GetPackageInstall")) + Expect(err.Error()).To(ContainSubstring("failure in Create Secret")) + }) + AfterEach(func() { + options = opts + err = os.Remove(testValuesFile) + Expect(err).ToNot(HaveOccurred()) + }) + }) + + Context("failure in updating secret", func() { + BeforeEach(func() { + options.ValuesFile = testValuesFile + kappCtl = &fakes.KappClient{} + crtCtl = &fakes.CRTClusterClient{} + kappCtl.GetClientReturns(crtCtl) + kappCtl.ListPackagesReturns(testPkgVersionList, nil) + err = os.WriteFile(testValuesFile, []byte("test"), 0644) + Expect(err).ToNot(HaveOccurred()) + crtCtl.CreateReturnsOnCall(0, apierrors.NewAlreadyExists(schema.GroupResource{Resource: tkgpackagedatamodel.KindServiceAccount}, testServiceAccountName)) + crtCtl.UpdateReturnsOnCall(0, nil) + crtCtl.CreateReturnsOnCall(1, apierrors.NewAlreadyExists(schema.GroupResource{Resource: tkgpackagedatamodel.KindClusterRole}, testClusterRoleName)) + crtCtl.UpdateReturnsOnCall(1, nil) + crtCtl.CreateReturnsOnCall(2, apierrors.NewAlreadyExists(schema.GroupResource{Resource: tkgpackagedatamodel.KindClusterRoleBinding}, testClusterRoleBindingName)) + crtCtl.UpdateReturnsOnCall(2, nil) + crtCtl.CreateReturnsOnCall(3, apierrors.NewAlreadyExists(schema.GroupResource{Resource: tkgpackagedatamodel.KindSecret}, testSecretName)) + crtCtl.UpdateReturnsOnCall(3, errors.New("failure in Update Secret")) + }) + It(testFailureMsg, func() { + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("failure in Update Secret")) + }) + AfterEach(func() { + options = opts + err = os.Remove(testValuesFile) + Expect(err).ToNot(HaveOccurred()) + }) + }) + + Context("failure in creating package install due to CreatePackageInstall API error", func() { + BeforeEach(func() { + options.ValuesFile = testValuesFile + kappCtl = &fakes.KappClient{} + crtCtl = &fakes.CRTClusterClient{} + kappCtl.GetClientReturns(crtCtl) + kappCtl.ListPackagesReturns(testPkgVersionList, nil) + err = os.WriteFile(testValuesFile, []byte("test"), 0644) + Expect(err).ToNot(HaveOccurred()) + crtCtl.CreateReturnsOnCall(0, nil) + crtCtl.CreateReturnsOnCall(1, nil) + crtCtl.CreateReturnsOnCall(2, nil) + crtCtl.CreateReturnsOnCall(3, nil) + kappCtl.CreatePackageInstallReturns(errors.New("failure in CreatePackageInstall")) + }) + It(testFailureMsg, func() { + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("failure in CreatePackageInstall")) + }) + AfterEach(func() { + options = opts + err = os.Remove(testValuesFile) + Expect(err).ToNot(HaveOccurred()) }) - AfterEach(func() { options = opts }) }) - Context("failure in installed package reconciliation", func() { + Context("failure when trying to install a package install (with reconciliation failure)", func() { BeforeEach(func() { options.Wait = true kappCtl = &fakes.KappClient{} crtCtl = &fakes.CRTClusterClient{} kappCtl.GetClientReturns(crtCtl) kappCtl.ListPackagesReturns(testPkgVersionList, nil) - testPkgInstall.Name = testPackageInstallName - kappCtl.GetPackageInstallReturns(testPkgInstall, nil) + kappCtl.GetPackageInstallReturnsOnCall(0, nil, apierrors.NewNotFound(schema.GroupResource{Resource: tkgpackagedatamodel.KindPackageInstall}, testPkgInstallName)) + kappCtl.GetPackageInstallReturnsOnCall(1, testPkgInstall, nil) Expect(testPkgInstall.Status.ObservedGeneration).To(Equal(testPkgInstall.Generation)) Expect(len(testPkgInstall.Status.Conditions)).To(BeNumerically("==", 2)) testPkgInstall.Status.Conditions[1] = kappctrl.AppCondition{Type: kappctrl.ReconcileFailed, Status: corev1.ConditionTrue} @@ -223,7 +498,6 @@ var _ = Describe("Install Package", func() { AfterEach(func() { options = opts testPkgInstall.Status.Conditions[1].Type = kappctrl.ReconcileSucceeded - testPkgInstall.Name = testPkgInstallName }) }) @@ -258,6 +532,23 @@ var _ = Describe("Install Package", func() { AfterEach(func() { options = opts }) }) + Context("failure in installing the package due to GetPackageInstall API error in waitForResourceInstallation", func() { + BeforeEach(func() { + options.Wait = true + kappCtl = &fakes.KappClient{} + crtCtl = &fakes.CRTClusterClient{} + kappCtl.GetClientReturns(crtCtl) + kappCtl.ListPackagesReturns(testPkgVersionList, nil) + kappCtl.GetPackageInstallReturnsOnCall(0, nil, nil) + kappCtl.GetPackageInstallReturnsOnCall(1, nil, errors.New("failure in GetPackageInstall")) + }) + It(testFailureMsg, func() { + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("failure in GetPackageInstall")) + }) + AfterEach(func() { options = opts }) + }) + Context("success in installing the package with a successful reconciliation (Wait flag being set)", func() { BeforeEach(func() { options.Wait = true @@ -265,8 +556,8 @@ var _ = Describe("Install Package", func() { crtCtl = &fakes.CRTClusterClient{} kappCtl.GetClientReturns(crtCtl) kappCtl.ListPackagesReturns(testPkgVersionList, nil) - testPkgInstall.Name = testPackageInstallName - kappCtl.GetPackageInstallReturns(testPkgInstall, nil) + kappCtl.GetPackageInstallReturnsOnCall(0, nil, apierrors.NewNotFound(schema.GroupResource{Resource: tkgpackagedatamodel.KindPackageInstall}, testPkgInstallName)) + kappCtl.GetPackageInstallReturnsOnCall(1, testPkgInstall, nil) Expect(testPkgInstall.Status.ObservedGeneration).To(Equal(testPkgInstall.Generation)) }) It(testSuccessMsg, func() { @@ -276,7 +567,6 @@ var _ = Describe("Install Package", func() { }) AfterEach(func() { options = opts - testPkgInstall.Name = testPkgInstallName }) }) @@ -302,19 +592,22 @@ var _ = Describe("Install Package", func() { }) }) - Context("failure when a duplicate package install name is provided", func() { + Context("failure when trying to update an existing package install, but providing a different o.PackageName than Spec.PackageRef.RefName", func() { BeforeEach(func() { options.Wait = true + options.PackageName = "some-other-package" kappCtl = &fakes.KappClient{} crtCtl = &fakes.CRTClusterClient{} + Expect(options.PackageName).NotTo(Equal(testPkgInstall.Spec.PackageRef.RefName)) kappCtl.GetPackageInstallReturns(testPkgInstall, nil) }) It(testFailureMsg, func() { Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring(fmt.Sprintf("package install '%s' already exists in namespace '%s'", options.PkgInstallName, options.Namespace))) + Expect(err.Error()).To(ContainSubstring(fmt.Sprintf("installed package '%s' is already associated with package '%s'", options.PkgInstallName, testPkgInstall.Spec.PackageRef.RefName))) }) AfterEach(func() { options = opts + options.PackageName = testPkgName }) }) }) @@ -331,7 +624,7 @@ func testPackageInstallPostValidation(crtCtl *fakes.CRTClusterClient, kappCtl *f kappCreateCallCnt := kappCtl.CreatePackageInstallCallCount() Expect(kappCreateCallCnt).To(BeNumerically("==", 1)) - installed, _, _ := kappCtl.CreatePackageInstallArgsForCall(0) + installed, _ := kappCtl.CreatePackageInstallArgsForCall(0) Expect(installed.Name).Should(Equal(testPkgInstallName)) } diff --git a/pkg/v1/tkg/tkgpackageclient/package_uninstall.go b/pkg/v1/tkg/tkgpackageclient/package_uninstall.go index 8a450d6b96..09d27333b0 100644 --- a/pkg/v1/tkg/tkgpackageclient/package_uninstall.go +++ b/pkg/v1/tkg/tkgpackageclient/package_uninstall.go @@ -40,7 +40,7 @@ func (p *pkgClient) UninstallPackage(o *tkgpackagedatamodel.PackageOptions, prog pkgInstall, err = p.kappClient.GetPackageInstall(o.PkgInstallName, o.Namespace) if err != nil { if apierrors.IsNotFound(err) { - if err := p.deletePreviouslyInstalledResources(o); err != nil { + if err := p.deletePreviouslyInstalledResources(o, progress.ProgressMsg); err != nil { return } err = &tkgpackagedatamodel.PackagePluginNonCriticalError{Reason: tkgpackagedatamodel.ErrPackageNotInstalled} @@ -145,37 +145,27 @@ func (p *pkgClient) deletePackageInstall(o *tkgpackagedatamodel.PackageOptions) } // deletePreviouslyInstalledResources deletes the related resources if previously installed through the package plugin -func (p *pkgClient) deletePreviouslyInstalledResources(o *tkgpackagedatamodel.PackageOptions) error { - var objMeta metav1.ObjectMeta +func (p *pkgClient) deletePreviouslyInstalledResources(o *tkgpackagedatamodel.PackageOptions, progress chan string) error { + var name string resourceAnnotation := fmt.Sprintf(tkgpackagedatamodel.TanzuPkgPluginResource, o.PkgInstallName, o.Namespace) - objMeta = metav1.ObjectMeta{ - Name: fmt.Sprintf(tkgpackagedatamodel.ClusterRoleBindingName, o.PkgInstallName, o.Namespace), - } - if err := p.deleteAnnotatedResource(&rbacv1.ClusterRoleBinding{}, crtclient.ObjectKey{Name: objMeta.Name}, resourceAnnotation); err != nil { + name = fmt.Sprintf(tkgpackagedatamodel.ClusterRoleBindingName, o.PkgInstallName, o.Namespace) + if err := p.deleteAnnotatedResource("role binding", &rbacv1.ClusterRoleBinding{}, crtclient.ObjectKey{Name: name}, resourceAnnotation, progress); err != nil { return err } - objMeta = metav1.ObjectMeta{ - Name: fmt.Sprintf(tkgpackagedatamodel.ClusterRoleName, o.PkgInstallName, o.Namespace), - } - if err := p.deleteAnnotatedResource(&rbacv1.ClusterRole{}, crtclient.ObjectKey{Name: objMeta.Name}, resourceAnnotation); err != nil { + name = fmt.Sprintf(tkgpackagedatamodel.ClusterRoleName, o.PkgInstallName, o.Namespace) + if err := p.deleteAnnotatedResource("admin role", &rbacv1.ClusterRole{}, crtclient.ObjectKey{Name: name}, resourceAnnotation, progress); err != nil { return err } - objMeta = metav1.ObjectMeta{ - Name: fmt.Sprintf(tkgpackagedatamodel.ServiceAccountName, o.PkgInstallName, o.Namespace), - Namespace: o.Namespace, - } - if err := p.deleteAnnotatedResource(&corev1.ServiceAccount{}, crtclient.ObjectKey{Name: objMeta.Name, Namespace: o.Namespace}, resourceAnnotation); err != nil { + name = fmt.Sprintf(tkgpackagedatamodel.ServiceAccountName, o.PkgInstallName, o.Namespace) + if err := p.deleteAnnotatedResource("service account", &corev1.ServiceAccount{}, crtclient.ObjectKey{Name: name, Namespace: o.Namespace}, resourceAnnotation, progress); err != nil { return err } - objMeta = metav1.ObjectMeta{ - Name: fmt.Sprintf(tkgpackagedatamodel.SecretName, o.PkgInstallName, o.Namespace), - Namespace: o.Namespace, - } - if err := p.deleteAnnotatedResource(&corev1.Secret{}, crtclient.ObjectKey{Name: objMeta.Name, Namespace: o.Namespace}, resourceAnnotation); err != nil { + name = fmt.Sprintf(tkgpackagedatamodel.SecretName, o.PkgInstallName, o.Namespace) + if err := p.deleteAnnotatedResource("secret", &corev1.Secret{}, crtclient.ObjectKey{Name: name, Namespace: o.Namespace}, resourceAnnotation, progress); err != nil { return err } @@ -183,12 +173,13 @@ func (p *pkgClient) deletePreviouslyInstalledResources(o *tkgpackagedatamodel.Pa } // deleteAnnotatedResource deletes the corresponding resource to the installed package name & namespace in case it has the package plugin annotation -func (p *pkgClient) deleteAnnotatedResource(obj runtime.Object, objKey crtclient.ObjectKey, resourceAnnotation string) error { +func (p *pkgClient) deleteAnnotatedResource(resourceType string, obj runtime.Object, objKey crtclient.ObjectKey, resourceAnnotation string, progress chan string) error { if err := p.kappClient.GetClient().Get(context.Background(), objKey, obj); err != nil { if !apierrors.IsNotFound(err) { return err } } else { + progress <- fmt.Sprintf("Deleting %s '%s'", resourceType, objKey.Name) o, err := meta.Accessor(obj) if err != nil { return err diff --git a/pkg/v1/tkg/tkgpackageclient/package_update.go b/pkg/v1/tkg/tkgpackageclient/package_update.go index 65697139f0..704854c65a 100644 --- a/pkg/v1/tkg/tkgpackageclient/package_update.go +++ b/pkg/v1/tkg/tkgpackageclient/package_update.go @@ -19,15 +19,23 @@ import ( "github.com/vmware-tanzu/tanzu-framework/pkg/v1/tkg/tkgpackagedatamodel" ) -func (p *pkgClient) UpdatePackage(o *tkgpackagedatamodel.PackageOptions, progress *tkgpackagedatamodel.PackageProgress) { +func (p *pkgClient) UpdatePackage(o *tkgpackagedatamodel.PackageOptions, progress *tkgpackagedatamodel.PackageProgress, operationType tkgpackagedatamodel.OperationType) { var ( - pkgInstall *kappipkg.PackageInstall - err error - secretCreated bool + pkgInstall *kappipkg.PackageInstall + pkgInstallToUpdate *kappipkg.PackageInstall + pkgPluginResourceCreationStatus tkgpackagedatamodel.PkgPluginResourceCreationStatus + err error + changed bool ) defer func() { - progressCleanup(err, progress) + if err != nil { + progress.Err <- err + } + if operationType == tkgpackagedatamodel.OperationTypeUpdate { + close(progress.ProgressMsg) + close(progress.Done) + } }() progress.ProgressMsg <- fmt.Sprintf("Getting package install for '%s'", o.PkgInstallName) @@ -48,41 +56,25 @@ func (p *pkgClient) UpdatePackage(o *tkgpackagedatamodel.PackageOptions, progres return } - pkgInstallToUpdate := pkgInstall.DeepCopy() - - if pkgInstallToUpdate.Spec.PackageRef == nil || pkgInstallToUpdate.Spec.PackageRef.VersionSelection == nil { - err = errors.New(fmt.Sprintf("failed to update package '%s' as no existing package reference/version was found in the package install", o.PkgInstallName)) + if pkgInstallToUpdate, changed, err = p.preparePackageInstallForUpdate(o, pkgInstall); err != nil { return } - // If o.PackageName is provided by the user (via --package-name flag), set the package name in PackageInstall to it. - // This is useful for the case in which the user made a typo in the package-name at the time of installation and it failed and they want to fix it through package update. - // Otherwise if o.PackageName is not provided, fill it from the installed package spec, as the validation logic in GetPackage() needs this field to be set. - if o.PackageName != "" { - pkgInstallToUpdate.Spec.PackageRef.RefName = o.PackageName - } else { - o.PackageName = pkgInstallToUpdate.Spec.PackageRef.RefName - } - - // If o.Version is provided by the user (via --version flag), set the version in PackageInstall to this version - // Otherwise if o.Version is not provided, fill it from the installed package spec, as the validation logic in GetPackage() needs this field to be set. - if o.Version != "" { - pkgInstallToUpdate.Spec.PackageRef.VersionSelection.Constraints = o.Version - } else { - o.Version = pkgInstallToUpdate.Spec.PackageRef.VersionSelection.Constraints - } - progress.ProgressMsg <- fmt.Sprintf("Getting package metadata for '%s'", pkgInstallToUpdate.Spec.PackageRef.RefName) if _, _, err = p.GetPackage(o); err != nil { return } - if secretCreated, err = p.createOrUpdateValuesSecret(o, pkgInstallToUpdate, progress.ProgressMsg); err != nil { + if pkgPluginResourceCreationStatus.IsSecretCreated, err = p.createOrUpdateValuesSecret(o, pkgInstallToUpdate, progress.ProgressMsg); err != nil { + return + } + + if o.ValuesFile == "" && !changed { return } progress.ProgressMsg <- fmt.Sprintf("Updating package install for '%s'", o.PkgInstallName) - if err = p.kappClient.UpdatePackageInstall(pkgInstallToUpdate, secretCreated); err != nil { + if err = p.kappClient.UpdatePackageInstall(pkgInstallToUpdate, &pkgPluginResourceCreationStatus); err != nil { err = errors.Wrap(err, fmt.Sprintf("failed to update package '%s'", o.PkgInstallName)) return } @@ -94,6 +86,42 @@ func (p *pkgClient) UpdatePackage(o *tkgpackagedatamodel.PackageOptions, progres } } +func (p *pkgClient) preparePackageInstallForUpdate(o *tkgpackagedatamodel.PackageOptions, pkgInstall *kappipkg.PackageInstall) (*kappipkg.PackageInstall, bool, error) { + var ( + changed bool + err error + ) + + pkgInstallToUpdate := pkgInstall.DeepCopy() + + if pkgInstallToUpdate.Spec.PackageRef == nil || pkgInstallToUpdate.Spec.PackageRef.VersionSelection == nil { + err = errors.New(fmt.Sprintf("failed to update package '%s' as no existing package reference/version was found in the package install", o.PkgInstallName)) + return nil, false, err + } + + // If o.PackageName is provided by the user (via --package-name flag), verify that the package name in PackageInstall matches it. + // This will prevent the users from accidentally overwriting an installed package with another package content due to choosing a pre-existing name for the package isntall. + // Otherwise if o.PackageName is not provided, fill it from the installed package spec, as the validation logic in GetPackage() needs this field to be set. + if o.PackageName != "" && pkgInstallToUpdate.Spec.PackageRef.RefName != o.PackageName { + err = errors.New(fmt.Sprintf("installed package '%s' is already associated with package '%s'", o.PkgInstallName, pkgInstallToUpdate.Spec.PackageRef.RefName)) + return nil, false, err + } + o.PackageName = pkgInstallToUpdate.Spec.PackageRef.RefName + + // If o.Version is provided by the user (via --version flag), set the version in PackageInstall to this version + // Otherwise if o.Version is not provided, fill it from the installed package spec, as the validation logic in GetPackage() needs this field to be set. + if o.Version != "" { + if pkgInstallToUpdate.Spec.PackageRef.VersionSelection.Constraints != o.Version { + changed = true + pkgInstallToUpdate.Spec.PackageRef.VersionSelection.Constraints = o.Version + } + } else { + o.Version = pkgInstallToUpdate.Spec.PackageRef.VersionSelection.Constraints + } + + return pkgInstallToUpdate, changed, nil +} + // createOrUpdateValuesSecret either creates or updates the values secret depending on whether the corresponding annotation exists or not func (p *pkgClient) createOrUpdateValuesSecret(o *tkgpackagedatamodel.PackageOptions, pkgInstallToUpdate *kappipkg.PackageInstall, progress chan string) (bool, error) { var ( @@ -115,14 +143,13 @@ func (p *pkgClient) createOrUpdateValuesSecret(o *tkgpackagedatamodel.PackageOpt } } else { progress <- fmt.Sprintf("Creating secret '%s'", o.SecretName) - if secretCreated, err = p.createDataValuesSecret(o); err != nil { + if secretCreated, err = p.createOrUpdateDataValuesSecret(o); err != nil { return secretCreated, errors.Wrap(err, "failed to create secret based on values file") } } pkgInstallToUpdate.Spec.Values = []kappipkg.PackageInstallValues{ - { - SecretRef: &kappipkg.PackageInstallValuesSecretRef{Name: o.SecretName}}, + {SecretRef: &kappipkg.PackageInstallValuesSecretRef{Name: o.SecretName}}, } return secretCreated, nil diff --git a/pkg/v1/tkg/tkgpackageclient/package_update_test.go b/pkg/v1/tkg/tkgpackageclient/package_update_test.go index b476f843a1..0954591aef 100644 --- a/pkg/v1/tkg/tkgpackageclient/package_update_test.go +++ b/pkg/v1/tkg/tkgpackageclient/package_update_test.go @@ -49,7 +49,7 @@ var _ = Describe("Update Package", func() { Done: make(chan struct{}), } ctl = &pkgClient{kappClient: kappCtl} - go ctl.UpdatePackage(&options, progress) + go ctl.UpdatePackage(&options, progress, tkgpackagedatamodel.OperationTypeUpdate) err = testReceive(progress) }) @@ -241,7 +241,7 @@ var _ = Describe("Update Package", func() { kappCtl.GetPackageInstallReturnsOnCall(0, testPkgInstall, nil) kappCtl.ListPackagesReturns(testPkgVersionList, nil) testPkgInstall.Spec.PackageRef = &kappipkg.PackageRef{ - RefName: testPkgInstallName, + RefName: testPkgName, VersionSelection: &versions.VersionSelectionSemver{}, } kappCtl.UpdatePackageInstallReturns(errors.New("failure in UpdatePackageInstall")) @@ -261,7 +261,7 @@ var _ = Describe("Update Package", func() { kappCtl.GetPackageInstallReturnsOnCall(0, testPkgInstall, nil) kappCtl.ListPackagesReturns(testPkgVersionList, nil) testPkgInstall.Spec.PackageRef = &kappipkg.PackageRef{ - RefName: testPkgInstallName, + RefName: testPkgName, VersionSelection: &versions.VersionSelectionSemver{}, } kappCtl.GetPackageInstallReturnsOnCall(1, nil, errors.New("failure in GetPackageInstall")) @@ -281,7 +281,7 @@ var _ = Describe("Update Package", func() { kappCtl.GetPackageInstallReturnsOnCall(0, testPkgInstall, nil) kappCtl.ListPackagesReturns(testPkgVersionList, nil) testPkgInstall.Spec.PackageRef = &kappipkg.PackageRef{ - RefName: testPkgInstallName, + RefName: testPkgName, VersionSelection: &versions.VersionSelectionSemver{}, } kappCtl.GetPackageInstallReturnsOnCall(1, testPkgInstall, nil) @@ -324,7 +324,7 @@ var _ = Describe("Update Package", func() { kappCtl.GetPackageInstallReturnsOnCall(0, testPkgInstall, nil) kappCtl.ListPackagesReturns(testPkgVersionList, nil) testPkgInstall.Spec.PackageRef = &kappipkg.PackageRef{ - RefName: testPkgInstallName, + RefName: testPkgName, VersionSelection: &versions.VersionSelectionSemver{}, } }) diff --git a/pkg/v1/tkg/tkgpackageclient/repository_add.go b/pkg/v1/tkg/tkgpackageclient/repository_add.go index 3fdb5fe95a..d3067f29d9 100644 --- a/pkg/v1/tkg/tkgpackageclient/repository_add.go +++ b/pkg/v1/tkg/tkgpackageclient/repository_add.go @@ -6,9 +6,9 @@ package tkgpackageclient import ( "fmt" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "github.com/pkg/errors" + k8serror "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" kappctrl "github.com/vmware-tanzu/carvel-kapp-controller/pkg/apis/kappctrl/v1alpha1" kappipkg "github.com/vmware-tanzu/carvel-kapp-controller/pkg/apis/packaging/v1alpha1" @@ -24,7 +24,10 @@ const ( // AddRepository validates the provided input and adds the package repository CR to the cluster func (p *pkgClient) AddRepository(o *tkgpackagedatamodel.RepositoryOptions, progress *tkgpackagedatamodel.PackageProgress, operationType tkgpackagedatamodel.OperationType) { - var err error + var ( + pkgRepository *kappipkg.PackageRepository + err error + ) defer func() { if err != nil { @@ -36,6 +39,20 @@ func (p *pkgClient) AddRepository(o *tkgpackagedatamodel.RepositoryOptions, prog } }() + if pkgRepository, err = p.kappClient.GetPackageRepository(o.RepositoryName, o.Namespace); err != nil { + if !k8serror.IsNotFound(err) { + return + } + err = nil + } + + if pkgRepository != nil { + progress.ProgressMsg <- fmt.Sprintf("Updating package repository '%s'", o.RepositoryName) + p.UpdateRepository(o, progress, tkgpackagedatamodel.OperationTypeInstall) + err = &tkgpackagedatamodel.PackagePluginNonCriticalError{Reason: tkgpackagedatamodel.ErrRepoAlreadyExists} + return + } + progress.ProgressMsg <- "Validating provided settings for the package repository" if err = p.validateRepositoryAdd(o.RepositoryName, o.RepositoryURL, o.Namespace); err != nil { return diff --git a/pkg/v1/tkg/tkgpackageclient/repository_add_test.go b/pkg/v1/tkg/tkgpackageclient/repository_add_test.go index dbcce0c103..d44281349c 100644 --- a/pkg/v1/tkg/tkgpackageclient/repository_add_test.go +++ b/pkg/v1/tkg/tkgpackageclient/repository_add_test.go @@ -141,6 +141,27 @@ var _ = Describe("Add Repository", func() { AfterEach(func() { options = opts }) }) + Context("falling back to update when trying to add an existing package repository (throwing non-critical error)", func() { + BeforeEach(func() { + options.Wait = true + options.RepositoryName = testRepoName + options.Wait = false + kappCtl = &fakes.KappClient{} + crtCtl = &fakes.CRTClusterClient{} + kappCtl.GetClientReturns(crtCtl) + kappCtl.ListPackageRepositoriesReturns(pkgRepositoryList, nil) + kappCtl.GetPackageRepositoryReturns(testRepository, nil) + }) + It(testFailureMsg, func() { + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring(tkgpackagedatamodel.ErrRepoAlreadyExists)) + }) + AfterEach(func() { + options = opts + options.RepositoryName = testRegistry + }) + }) + Context("success in creating the package repository in not previously existing 'test-ns' namespace", func() { BeforeEach(func() { options.CreateNamespace = true diff --git a/pkg/v1/tkg/tkgpackageclient/repository_delete.go b/pkg/v1/tkg/tkgpackageclient/repository_delete.go index a4f5480390..990072f758 100644 --- a/pkg/v1/tkg/tkgpackageclient/repository_delete.go +++ b/pkg/v1/tkg/tkgpackageclient/repository_delete.go @@ -33,7 +33,7 @@ func (p *pkgClient) DeleteRepository(o *tkgpackagedatamodel.RepositoryOptions, p return } - progress.ProgressMsg <- "Deleting package repository resoure" + progress.ProgressMsg <- "Deleting package repository resource" err = p.kappClient.DeletePackageRepository(packageRepo) if err != nil { err = errors.Wrap(err, fmt.Sprintf("failed to delete package repository '%s' from namespace '%s'", o.RepositoryName, o.Namespace)) diff --git a/pkg/v1/tkg/tkgpackageclient/repository_update.go b/pkg/v1/tkg/tkgpackageclient/repository_update.go index 835d86a9fa..a875f11fd1 100644 --- a/pkg/v1/tkg/tkgpackageclient/repository_update.go +++ b/pkg/v1/tkg/tkgpackageclient/repository_update.go @@ -17,7 +17,7 @@ import ( "github.com/vmware-tanzu/tanzu-framework/pkg/v1/tkg/tkgpackagedatamodel" ) -func (p *pkgClient) UpdateRepository(o *tkgpackagedatamodel.RepositoryOptions, progress *tkgpackagedatamodel.PackageProgress) { +func (p *pkgClient) UpdateRepository(o *tkgpackagedatamodel.RepositoryOptions, progress *tkgpackagedatamodel.PackageProgress, operationType tkgpackagedatamodel.OperationType) { var ( existingRepository *kappipkg.PackageRepository err error @@ -25,7 +25,13 @@ func (p *pkgClient) UpdateRepository(o *tkgpackagedatamodel.RepositoryOptions, p ) defer func() { - progressCleanup(err, progress) + if err != nil { + progress.Err <- err + } + if operationType == tkgpackagedatamodel.OperationTypeUpdate { + close(progress.ProgressMsg) + close(progress.Done) + } }() progress.ProgressMsg <- fmt.Sprintf("Getting package repository '%s'", o.RepositoryName) @@ -40,6 +46,7 @@ func (p *pkgClient) UpdateRepository(o *tkgpackagedatamodel.RepositoryOptions, p if existingRepository != nil { repositoryToUpdate := existingRepository.DeepCopy() + progress.ProgressMsg <- "Validating provided settings for the package repository" if err = p.validateRepositoryUpdate(o.RepositoryName, o.RepositoryURL, o.Namespace); err != nil { return } diff --git a/pkg/v1/tkg/tkgpackageclient/repository_update_test.go b/pkg/v1/tkg/tkgpackageclient/repository_update_test.go index 19c19afc86..15d7e58036 100644 --- a/pkg/v1/tkg/tkgpackageclient/repository_update_test.go +++ b/pkg/v1/tkg/tkgpackageclient/repository_update_test.go @@ -43,7 +43,7 @@ var _ = Describe("Update Repository", func() { Done: make(chan struct{}), } ctl = &pkgClient{kappClient: kappCtl} - go ctl.UpdateRepository(&options, progress) + go ctl.UpdateRepository(&options, progress, tkgpackagedatamodel.OperationTypeUpdate) err = testReceive(progress) }) diff --git a/pkg/v1/tkg/tkgpackagedatamodel/constants.go b/pkg/v1/tkg/tkgpackagedatamodel/constants.go index 3284046a3b..4649a59ce7 100644 --- a/pkg/v1/tkg/tkgpackagedatamodel/constants.go +++ b/pkg/v1/tkg/tkgpackagedatamodel/constants.go @@ -16,6 +16,8 @@ const ( DefaultRepositoryImageTagConstraint = ">0.0.0" ErrPackageNotInstalled = "package install does not exist in the namespace" ErrRepoNotExists = "package repository does not exist in the namespace" + ErrPackageAlreadyExists = "package install already exists in the namespace" + ErrRepoAlreadyExists = "package repository already exists in the namespace" KindClusterRole = "ClusterRole" KindClusterRoleBinding = "ClusterRoleBinding" KindNamespace = "Namespace" diff --git a/pkg/v1/tkg/tkgpackagedatamodel/types.go b/pkg/v1/tkg/tkgpackagedatamodel/types.go index 5a7186ea92..5c3806062d 100644 --- a/pkg/v1/tkg/tkgpackagedatamodel/types.go +++ b/pkg/v1/tkg/tkgpackagedatamodel/types.go @@ -41,6 +41,11 @@ const ( OperationTypeUpdate ) +type PkgPluginResourceCreationStatus struct { + IsServiceAccountCreated bool + IsSecretCreated bool +} + // TypeBoolPtr satisfies Value interface defined in "https://github.com/spf13/pflag/blob/master/flag.go" type TypeBoolPtr struct { ExportToAllNamespaces *bool