Repository: vmware-tanzu/tanzu-framework (language: go)

Issue:
uTKG cli addon's monitoring logic is not looking for correct addon's package name
**Bug description**
For uTKG, CLI cluster creation monitoring logic for the core addon's packages is not working when the package name is not correct in the cluster bootstrap object

**Affected product area (please put an X in all that apply)**

- ( ) APIs
- ( ) Addons
- (x) CLI
- ( ) Docs
- ( ) IAM
- ( ) Installation
- ( ) Plugin
- ( ) Security
- ( ) Test and Release
- ( ) User Experience
- ( ) Developer Experience

**Expected behavior**

**Steps to reproduce the bug**

**Version** (include the SHA if the version is not obvious)

**Environment where the bug was observed (cloud, OS, etc)**

**Relevant Debug Output (Logs, manifests, etc)**



Discussion / hints:

Fix patch:
diff --git a/pkg/v1/tkg/client/cluster.go b/pkg/v1/tkg/client/cluster.go
index 2fc1d71734..7e4287c64c 100644
--- a/pkg/v1/tkg/client/cluster.go
+++ b/pkg/v1/tkg/client/cluster.go
@@ -440,7 +440,10 @@ func (c *TkgClient) WaitForAddonsCorePackagesInstallation(options waitForAddonsO
 	} else {
 		corePackagesNamespace = constants.CorePackagesNamespaceInTKGM
 	}
-	packages := GetCorePackagesFromClusterBootstrap(clusterBootstrap, corePackagesNamespace)
+	packages, err := GetCorePackagesFromClusterBootstrap(options.regionalClusterClient, options.workloadClusterClient, clusterBootstrap, corePackagesNamespace, options.clusterName)
+	if err != nil {
+		return err
+	}
 	return MonitorAddonsCorePackageInstallation(options.regionalClusterClient, options.workloadClusterClient, packages, c.getPackageInstallTimeoutFromConfig())
 }
 
diff --git a/pkg/v1/tkg/client/package_helper.go b/pkg/v1/tkg/client/package_helper.go
index 1edfbdc7eb..56e02f2333 100644
--- a/pkg/v1/tkg/client/package_helper.go
+++ b/pkg/v1/tkg/client/package_helper.go
@@ -5,6 +5,7 @@ package client
 
 import (
 	"context"
+	"fmt"
 	"strings"
 	"time"
 
@@ -18,7 +19,7 @@ import (
 	"github.com/vmware-tanzu/tanzu-framework/pkg/v1/tkg/log"
 )
 
-// GetClusterBootstrap returns ClusterBootstrap object for the given clustername in the management cluster
+// GetClusterBootstrap returns ClusterBootstrap object for the given clusterName in the management cluster
 func GetClusterBootstrap(managementClusterClient clusterclient.Client, clusterName, namespace string) (*runtanzuv1alpha3.ClusterBootstrap, error) {
 	log.V(3).Infof("getting ClusterBootstrap object for cluster: %v", clusterName)
 	clusterBootstrap := &runtanzuv1alpha3.ClusterBootstrap{}
@@ -26,29 +27,68 @@ func GetClusterBootstrap(managementClusterClient clusterclient.Client, clusterNa
 	return clusterBootstrap, err
 }
 
-// GetCorePackagesFromClusterBootstrap returns addon's core packages details from the given ClsuterBootstrap object
-func GetCorePackagesFromClusterBootstrap(clusterBootstrap *runtanzuv1alpha3.ClusterBootstrap, corePackagesNamespace string) []kapppkgv1alpha1.Package {
+// GetCorePackagesFromClusterBootstrap returns addon's core packages details from the given ClusterBootstrap object
+func GetCorePackagesFromClusterBootstrap(regionalClusterClient clusterclient.Client, workloadClusterClient clusterclient.Client, clusterBootstrap *runtanzuv1alpha3.ClusterBootstrap, corePackagesNamespace, clusterName string) ([]kapppkgv1alpha1.Package, error) {
 	var packages []kapppkgv1alpha1.Package
-	suffixStr := "-package"
-	// kapp package is installed in namespace in which workload cluster created
-	if clusterBootstrap.Spec.Kapp != nil && clusterBootstrap.Spec.Kapp.ValuesFrom != nil && clusterBootstrap.Spec.Kapp.ValuesFrom.ProviderRef != nil {
-		name := strings.TrimSuffix(clusterBootstrap.Spec.Kapp.ValuesFrom.ProviderRef.Name, suffixStr)
-		packages = append(packages, kapppkgv1alpha1.Package{ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: clusterBootstrap.ObjectMeta.Namespace}})
+	// kapp package is installed in management cluster, in namespace in which workload cluster created
+	if isPackageExists(clusterBootstrap.Spec.Kapp) {
+		pkg, err := getPackage(regionalClusterClient, clusterBootstrap.Spec.Kapp.RefName, clusterBootstrap.Namespace, clusterName)
+		if err != nil {
+			return packages, err
+		}
+		packages = append(packages, *pkg)
 	}
-	// Core packages CNI, CSI and CPI (other than Kapp) installed in tkg-system namespace in case of tkgm, and in case of tkgs installed in vmware-system-tkg namespace
-	if clusterBootstrap.Spec.CNI != nil && clusterBootstrap.Spec.CNI.ValuesFrom != nil && clusterBootstrap.Spec.CNI.ValuesFrom.ProviderRef != nil {
-		name := strings.TrimSuffix(clusterBootstrap.Spec.CNI.ValuesFrom.ProviderRef.Name, suffixStr)
-		packages = append(packages, kapppkgv1alpha1.Package{ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: corePackagesNamespace}})
+
+	// Core packages CNI, CSI and CPI (other than Kapp) installed in workload cluster
+	if isPackageExists(clusterBootstrap.Spec.CNI) {
+		pkg, err := getPackage(workloadClusterClient, clusterBootstrap.Spec.CNI.RefName, corePackagesNamespace, clusterName)
+		if err != nil {
+			return packages, err
+		}
+		packages = append(packages, *pkg)
 	}
-	if clusterBootstrap.Spec.CSI != nil && clusterBootstrap.Spec.CSI.ValuesFrom != nil && clusterBootstrap.Spec.CSI.ValuesFrom.ProviderRef != nil {
-		name := strings.TrimSuffix(clusterBootstrap.Spec.CSI.ValuesFrom.ProviderRef.Name, suffixStr)
-		packages = append(packages, kapppkgv1alpha1.Package{ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: corePackagesNamespace}})
+	if isPackageExists(clusterBootstrap.Spec.CSI) {
+		pkg, err := getPackage(workloadClusterClient, clusterBootstrap.Spec.CSI.RefName, corePackagesNamespace, clusterName)
+		if err != nil {
+			return packages, err
+		}
+		packages = append(packages, *pkg)
 	}
-	if clusterBootstrap.Spec.CPI != nil && clusterBootstrap.Spec.CPI.ValuesFrom != nil && clusterBootstrap.Spec.CPI.ValuesFrom.ProviderRef != nil {
-		name := strings.TrimSuffix(clusterBootstrap.Spec.CPI.ValuesFrom.ProviderRef.Name, suffixStr)
-		packages = append(packages, kapppkgv1alpha1.Package{ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: corePackagesNamespace}})
+	if isPackageExists(clusterBootstrap.Spec.CPI) {
+		pkg, err := getPackage(workloadClusterClient, clusterBootstrap.Spec.CPI.RefName, corePackagesNamespace, clusterName)
+		if err != nil {
+			return packages, err
+		}
+		packages = append(packages, *pkg)
 	}
-	return packages
+	return packages, nil
+}
+
+func isPackageExists(cbPackage *runtanzuv1alpha3.ClusterBootstrapPackage) bool {
+	return cbPackage != nil && cbPackage.ValuesFrom != nil && cbPackage.ValuesFrom.ProviderRef != nil
+}
+
+// getPackage takes clusterclient, package name, namespace and cluster name, queries for package details and give package object (which has package name and namespace details)
+func getPackage(clusterClient clusterclient.Client, packageRefName, packagesNamespace, clusterName string) (*kapppkgv1alpha1.Package, error) {
+	pkg := &kapppkgv1alpha1.Package{}
+	pkgFromCluster, err := clusterClient.GetPackage(packageRefName, packagesNamespace)
+	if err != nil {
+		return pkg, err
+	}
+	pkgName := GeneratePackageInstallName(clusterName, pkgFromCluster.Spec.RefName)
+	pkg = &kapppkgv1alpha1.Package{ObjectMeta: metav1.ObjectMeta{Name: pkgName, Namespace: packagesNamespace}}
+	return pkg, err
+}
+
+// GeneratePackageInstallName is the util function to generate the PackageInstall CR name in a consistent manner.
+// clusterName is the name of cluster within which all resources associated with this PackageInstall CR is installed.
+// It does not necessarily
+// mean the PackageInstall CR will be installed in that cluster. I.e., the kapp-controller PackageInstall CR is installed
+// in the management cluster but is named after "<workload-cluster-name>-kapp-controller". It indicates that this kapp-controller
+// PackageInstall is for reconciling resources in a cluster named "<workload-cluster-name>".
+// addonName is the short name of a Tanzu addon with which the PackageInstall CR is associated.
+func GeneratePackageInstallName(clusterName, addonName string) string {
+	return fmt.Sprintf("%s-%s", clusterName, strings.Split(addonName, ".")[0])
 }
 
 // MonitorAddonsCorePackageInstallation monitors addon's core packages (kapp, cni, csi and cpi) and returns error if any while monitoring packages or any packages are not installed successfully. First it monitors kapp package in management cluster then it monitors other core packages in workload cluster.
diff --git a/pkg/v1/tkg/clusterclient/clusterclient.go b/pkg/v1/tkg/clusterclient/clusterclient.go
index f87fa9a487..0d3bca712f 100644
--- a/pkg/v1/tkg/clusterclient/clusterclient.go
+++ b/pkg/v1/tkg/clusterclient/clusterclient.go
@@ -62,6 +62,7 @@ import (
 
 	kappipkg "github.com/vmware-tanzu/carvel-kapp-controller/pkg/apis/packaging/v1alpha1"
 
+	kapppkgv1alpha1 "github.com/vmware-tanzu/carvel-kapp-controller/pkg/apiserver/apis/datapackaging/v1alpha1"
 	cliv1alpha1 "github.com/vmware-tanzu/tanzu-framework/apis/cli/v1alpha1"
 	configv1alpha1 "github.com/vmware-tanzu/tanzu-framework/apis/config/v1alpha1"
 	runv1alpha1 "github.com/vmware-tanzu/tanzu-framework/apis/run/v1alpha1"
@@ -86,6 +87,7 @@ const (
 	// DefaultKappControllerHostPort is the default kapp-controller port for it's extension apiserver
 	DefaultKappControllerHostPort           = 10100
 	waitPeriodBeforePollingForUpgradeStatus = 60 * time.Second
+	ErrUnableToGetPackage                   = "unable to get the package: '%s' in namespace: '%s'"
 )
 
 var (
@@ -131,6 +133,8 @@ type Client interface {
 	WaitK8sVersionUpdateForWorkerNodes(clusterName, namespace, kubernetesVersion string, workloadClusterClient Client) error
 	// GetKubeConfigForCluster returns the admin kube config for accessing the cluster
 	GetKubeConfigForCluster(clusterName string, namespace string, pollOptions *PollOptions) ([]byte, error)
+	// GetPackage returns the package for given package name in a given namespace
+	GetPackage(carvelPkgName, carvelPkgNamespace string) (*kapppkgv1alpha1.Package, error)
 	// GetSecretValue returns the value for a given key in a Secret
 	GetSecretValue(secretName, key, namespace string, pollOptions *PollOptions) ([]byte, error)
 	// GetCurrentNamespace returns the namespace from the current context in the kubeconfig file
@@ -446,6 +450,7 @@ func init() {
 	_ = kappipkg.AddToScheme(scheme)
 	_ = cliv1alpha1.AddToScheme(scheme)
 	_ = configv1alpha1.AddToScheme(scheme)
+	_ = kapppkgv1alpha1.AddToScheme(scheme)
 }
 
 // ClusterStatusInfo defines the cluster status involving all main components
@@ -1335,6 +1340,17 @@ func (c *client) kubectlGetResource(resource string, args ...string) ([]byte, er
 	return out, nil
 }
 
+// GetPackage returns the package for given package name in a given namespace
+func (c *client) GetPackage(carvelPkgName, carvelPkgNamespace string) (*kapppkgv1alpha1.Package, error) {
+	pkg := &kapppkgv1alpha1.Package{}
+	log.V(9).Infof("getting package:%s in namespace:%s", carvelPkgName, carvelPkgNamespace)
+	err := c.GetResource(pkg, carvelPkgName, carvelPkgNamespace, nil, nil)
+	if err != nil {
+		return pkg, errors.Wrapf(err, ErrUnableToGetPackage, carvelPkgName, carvelPkgNamespace)
+	}
+	return pkg, nil
+}
+
 func (c *client) GetSecretValue(secretName, key, namespace string, pollOptions *PollOptions) ([]byte, error) {
 	var err error
 
diff --git a/pkg/v1/tkg/fakes/clusterclient.go b/pkg/v1/tkg/fakes/clusterclient.go
index 167650b1a2..bd6c2f2c9a 100644
--- a/pkg/v1/tkg/fakes/clusterclient.go
+++ b/pkg/v1/tkg/fakes/clusterclient.go
@@ -12,8 +12,9 @@ import (
 	"sigs.k8s.io/cluster-api/controlplane/kubeadm/api/v1beta1"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
-	v1alpha1a "github.com/vmware-tanzu/tanzu-framework/apis/cli/v1alpha1"
-	"github.com/vmware-tanzu/tanzu-framework/apis/run/v1alpha1"
+	"github.com/vmware-tanzu/carvel-kapp-controller/pkg/apiserver/apis/datapackaging/v1alpha1"
+	v1alpha1b "github.com/vmware-tanzu/tanzu-framework/apis/cli/v1alpha1"
+	v1alpha1a "github.com/vmware-tanzu/tanzu-framework/apis/run/v1alpha1"
 	"github.com/vmware-tanzu/tanzu-framework/apis/run/v1alpha2"
 	"github.com/vmware-tanzu/tanzu-framework/pkg/v1/tkg/azure"
 	"github.com/vmware-tanzu/tanzu-framework/pkg/v1/tkg/clusterclient"
@@ -419,6 +420,20 @@ type ClusterClient struct {
 		result1 []string
 		result2 error
 	}
+	GetPackageStub        func(string, string) (*v1alpha1.Package, error)
+	getPackageMutex       sync.RWMutex
+	getPackageArgsForCall []struct {
+		arg1 string
+		arg2 string
+	}
+	getPackageReturns struct {
+		result1 *v1alpha1.Package
+		result2 error
+	}
+	getPackageReturnsOnCall map[int]struct {
+		result1 *v1alpha1.Package
+		result2 error
+	}
 	GetPinnipedIssuerURLAndCAStub        func() (string, string, error)
 	getPinnipedIssuerURLAndCAMutex       sync.RWMutex
 	getPinnipedIssuerURLAndCAArgsForCall []struct {
@@ -492,17 +507,17 @@ type ClusterClient struct {
 		result1 []byte
 		result2 error
 	}
-	GetTanzuKubernetesReleasesStub        func(string) ([]v1alpha1.TanzuKubernetesRelease, error)
+	GetTanzuKubernetesReleasesStub        func(string) ([]v1alpha1a.TanzuKubernetesRelease, error)
 	getTanzuKubernetesReleasesMutex       sync.RWMutex
 	getTanzuKubernetesReleasesArgsForCall []struct {
 		arg1 string
 	}
 	getTanzuKubernetesReleasesReturns struct {
-		result1 []v1alpha1.TanzuKubernetesRelease
+		result1 []v1alpha1a.TanzuKubernetesRelease
 		result2 error
 	}
 	getTanzuKubernetesReleasesReturnsOnCall map[int]struct {
-		result1 []v1alpha1.TanzuKubernetesRelease
+		result1 []v1alpha1a.TanzuKubernetesRelease
 		result2 error
 	}
 	GetVCClientAndDataCenterStub        func(string, string, string) (vc.Client, string, error)
@@ -626,16 +641,16 @@ type ClusterClient struct {
 	isRegionalClusterReturnsOnCall map[int]struct {
 		result1 error
 	}
-	ListCLIPluginResourcesStub        func() ([]v1alpha1a.CLIPlugin, error)
+	ListCLIPluginResourcesStub        func() ([]v1alpha1b.CLIPlugin, error)
 	listCLIPluginResourcesMutex       sync.RWMutex
 	listCLIPluginResourcesArgsForCall []struct {
 	}
 	listCLIPluginResourcesReturns struct {
-		result1 []v1alpha1a.CLIPlugin
+		result1 []v1alpha1b.CLIPlugin
 		result2 error
 	}
 	listCLIPluginResourcesReturnsOnCall map[int]struct {
-		result1 []v1alpha1a.CLIPlugin
+		result1 []v1alpha1b.CLIPlugin
 		result2 error
 	}
 	ListClustersStub        func(string) ([]v1beta1a.Cluster, error)
@@ -3137,6 +3152,71 @@ func (fake *ClusterClient) GetPacificTanzuKubernetesReleasesReturnsOnCall(i int,
 	}{result1, result2}
 }
 
+func (fake *ClusterClient) GetPackage(arg1 string, arg2 string) (*v1alpha1.Package, error) {
+	fake.getPackageMutex.Lock()
+	ret, specificReturn := fake.getPackageReturnsOnCall[len(fake.getPackageArgsForCall)]
+	fake.getPackageArgsForCall = append(fake.getPackageArgsForCall, struct {
+		arg1 string
+		arg2 string
+	}{arg1, arg2})
+	stub := fake.GetPackageStub
+	fakeReturns := fake.getPackageReturns
+	fake.recordInvocation("GetPackage", []interface{}{arg1, arg2})
+	fake.getPackageMutex.Unlock()
+	if stub != nil {
+		return stub(arg1, arg2)
+	}
+	if specificReturn {
+		return ret.result1, ret.result2
+	}
+	return fakeReturns.result1, fakeReturns.result2
+}
+
+func (fake *ClusterClient) GetPackageCallCount() int {
+	fake.getPackageMutex.RLock()
+	defer fake.getPackageMutex.RUnlock()
+	return len(fake.getPackageArgsForCall)
+}
+
+func (fake *ClusterClient) GetPackageCalls(stub func(string, string) (*v1alpha1.Package, error)) {
+	fake.getPackageMutex.Lock()
+	defer fake.getPackageMutex.Unlock()
+	fake.GetPackageStub = stub
+}
+
+func (fake *ClusterClient) GetPackageArgsForCall(i int) (string, string) {
+	fake.getPackageMutex.RLock()
+	defer fake.getPackageMutex.RUnlock()
+	argsForCall := fake.getPackageArgsForCall[i]
+	return argsForCall.arg1, argsForCall.arg2
+}
+
+func (fake *ClusterClient) GetPackageReturns(result1 *v1alpha1.Package, result2 error) {
+	fake.getPackageMutex.Lock()
+	defer fake.getPackageMutex.Unlock()
+	fake.GetPackageStub = nil
+	fake.getPackageReturns = struct {
+		result1 *v1alpha1.Package
+		result2 error
+	}{result1, result2}
+}
+
+func (fake *ClusterClient) GetPackageReturnsOnCall(i int, result1 *v1alpha1.Package, result2 error) {
+	fake.getPackageMutex.Lock()
+	defer fake.getPackageMutex.Unlock()
+	fake.GetPackageStub = nil
+	if fake.getPackageReturnsOnCall == nil {
+		fake.getPackageReturnsOnCall = make(map[int]struct {
+			result1 *v1alpha1.Package
+			result2 error
+		})
+	}
+	fake.getPackageReturnsOnCall[i] = struct {
+		result1 *v1alpha1.Package
+		result2 error
+	}{result1, result2}
+}
+
 func (fake *ClusterClient) GetPinnipedIssuerURLAndCA() (string, string, error) {
 	fake.getPinnipedIssuerURLAndCAMutex.Lock()
 	ret, specificReturn := fake.getPinnipedIssuerURLAndCAReturnsOnCall[len(fake.getPinnipedIssuerURLAndCAArgsForCall)]
@@ -3457,7 +3537,7 @@ func (fake *ClusterClient) GetSecretValueReturnsOnCall(i int, result1 []byte, re
 	}{result1, result2}
 }
 
-func (fake *ClusterClient) GetTanzuKubernetesReleases(arg1 string) ([]v1alpha1.TanzuKubernetesRelease, error) {
+func (fake *ClusterClient) GetTanzuKubernetesReleases(arg1 string) ([]v1alpha1a.TanzuKubernetesRelease, error) {
 	fake.getTanzuKubernetesReleasesMutex.Lock()
 	ret, specificReturn := fake.getTanzuKubernetesReleasesReturnsOnCall[len(fake.getTanzuKubernetesReleasesArgsForCall)]
 	fake.getTanzuKubernetesReleasesArgsForCall = append(fake.getTanzuKubernetesReleasesArgsForCall, struct {
@@ -3482,7 +3562,7 @@ func (fake *ClusterClient) GetTanzuKubernetesReleasesCallCount() int {
 	return len(fake.getTanzuKubernetesReleasesArgsForCall)
 }
 
-func (fake *ClusterClient) GetTanzuKubernetesReleasesCalls(stub func(string) ([]v1alpha1.TanzuKubernetesRelease, error)) {
+func (fake *ClusterClient) GetTanzuKubernetesReleasesCalls(stub func(string) ([]v1alpha1a.TanzuKubernetesRelease, error)) {
 	fake.getTanzuKubernetesReleasesMutex.Lock()
 	defer fake.getTanzuKubernetesReleasesMutex.Unlock()
 	fake.GetTanzuKubernetesReleasesStub = stub
@@ -3495,28 +3575,28 @@ func (fake *ClusterClient) GetTanzuKubernetesReleasesArgsForCall(i int) string {
 	return argsForCall.arg1
 }
 
-func (fake *ClusterClient) GetTanzuKubernetesReleasesReturns(result1 []v1alpha1.TanzuKubernetesRelease, result2 error) {
+func (fake *ClusterClient) GetTanzuKubernetesReleasesReturns(result1 []v1alpha1a.TanzuKubernetesRelease, result2 error) {
 	fake.getTanzuKubernetesReleasesMutex.Lock()
 	defer fake.getTanzuKubernetesReleasesMutex.Unlock()
 	fake.GetTanzuKubernetesReleasesStub = nil
 	fake.getTanzuKubernetesReleasesReturns = struct {
-		result1 []v1alpha1.TanzuKubernetesRelease
+		result1 []v1alpha1a.TanzuKubernetesRelease
 		result2 error
 	}{result1, result2}
 }
 
-func (fake *ClusterClient) GetTanzuKubernetesReleasesReturnsOnCall(i int, result1 []v1alpha1.TanzuKubernetesRelease, result2 error) {
+func (fake *ClusterClient) GetTanzuKubernetesReleasesReturnsOnCall(i int, result1 []v1alpha1a.TanzuKubernetesRelease, result2 error) {
 	fake.getTanzuKubernetesReleasesMutex.Lock()
 	defer fake.getTanzuKubernetesReleasesMutex.Unlock()
 	fake.GetTanzuKubernetesReleasesStub = nil
 	if fake.getTanzuKubernetesReleasesReturnsOnCall == nil {
 		fake.getTanzuKubernetesReleasesReturnsOnCall = make(map[int]struct {
-			result1 []v1alpha1.TanzuKubernetesRelease
+			result1 []v1alpha1a.TanzuKubernetesRelease
 			result2 error
 		})
 	}
 	fake.getTanzuKubernetesReleasesReturnsOnCall[i] = struct {
-		result1 []v1alpha1.TanzuKubernetesRelease
+		result1 []v1alpha1a.TanzuKubernetesRelease
 		result2 error
 	}{result1, result2}
 }
@@ -4075,7 +4155,7 @@ func (fake *ClusterClient) IsRegionalClusterReturnsOnCall(i int, result1 error)
 	}{result1}
 }
 
-func (fake *ClusterClient) ListCLIPluginResources() ([]v1alpha1a.CLIPlugin, error) {
+func (fake *ClusterClient) ListCLIPluginResources() ([]v1alpha1b.CLIPlugin, error) {
 	fake.listCLIPluginResourcesMutex.Lock()
 	ret, specificReturn := fake.listCLIPluginResourcesReturnsOnCall[len(fake.listCLIPluginResourcesArgsForCall)]
 	fake.listCLIPluginResourcesArgsForCall = append(fake.listCLIPluginResourcesArgsForCall, struct {
@@ -4099,34 +4179,34 @@ func (fake *ClusterClient) ListCLIPluginResourcesCallCount() int {
 	return len(fake.listCLIPluginResourcesArgsForCall)
 }
 
-func (fake *ClusterClient) ListCLIPluginResourcesCalls(stub func() ([]v1alpha1a.CLIPlugin, error)) {
+func (fake *ClusterClient) ListCLIPluginResourcesCalls(stub func() ([]v1alpha1b.CLIPlugin, error)) {
 	fake.listCLIPluginResourcesMutex.Lock()
 	defer fake.listCLIPluginResourcesMutex.Unlock()
 	fake.ListCLIPluginResourcesStub = stub
 }
 
-func (fake *ClusterClient) ListCLIPluginResourcesReturns(result1 []v1alpha1a.CLIPlugin, result2 error) {
+func (fake *ClusterClient) ListCLIPluginResourcesReturns(result1 []v1alpha1b.CLIPlugin, result2 error) {
 	fake.listCLIPluginResourcesMutex.Lock()
 	defer fake.listCLIPluginResourcesMutex.Unlock()
 	fake.ListCLIPluginResourcesStub = nil
 	fake.listCLIPluginResourcesReturns = struct {
-		result1 []v1alpha1a.CLIPlugin
+		result1 []v1alpha1b.CLIPlugin
 		result2 error
 	}{result1, result2}
 }
 
-func (fake *ClusterClient) ListCLIPluginResourcesReturnsOnCall(i int, result1 []v1alpha1a.CLIPlugin, result2 error) {
+func (fake *ClusterClient) ListCLIPluginResourcesReturnsOnCall(i int, result1 []v1alpha1b.CLIPlugin, result2 error) {
 	fake.listCLIPluginResourcesMutex.Lock()
 	defer fake.listCLIPluginResourcesMutex.Unlock()
 	fake.ListCLIPluginResourcesStub = nil
 	if fake.listCLIPluginResourcesReturnsOnCall == nil {
 		fake.listCLIPluginResourcesReturnsOnCall = make(map[int]struct {
-			result1 []v1alpha1a.CLIPlugin
+			result1 []v1alpha1b.CLIPlugin
 			result2 error
 		})
 	}
 	fake.listCLIPluginResourcesReturnsOnCall[i] = struct {
-		result1 []v1alpha1a.CLIPlugin
+		result1 []v1alpha1b.CLIPlugin
 		result2 error
 	}{result1, result2}
 }
@@ -6965,6 +7045,8 @@ func (fake *ClusterClient) Invocations() map[string][][]interface{} {
 	defer fake.getPacificTKCAPIVersionMutex.RUnlock()
 	fake.getPacificTanzuKubernetesReleasesMutex.RLock()
 	defer fake.getPacificTanzuKubernetesReleasesMutex.RUnlock()
+	fake.getPackageMutex.RLock()
+	defer fake.getPackageMutex.RUnlock()
 	fake.getPinnipedIssuerURLAndCAMutex.RLock()
 	defer fake.getPinnipedIssuerURLAndCAMutex.RUnlock()
 	fake.getRegionalClusterDefaultProviderNameMutex.RLock()


Test patch:
diff --git a/pkg/v1/tkg/client/package_helper_test.go b/pkg/v1/tkg/client/package_helper_test.go
index 095a88e0a2..60a200b782 100644
--- a/pkg/v1/tkg/client/package_helper_test.go
+++ b/pkg/v1/tkg/client/package_helper_test.go
@@ -4,6 +4,7 @@
 package client_test
 
 import (
+	"fmt"
 	"os"
 	"time"
 
@@ -13,32 +14,75 @@ import (
 
 	"sigs.k8s.io/yaml"
 
+	kapppkgv1alpha1 "github.com/vmware-tanzu/carvel-kapp-controller/pkg/apiserver/apis/datapackaging/v1alpha1"
 	runtanzuv1alpha3 "github.com/vmware-tanzu/tanzu-framework/apis/run/v1alpha3"
 	. "github.com/vmware-tanzu/tanzu-framework/pkg/v1/tkg/client"
+	"github.com/vmware-tanzu/tanzu-framework/pkg/v1/tkg/clusterclient"
 	"github.com/vmware-tanzu/tanzu-framework/pkg/v1/tkg/constants"
 	"github.com/vmware-tanzu/tanzu-framework/pkg/v1/tkg/fakes"
 )
 
 const bootstrapObject = "../fakes/config/clusterbootstrap.yaml"
+const kappControllerObj = "../fakes/config/kapp-controller_object.yaml"
 
 var (
 	fakeMgtClusterClient *fakes.ClusterClient
 	fakeWcClusterClient  *fakes.ClusterClient
 	timeout              time.Duration
 	clusterBootstrap     *runtanzuv1alpha3.ClusterBootstrap
+	pkg_kapp             *kapppkgv1alpha1.Package
+	pkg_cni              *kapppkgv1alpha1.Package
+	pkg_csi              *kapppkgv1alpha1.Package
+	pkg_cpi              *kapppkgv1alpha1.Package
 )
 
+const pkg_kappObjStr = `apiVersion: packaging.carvel.dev/v1alpha1
+kind: PackageInstall
+spec:
+    refname: kapp-controller.tanzu.vmware.com
+    version: 0.38.4+vmware.1-tkg.2-zshippable`
+const pkg_cniObjStr = `apiVersion: packaging.carvel.dev/v1alpha1
+kind: PackageInstall
+metadata:
+spec:
+    refname: antrea.tanzu.vmware.com
+    version: 0.38.4+vmware.1-tkg.2-zshippable`
+const pkg_csiObjStr = `apiVersion: packaging.carvel.dev/v1alpha1
+kind: PackageInstall
+spec:
+    refname: vsphere-pv-csi.tanzu.vmware.com
+    version: 0.38.4+vmware.1-tkg.2-zshippable`
+
+const pkg_cpiObjStr = `apiVersion: packaging.carvel.dev/v1alpha1
+kind: PackageInstall
+spec:
+    refname: vsphere-cpi.tanzu.vmware.com
+    version: 0.38.4+vmware.1-tkg.2-zshippable`
+
 func init() {
-	fakeMgtClusterClient = &fakes.ClusterClient{}
-	fakeWcClusterClient = &fakes.ClusterClient{}
 	timeout = time.Duration(1)
+	bs, _ := os.ReadFile(bootstrapObject)
+	clusterBootstrap = &runtanzuv1alpha3.ClusterBootstrap{}
+	//Expect(yaml.Unmarshal(bs, clusterBootstrap)).To(Succeed(), "Failed to convert the cluster bootstrap input file to yaml")
+	yaml.Unmarshal(bs, clusterBootstrap)
+	pkg_kapp = &kapppkgv1alpha1.Package{}
+	yaml.Unmarshal([]byte(pkg_kappObjStr), pkg_kapp)
+
+	pkg_cni = &kapppkgv1alpha1.Package{}
+	yaml.Unmarshal([]byte(pkg_cniObjStr), pkg_cni)
+
+	pkg_csi = &kapppkgv1alpha1.Package{}
+	yaml.Unmarshal([]byte(pkg_csiObjStr), pkg_csi)
 
+	pkg_cpi = &kapppkgv1alpha1.Package{}
+	yaml.Unmarshal([]byte(pkg_cpiObjStr), pkg_cpi)
 }
 
 var _ = Describe("unit tests for monitor addon's packages installation", func() {
 	Context("get bootstrap object for workload cluster", func() {
 		When("bootstrap object exists should not return any error", func() {
 			BeforeEach(func() {
+				setFakeClientAndCalls()
 				fakeMgtClusterClient.GetResourceReturns(nil)
 			})
 			It("should not return error", func() {
@@ -49,6 +93,7 @@ var _ = Describe("unit tests for monitor addon's packages installation", func()
 		When("bootstrap object not exists should return an error", func() {
 			resourceNotExists := "resource not exists"
 			BeforeEach(func() {
+				setFakeClientAndCalls()
 				fakeMgtClusterClient.GetResourceReturns(errors.New(resourceNotExists))
 			})
 			It("should return error", func() {
@@ -58,19 +103,14 @@ var _ = Describe("unit tests for monitor addon's packages installation", func()
 		})
 	})
 	Context("get packages from bootstrap object and monitor packages installation", func() {
-		BeforeEach(func() {
-			bs, _ := os.ReadFile(bootstrapObject)
-			clusterBootstrap = &runtanzuv1alpha3.ClusterBootstrap{}
-			Expect(yaml.Unmarshal(bs, clusterBootstrap)).To(Succeed(), "Failed to convert the cluster bootstrap input file to yaml")
-		})
 		When("package installation successful should not return error", func() {
 			BeforeEach(func() {
-				fakeMgtClusterClient.WaitForPackageInstallReturns(nil)
-				fakeWcClusterClient.WaitForPackageInstallReturns(nil)
+				setFakeClientAndCalls()
 			})
 			It("should not return error", func() {
-				packages := GetCorePackagesFromClusterBootstrap(clusterBootstrap, constants.CorePackagesNamespaceInTKGM)
-				err := MonitorAddonsCorePackageInstallation(fakeMgtClusterClient, fakeWcClusterClient, packages, timeout)
+				packages, err := GetCorePackagesFromClusterBootstrap(fakeMgtClusterClient, fakeWcClusterClient, clusterBootstrap, constants.CorePackagesNamespaceInTKGM, clusterBootstrap.Name)
+				Expect(err).NotTo(HaveOccurred())
+				err = MonitorAddonsCorePackageInstallation(fakeMgtClusterClient, fakeWcClusterClient, packages, timeout)
 				pkg, ns, _ := fakeMgtClusterClient.WaitForPackageInstallArgsForCall(0)
 				Expect(pkg).To(Equal(packages[0].ObjectMeta.Name))
 				Expect(ns).To(Equal(packages[0].ObjectMeta.Namespace))
@@ -80,14 +120,31 @@ var _ = Describe("unit tests for monitor addon's packages installation", func()
 				Expect(err).NotTo(HaveOccurred())
 			})
 		})
+
+		When("GetPackage returns error then GetCorePackagesFromClusterBootstrap also should return error", func() {
+			errorStr := fmt.Sprintf(clusterclient.ErrUnableToGetPackage, "kapp-controller.tanzu.vmware.com.0.38.4+vmware.1-tkg.1-zshippable", clusterBootstrap.Namespace)
+			BeforeEach(func() {
+				setFakeClientAndCalls()
+				err := errors.New(errorStr)
+				fakeMgtClusterClient.GetPackageReturnsOnCall(0, pkg_kapp, err)
+			})
+			It("should return error", func() {
+				_, err := GetCorePackagesFromClusterBootstrap(fakeMgtClusterClient, fakeWcClusterClient, clusterBootstrap, constants.CorePackagesNamespaceInTKGM, clusterBootstrap.Name)
+				Expect(err).To(HaveOccurred())
+				Expect(err.Error()).To(ContainSubstring(errorStr))
+			})
+		})
+
 		When("package installation not successful because of MC packages error, should return error", func() {
 			packageNotFound := "package not found"
 			BeforeEach(func() {
+				setFakeClientAndCalls()
 				fakeMgtClusterClient.WaitForPackageInstallReturns(errors.New(packageNotFound))
 			})
 			It("should return error", func() {
-				packages := GetCorePackagesFromClusterBootstrap(clusterBootstrap, constants.CorePackagesNamespaceInTKGM)
-				err := MonitorAddonsCorePackageInstallation(fakeMgtClusterClient, fakeWcClusterClient, packages, timeout)
+				packages, err := GetCorePackagesFromClusterBootstrap(fakeMgtClusterClient, fakeWcClusterClient, clusterBootstrap, constants.CorePackagesNamespaceInTKGM, clusterBootstrap.Name)
+				Expect(err).NotTo(HaveOccurred())
+				err = MonitorAddonsCorePackageInstallation(fakeMgtClusterClient, fakeWcClusterClient, packages, timeout)
 				Expect(err).To(HaveOccurred())
 				Expect(err.Error()).To(ContainSubstring(packageNotFound))
 			})
@@ -95,11 +152,13 @@ var _ = Describe("unit tests for monitor addon's packages installation", func()
 		When("package installation not successful because of WC packages error, should return error", func() {
 			packageNotFound := "package not found"
 			BeforeEach(func() {
+				setFakeClientAndCalls()
 				fakeWcClusterClient.WaitForPackageInstallReturns(errors.New(packageNotFound))
 			})
 			It("should return error", func() {
-				packages := GetCorePackagesFromClusterBootstrap(clusterBootstrap, constants.CorePackagesNamespaceInTKGM)
-				err := MonitorAddonsCorePackageInstallation(fakeMgtClusterClient, fakeWcClusterClient, packages, timeout)
+				packages, err := GetCorePackagesFromClusterBootstrap(fakeMgtClusterClient, fakeWcClusterClient, clusterBootstrap, constants.CorePackagesNamespaceInTKGM, clusterBootstrap.Name)
+				Expect(err).NotTo(HaveOccurred())
+				err = MonitorAddonsCorePackageInstallation(fakeMgtClusterClient, fakeWcClusterClient, packages, timeout)
 				Expect(err).To(HaveOccurred())
 				Expect(err.Error()).To(ContainSubstring(packageNotFound))
 			})
@@ -107,3 +166,14 @@ var _ = Describe("unit tests for monitor addon's packages installation", func()
 	})
 
 })
+
+func setFakeClientAndCalls() {
+	fakeMgtClusterClient = &fakes.ClusterClient{}
+	fakeWcClusterClient = &fakes.ClusterClient{}
+	fakeMgtClusterClient.WaitForPackageInstallReturns(nil)
+	fakeMgtClusterClient.GetPackageReturnsOnCall(0, pkg_kapp, nil)
+	fakeWcClusterClient.WaitForPackageInstallReturns(nil)
+	fakeWcClusterClient.GetPackageReturnsOnCall(0, pkg_cni, nil)
+	fakeWcClusterClient.GetPackageReturnsOnCall(1, pkg_csi, nil)
+	fakeWcClusterClient.GetPackageReturnsOnCall(2, pkg_cpi, nil)
+}