Allow listing the CLIPlugins resources even if the CRD discovery API …

…returns an error for the kubernetes context (vmware-tanzu#642)

* Check the cliplugin resource even if the CRD discovery API returns error

pkg/discovery/kubernetes.go

@@ -61,21 +61,34 @@ func (k *KubernetesDiscovery) Manifest() ([]Discovered, error) {
 func (k *KubernetesDiscovery) GetDiscoveredPlugins(clusterClient cluster.Client) ([]Discovered, error) {
 	plugins := make([]Discovered, 0)
 
-	exists, err := clusterClient.VerifyCLIPluginCRD()
-	if !exists || err != nil {
-		logMsg := "Skipping context-aware plugin discovery because CLIPlugin CRD not present on the logged in cluster. "
-		if err != nil {
-			logMsg += err.Error()
-		}
+	logMsg := "Skipping context-aware plugin discovery because CLIPlugin CRD not present on the logged in cluster. "
+
+	crdExists, errVerifyCRD := clusterClient.VerifyCLIPluginCRD()
+	// If no error and CRD does not exists, return with a log message
+	// If there is an error, wait for the call of ListCLIPluginResources to return
+	// and then decide. As sometimes discovery API can return premature error
+	// if the k8s apiserver is not stable but ListCLIPluginResources might return
+	// correct result. In this case, we will ignore the error we get here.
+	if errVerifyCRD == nil && !crdExists {
 		log.V(4).Info(logMsg)
 		return nil, nil
 	}
 
-	// get all cliplugins resources available on the cluster
-	cliplugins, err := clusterClient.ListCLIPluginResources()
-	if err != nil {
-		log.V(4).Infof("error while fetching the list of CLIPlugin resources. %v", err.Error())
-		return nil, err
+	// Try to get all cliplugins resources available on the cluster
+	cliplugins, errListCLIPlugins := clusterClient.ListCLIPluginResources()
+	if errListCLIPlugins != nil {
+		// If there was an earlier error while verifying CRD, assuming that it was a legitimate
+		// error and will just log a warning and return without error
+		if errVerifyCRD != nil {
+			logMsg += errVerifyCRD.Error()
+			log.V(4).Info(logMsg)
+			return nil, nil
+		}
+
+		// If the CRD was present and we see error while listing the CLIPlugin resources
+		// log an error message and return an error
+		log.V(4).Infof("error while fetching the list of CLIPlugin resources. %v", errListCLIPlugins.Error())
+		return nil, errListCLIPlugins
 	}
 
 	log.V(4).Infof("found %v CLIPlugin resources.", len(cliplugins))

pkg/discovery/kubernetes_test.go

@@ -41,16 +41,17 @@ var _ = Describe("Unit tests for kubernetes discovery", func() {
 			plugins, err = kd.GetDiscoveredPlugins(currentClusterClient)
 		})
 
-		Context("When CLIPlugin CRD verification throws error", func() {
+		Context("When VerifyCLIPluginCRD throws error and ListCLIPluginResources also throws an error", func() {
 			BeforeEach(func() {
 				currentClusterClient.VerifyCLIPluginCRDReturns(false, errors.New("fake error"))
+				currentClusterClient.ListCLIPluginResourcesReturns(nil, errors.New("fake error"))
 			})
 			It("should return empty plugin list without an error", func() {
 				Expect(err).NotTo(HaveOccurred())
 				Expect(len(plugins)).To(Equal(0))
 			})
 		})
-		Context("When CLIPlugin CRD verification returns false and no error", func() {
+		Context("When VerifyCLIPluginCRD returns false and no error", func() {
 			BeforeEach(func() {
 				currentClusterClient.VerifyCLIPluginCRDReturns(false, nil)
 			})
@@ -59,17 +60,17 @@ var _ = Describe("Unit tests for kubernetes discovery", func() {
 				Expect(len(plugins)).To(Equal(0))
 			})
 		})
-		Context("When CLIPlugin CRD verification returns true with error", func() {
+		Context("When VerifyCLIPluginCRD returns true with error and ListCLIPluginResources also throws an error", func() {
 			BeforeEach(func() {
 				currentClusterClient.VerifyCLIPluginCRDReturns(true, errors.New("fake error"))
+				currentClusterClient.ListCLIPluginResourcesReturns(nil, errors.New("fake error"))
 			})
 			It("should return empty plugin list without an error", func() {
 				Expect(err).NotTo(HaveOccurred())
 				Expect(len(plugins)).To(Equal(0))
 			})
 		})
-
-		Context("When ListCLIPluginResources returns error", func() {
+		Context("When VerifyCLIPluginCRD does NOT return an error but ListCLIPluginResources returns error", func() {
 			BeforeEach(func() {
 				currentClusterClient.VerifyCLIPluginCRDReturns(true, nil)
 				currentClusterClient.ListCLIPluginResourcesReturns(nil, errors.New("fake error"))
@@ -79,7 +80,26 @@ var _ = Describe("Unit tests for kubernetes discovery", func() {
 				Expect(len(plugins)).To(Equal(0))
 			})
 		})
-		Context("When ListCLIPluginResources list of CLIPlugin resources", func() {
+		Context("When VerifyCLIPluginCRD returns error but the ListCLIPluginResources does not return error and returns CLIPlugin list", func() {
+			BeforeEach(func() {
+				cliplugin1 := fakehelper.NewCLIPlugin(fakehelper.TestCLIPluginOption{Name: "plugin1", Description: "plugin1 desc", RecommendedVersion: "v0.0.1"})
+				cliplugin2 := fakehelper.NewCLIPlugin(fakehelper.TestCLIPluginOption{Name: "plugin2", Description: "plugin2 desc", RecommendedVersion: "v0.0.2"})
+				cliplugins = []v1alpha1.CLIPlugin{}
+				cliplugins = append(cliplugins, cliplugin1, cliplugin2)
+				currentClusterClient.VerifyCLIPluginCRDReturns(false, errors.New("fake error verify CRD"))
+				currentClusterClient.ListCLIPluginResourcesReturns(cliplugins, nil)
+				currentClusterClient.GetCLIPluginImageRepositoryOverrideReturns(map[string]string{}, nil)
+			})
+			It("should return ordered list of plugins without error", func() {
+				Expect(len(plugins)).To(Equal(2))
+				Expect(plugins[0].Name).To(Equal("plugin1"))
+				Expect(plugins[0].RecommendedVersion).To(Equal("v0.0.1"))
+				Expect(plugins[1].Name).To(Equal("plugin2"))
+				Expect(plugins[1].RecommendedVersion).To(Equal("v0.0.2"))
+				Expect(err).NotTo(HaveOccurred())
+			})
+		})
+		Context("When VerifyCLIPluginCRD returns true without error and ListCLIPluginResources list of CLIPlugin resources", func() {
 			BeforeEach(func() {
 				cliplugin1 := fakehelper.NewCLIPlugin(fakehelper.TestCLIPluginOption{Name: "plugin1", Description: "plugin1 desc", RecommendedVersion: "v0.0.1"})
 				cliplugin2 := fakehelper.NewCLIPlugin(fakehelper.TestCLIPluginOption{Name: "plugin2", Description: "plugin2 desc", RecommendedVersion: "v0.0.2"})