Fix: Non odo components not reported (#6021)

* Fix: Non odo components not reported Signed-off-by: Parthvi Vala <[email protected]> * Add unit test Signed-off-by: Parthvi Vala <[email protected]> Signed-off-by: Parthvi Vala <[email protected]>
redhat-developer · Aug 17, 2022 · b4ee872 · b4ee872
1 parent 72d632b
commit b4ee872
Show file tree

Hide file tree

Showing 5 changed files with 137 additions and 14 deletions.
diff --git a/pkg/component/component.go b/pkg/component/component.go
@@ -156,12 +156,12 @@ func ListAllClusterComponents(client kclient.ClientInterface, namespace string)
 		}
 
 		// Get the managedBy label
-		// IMPORTANT. If "managed-by" label is BLANK, it is most likely an operator
+		// IMPORTANT. If both "managed-by" and "instance" labels are BLANK, it is most likely an operator
 		// or a non-component. We do not want to show these in the list of components
 		// so we skip them if there is no "managed-by" label.
 
 		managedBy := odolabels.GetManagedBy(labels)
-		if managedBy == "" {
+		if managedBy == "" && name == "" {
 			continue
 		}
 

diff --git a/pkg/component/component_test.go b/pkg/component/component_test.go
@@ -12,14 +12,16 @@ import (
 	"github.com/redhat-developer/odo/pkg/kclient"
 	"github.com/redhat-developer/odo/pkg/labels"
 
-	"github.com/redhat-developer/odo/pkg/api"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+
+	"github.com/redhat-developer/odo/pkg/api"
 )
 
 func TestListAllClusterComponents(t *testing.T) {
 	res1 := getUnstructured("dep1", "deployment", "v1", "Unknown", "Unknown", "my-ns")
 	res2 := getUnstructured("svc1", "service", "v1", "odo", "nodejs", "my-ns")
-
+	res3 := getUnstructured("dep1", "deployment", "v1", "Unknown", "Unknown", "my-ns")
+	res3.SetLabels(map[string]string{})
 	commonLabels := labels.Builder().WithComponentName("comp1").WithManager("odo")
 
 	resDev := getUnstructured("depDev", "deployment", "v1", "odo", "nodejs", "my-ns")
@@ -66,15 +68,31 @@ func TestListAllClusterComponents(t *testing.T) {
 			}},
 			wantErr: false,
 		},
+		{
+			name: "0 non-odo resource without instance label is not returned",
+			fields: fields{
+				kubeClient: func(ctrl *gomock.Controller) kclient.ClientInterface {
+					var resources []unstructured.Unstructured
+					resources = append(resources, res3)
+					client := kclient.NewMockClientInterface(ctrl)
+					client.EXPECT().GetAllResourcesFromSelector(gomock.Any(), "my-ns").Return(resources, nil)
+					return client
+				},
+			},
+			args: args{
+				namespace: "my-ns",
+			},
+			want:    nil,
+			wantErr: false,
+		},
 		{
 			name: "1 non-odo resource returned with Unknown, and 1 odo resource returned with odo",
 			fields: fields{
 				kubeClient: func(ctrl *gomock.Controller) kclient.ClientInterface {
 					var resources []unstructured.Unstructured
 					resources = append(resources, res1, res2)
 					client := kclient.NewMockClientInterface(ctrl)
-					selector := ""
-					client.EXPECT().GetAllResourcesFromSelector(selector, "my-ns").Return(resources, nil)
+					client.EXPECT().GetAllResourcesFromSelector(gomock.Any(), "my-ns").Return(resources, nil)
 					return client
 				},
 			},
@@ -101,8 +119,7 @@ func TestListAllClusterComponents(t *testing.T) {
 					var resources []unstructured.Unstructured
 					resources = append(resources, resDev, resDeploy)
 					client := kclient.NewMockClientInterface(ctrl)
-					selector := ""
-					client.EXPECT().GetAllResourcesFromSelector(selector, "my-ns").Return(resources, nil)
+					client.EXPECT().GetAllResourcesFromSelector(gomock.Any(), "my-ns").Return(resources, nil)
 					return client
 				},
 			},

diff --git a/tests/examples/manifests/deployment-app-label.yaml b/tests/examples/manifests/deployment-app-label.yaml
@@ -5,7 +5,7 @@ metadata:
   labels:
     app: app
     app.kubernetes.io/instance: example-deployment
-    app.kubernetes.io/managed-by: console
+    app.kubernetes.io/managed-by: some-tool
     app.kubernetes.io/managed-by-version: v4.7.0
     app.kubernetes.io/name: example-deployment
     app.kubernetes.io/part-of: app
@@ -18,11 +18,6 @@ spec:
     metadata:
       labels:
         app: app
-        app.kubernetes.io/instance: example-deployment
-        app.kubernetes.io/managed-by: console
-        app.kubernetes.io/managed-by-version: v4.7.0
-        app.kubernetes.io/name: example-deployment
-        app.kubernetes.io/part-of: app
     spec:
       containers:
         - name: httpd

diff --git a/tests/examples/manifests/deployment-without-managed-by-label.yaml b/tests/examples/manifests/deployment-without-managed-by-label.yaml
@@ -0,0 +1,47 @@
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  annotations:
+    alpha.image.policy.openshift.io/resolve-names: '*'
+    app.openshift.io/vcs-ref: ""
+    app.openshift.io/vcs-uri: https://github.com/devfile-samples/devfile-sample-java-springboot-basic.git
+    deployment.kubernetes.io/revision: "2"
+    image.openshift.io/triggers: '[{"from":{"kind":"ImageStreamTag","name":"java-springboot-basic:latest","namespace":"myproject"},"fieldPath":"spec.template.spec.containers[?(@.name==\"java-springboot-basic\")].image","pause":"false"}]'
+    isFromDevfile: "true"
+    openshift.io/generated-by: OpenShiftWebConsole
+  labels:
+    app: java-springboot-basic
+    app.kubernetes.io/component: java-springboot-basic
+    app.kubernetes.io/instance: java-springboot-basic
+    app.kubernetes.io/name: java-springboot-basic
+    app.kubernetes.io/part-of: sample-app
+    app.openshift.io/runtime: java-springboot-basic
+  name: java-springboot-basic
+spec:
+  replicas: 1
+  selector:
+    matchLabels:
+      app: java-springboot-basic
+  template:
+    metadata:
+      labels:
+        app: java-springboot-basic
+    spec:
+      containers:
+      - env:
+        - name: DEBUG_PORT
+          value: "5858"
+        - name: PROJECTS_ROOT
+          value: /projects
+        - name: PROJECT_SOURCE
+          value: /projects
+        image: image-registry.openshift-image-registry.svc:5000/myproject/java-springboot-basic@sha256:4a02a410484e01984a05e50b8ca4dbb336baf03f85ce504bb7f4fdc4f18d5d9c
+        imagePullPolicy: Always
+        name: java-springboot-basic
+        ports:
+        - containerPort: 8080
+          name: 8080-tcp
+          protocol: TCP
+        resources:
+          limits:
+            memory: 768Mi
diff --git a/tests/integration/cmd_devfile_list_test.go b/tests/integration/cmd_devfile_list_test.go
@@ -5,6 +5,7 @@ import (
 	"path/filepath"
 
 	devfilepkg "github.com/devfile/api/v2/pkg/devfile"
+	"github.com/tidwall/gjson"
 
 	"github.com/redhat-developer/odo/tests/helper"
 
@@ -25,6 +26,69 @@ var _ = Describe("odo list with devfile", func() {
 	var _ = AfterEach(func() {
 		helper.CommonAfterEach(commonVar)
 	})
+	Context("listing non-odo managed components", func() {
+		When("a non-odo managed component is deployed", func() {
+			const (
+				// hard coded names from the deployment-app-label.yaml
+				deploymentName = "example-deployment"
+				managedBy      = "some-tool"
+			)
+			BeforeEach(func() {
+				commonVar.CliRunner.Run("create", "-f", helper.GetExamplePath("manifests", "deployment-app-label.yaml"))
+			})
+			AfterEach(func() {
+				commonVar.CliRunner.Run("delete", "-f", helper.GetExamplePath("manifests", "deployment-app-label.yaml"))
+			})
+			It("should list the component with odo list", func() {
+				output := helper.Cmd("odo", "list").ShouldPass().Out()
+				helper.MatchAllInOutput(output, []string{deploymentName, "Unknown", "None", managedBy})
+			})
+			It("should list the component in JSON", func() {
+				output := helper.Cmd("odo", "list", "-o", "json").ShouldPass().Out()
+				helper.JsonPathContentIs(output, "components.0.name", deploymentName)
+				Expect(gjson.Get(output, "components.0.runningIn").String()).To(BeEmpty())
+				helper.JsonPathContentIs(output, "components.0.projectType", "Unknown")
+				helper.JsonPathContentIs(output, "components.0.managedBy", managedBy)
+			})
+		})
+		When("a non-odo managed component without the managed-by label is deployed", func() {
+			const (
+				// hard coded names from the deployment-without-managed-by-label.yaml
+				deploymentName = "java-springboot-basic"
+			)
+			BeforeEach(func() {
+				commonVar.CliRunner.Run("create", "-f", helper.GetExamplePath("manifests", "deployment-without-managed-by-label.yaml"))
+			})
+			AfterEach(func() {
+				commonVar.CliRunner.Run("delete", "-f", helper.GetExamplePath("manifests", "deployment-without-managed-by-label.yaml"))
+			})
+			It("should list the component with odo list", func() {
+				output := helper.Cmd("odo", "list").ShouldPass().Out()
+				helper.MatchAllInOutput(output, []string{deploymentName, "Unknown", "None", "Unknown"})
+			})
+			It("should list the component in JSON", func() {
+				output := helper.Cmd("odo", "list", "-o", "json").ShouldPass().Out()
+				helper.JsonPathContentContain(output, "components.0.name", deploymentName)
+				Expect(gjson.Get(output, "components.0.runningIn").String()).To(BeEmpty())
+				helper.JsonPathContentContain(output, "components.0.projectType", "Unknown")
+				helper.JsonPathContentContain(output, "components.0.managedBy", "")
+			})
+		})
+		When("an operator managed deployment(without instance and managed-by label) is deployed", func() {
+			deploymentName := "nginx"
+			BeforeEach(func() {
+				commonVar.CliRunner.Run("create", "deployment", deploymentName, "--image=nginx")
+			})
+			AfterEach(func() {
+				commonVar.CliRunner.Run("delete", "deployment", deploymentName)
+			})
+			It("should not be listed in the odo list output", func() {
+				output := helper.Cmd("odo", "list").ShouldRun().Out()
+				Expect(output).ToNot(ContainSubstring(deploymentName))
+
+			})
+		})
+	})
 
 	When("a component created in 'app' application", func() {