Follow-up to comments on #6654 (support for autoBuild and `deployBy…

…Default`) (#6720)

* Remove additional non-nil checks on component fields since they are already filtered by type

We are confident that the fields we expect
(comp.{Image,Kubernetes,OpenShift}) are non-nil.

* Rename 'libdevfile.GetImageComponentsToPush' into 'libdevfile.GetImageComponentsToPushAutomatically'

* Document with examples cases where 'deployByDefault' and 'autoBuild' are `false` and the component is not referenced

* Rename 'image-autobuild-true-and-referenced' command into 'image-autobuild-true' in test Devfile

* Simplify parsing the output on Podman in the tests when looking for K8s/OpenShift components

* Use 'source/nodejs' instead of 'source/devfiles/nodejs/project' in the tests

* Revert "Rename 'image-autobuild-true-and-referenced' command into 'image-autobuild-true' in test Devfile"

This reverts commit 65812b8.

docs/website/docs/development/devfile.md

@@ -164,7 +164,7 @@ variables:
   CONTAINER_IMAGE_REPO: localhost:5000/odo-dev/node
 
 components:
-  # autoBuild true => automatically created on startup
+  # autoBuild set to true => automatically created on startup
   - image:
       autoBuild: true
       dockerfile:
@@ -173,7 +173,7 @@ components:
       imageName: "{{ CONTAINER_IMAGE_REPO }}:autobuild-true"
     name: "autobuild-true"
 
-  # autoBuild false, referenced in apply command => created when apply command is invoked
+  # autoBuild set to false, referenced in apply command => created when apply command is invoked
   - image:
       autoBuild: false
       dockerfile:
@@ -189,6 +189,15 @@ components:
         uri: Dockerfile
       imageName: "{{ CONTAINER_IMAGE_REPO }}:autobuild-not-set-and-not-referenced"
     name: "autobuild-not-set-and-not-referenced"
+    
+  # autoBuild set to false, not referenced in apply command => never created
+  - image:
+      autoBuild: false
+      dockerfile:
+        buildContext: .
+        uri: Dockerfile
+      imageName: "{{ CONTAINER_IMAGE_REPO }}:autobuild-false-and-not-referenced"
+    name: "autobuild-false-and-not-referenced"
 
   - container:
       image: "{{ CONTAINER_IMAGE_REPO }}:autobuild-not-set-and-not-referenced"
@@ -224,6 +233,8 @@ When running `odo`, the following Image components will be applied automatically
 Because the `image` component `autobuild-false-and-referenced` is referenced by the `apply` command `image-autobuild-false-and-referenced`, it will be applied when this command
 is invoked, that is, in this example, when the `run` or `deploy` commands are invoked.
 
+Because the `image` component `autobuild-false-and-not-referenced` has `autoBuild` set to `false` and is not referenced by any `apply` commands, it will never be applied.
+
 
 #### `deployByDefault` for Kubernetes/OpenShift Components
 
@@ -239,14 +250,14 @@ variables:
   CONTAINER_IMAGE_REPO: localhost:5000/odo-dev/node
 
 components:
-  # deployByDefault true => automatically created on startup
+  # deployByDefault set to true => automatically created on startup
   - kubernetes:
       deployByDefault: true
       inlined: |
         [...]
     name: "k8s-deploybydefault-true"
 
-  # deployByDefault false, referenced in apply command => created when apply command is invoked
+  # deployByDefault set to false, referenced in apply command => created when apply command is invoked
   - kubernetes:
       deployByDefault: false
       inlined: |
@@ -259,6 +270,13 @@ components:
         [...]
     name: "k8s-deploybydefault-not-set-and-not-referenced"
 
+  # deployByDefault set to false, not referenced in apply command => never created
+  - kubernetes:
+      deployByDefault: false
+      inlined: |
+        [...]
+    name: "k8s-deploybydefault-false-and-not-referenced"
+
   - container:
       image: "{{ CONTAINER_IMAGE_REPO }}:autobuild-not-set-and-not-referenced"
     name: runtime
@@ -290,9 +308,10 @@ When running `odo`, the following Kubernetes components will be applied automati
 - `k8s-deploybydefault-true` because it has `deployByDefault` set to `true`.
 - `k8s-deploybydefault-not-set-and-not-referenced` because it doesn't set `deployByDefault` and is not referenced by any `apply` commands.
 
-Because the `kubernetes` component `k8s-deploybydefault-false-and-referenced` is referenced by the `apply` command `apply-k8s-deploybydefault-false-and-referenced` , it will be applied when this command
+Because the `kubernetes` component `k8s-deploybydefault-false-and-referenced` is referenced by the `apply` command `apply-k8s-deploybydefault-false-and-referenced`, it will be applied when this command
 is invoked, that is, in this example, when the `run` or `deploy` commands are invoked.
 
+Because the `kubernetes` component `k8s-deploybydefault-false-and-not-referenced` has `deployByDefault` set to `false` and is not referenced by any `apply` commands, it will never be applied.
 
 ## File Reference
 

pkg/deploy/deploy.go

@@ -67,7 +67,7 @@ func (o *DeployClient) Deploy(ctx context.Context) error {
 }
 
 func (o *DeployClient) buildPushAutoImageComponents(handler *deployHandler, devfileObj parser.DevfileObj) error {
-	components, err := libdevfile.GetImageComponentsToPush(devfileObj)
+	components, err := libdevfile.GetImageComponentsToPushAutomatically(devfileObj)
 	if err != nil {
 		return err
 	}

pkg/dev/podmandev/reconcile.go

@@ -169,7 +169,7 @@ func (o *DevClient) warnAboutK8sComponents(devfileObj parser.DevfileObj) {
 }
 
 func (o *DevClient) buildPushAutoImageComponents(ctx context.Context, devfileObj parser.DevfileObj) error {
-	components, err := libdevfile.GetImageComponentsToPush(devfileObj)
+	components, err := libdevfile.GetImageComponentsToPushAutomatically(devfileObj)
 	if err != nil {
 		return err
 	}

pkg/devfile/adapters/kubernetes/component/push.go

@@ -41,7 +41,7 @@ func (a *Adapter) getComponentDeployment() (*appsv1.Deployment, bool, error) {
 }
 
 func (a *Adapter) buildPushAutoImageComponents(ctx context.Context, fs filesystem.Filesystem, devfileObj parser.DevfileObj, compStatus *watch.ComponentStatus) error {
-	components, err := libdevfile.GetImageComponentsToPush(devfileObj)
+	components, err := libdevfile.GetImageComponentsToPushAutomatically(devfileObj)
 	if err != nil {
 		return err
 	}

pkg/libdevfile/component_image.go

@@ -29,10 +29,10 @@ func (e *imageComponent) Apply(handler Handler) error {
 	return handler.ApplyImage(e.component)
 }
 
-// GetImageComponentsToPush returns the list of Image components that can be automatically created on startup.
+// GetImageComponentsToPushAutomatically returns the list of Image components that can be automatically created on startup.
 // The list returned is governed by the AutoBuild field in each component.
 // All components with AutoBuild set to true are included, along with those with no AutoBuild set and not-referenced.
-func GetImageComponentsToPush(devfileObj parser.DevfileObj) ([]v1alpha2.Component, error) {
+func GetImageComponentsToPushAutomatically(devfileObj parser.DevfileObj) ([]v1alpha2.Component, error) {
 	imageComponents, err := devfileObj.Data.GetComponents(parsercommon.DevfileOptions{
 		ComponentOptions: parsercommon.ComponentOptions{ComponentType: v1alpha2.ImageComponentType},
 	})
@@ -49,9 +49,6 @@ func GetImageComponentsToPush(devfileObj parser.DevfileObj) ([]v1alpha2.Componen
 
 	m := make(map[string]v1alpha2.Component)
 	for _, comp := range imageComponents {
-		if comp.Image == nil {
-			continue
-		}
 		var add bool
 		if comp.Image.AutoBuild == nil {
 			// auto-created only if not referenced by any apply command

pkg/libdevfile/component_image_test.go

@@ -16,7 +16,7 @@ import (
 	devfiletesting "github.com/redhat-developer/odo/pkg/devfile/testing"
 )
 
-func TestGetImageComponentsToPush(t *testing.T) {
+func TestGetImageComponentsToPushAutomatically(t *testing.T) {
 	fs := devfileFileSystem.NewFakeFs()
 
 	buildImageComponent := func(name string, autoBuild *bool, referenced bool) (devfilev1.Component, devfilev1.Command) {
@@ -182,9 +182,9 @@ func TestGetImageComponentsToPush(t *testing.T) {
 				return
 			}
 
-			got, err := GetImageComponentsToPush(devfileObj)
+			got, err := GetImageComponentsToPushAutomatically(devfileObj)
 			if (err != nil) != tt.wantErr {
-				t.Errorf("GetImageComponentsToPush() error = %v, wantErr %v", err, tt.wantErr)
+				t.Errorf("GetImageComponentsToPushAutomatically() error = %v, wantErr %v", err, tt.wantErr)
 				return
 			}
 			if len(got) != len(tt.want) {
@@ -195,7 +195,7 @@ func TestGetImageComponentsToPush(t *testing.T) {
 				return x.Name < y.Name
 			}
 			if diff := cmp.Diff(tt.want, got, cmpopts.EquateEmpty(), cmpopts.SortSlices(lessFn)); diff != "" {
-				t.Errorf("GetImageComponentsToPush() mismatch (-want +got):\n%s", diff)
+				t.Errorf("GetImageComponentsToPushAutomatically() mismatch (-want +got):\n%s", diff)
 			}
 		})
 	}

pkg/libdevfile/component_kubernetes.go

@@ -34,23 +34,23 @@ func (e *kubernetesComponent) Apply(handler Handler) error {
 // All components with DeployByDefault set to true are included, along with those with no DeployByDefault set and not-referenced.
 // It takes an additional allowApply boolean, which set to true, will append the components referenced from apply commands to the list.
 func GetK8sAndOcComponentsToPush(devfileObj parser.DevfileObj, allowApply bool) ([]v1alpha2.Component, error) {
-	var allComponents []v1alpha2.Component
+	var allK8sAndOcComponents []v1alpha2.Component
 
 	k8sComponents, err := devfileObj.Data.GetComponents(parsercommon.DevfileOptions{
 		ComponentOptions: parsercommon.ComponentOptions{ComponentType: v1alpha2.KubernetesComponentType},
 	})
 	if err != nil {
 		return nil, err
 	}
-	allComponents = append(allComponents, k8sComponents...)
+	allK8sAndOcComponents = append(allK8sAndOcComponents, k8sComponents...)
 
 	ocComponents, err := devfileObj.Data.GetComponents(parsercommon.DevfileOptions{
 		ComponentOptions: parsercommon.ComponentOptions{ComponentType: v1alpha2.OpenshiftComponentType},
 	})
 	if err != nil {
 		return nil, err
 	}
-	allComponents = append(allComponents, ocComponents...)
+	allK8sAndOcComponents = append(allK8sAndOcComponents, ocComponents...)
 
 	allApplyCommands, err := devfileObj.Data.GetCommands(parsercommon.DevfileOptions{
 		CommandOptions: parsercommon.CommandOptions{CommandType: v1alpha2.ApplyCommandType},
@@ -60,10 +60,7 @@ func GetK8sAndOcComponentsToPush(devfileObj parser.DevfileObj, allowApply bool)
 	}
 
 	m := make(map[string]v1alpha2.Component)
-	for _, comp := range allComponents {
-		if comp.Kubernetes == nil && comp.Openshift == nil {
-			continue
-		}
+	for _, comp := range allK8sAndOcComponents {
 		var k v1alpha2.K8sLikeComponent
 		if comp.Kubernetes != nil {
 			k = comp.Kubernetes.K8sLikeComponent

tests/examples/source/nodejs/package.json

@@ -14,6 +14,7 @@
   },
   "scripts": {
     "dev": "nodemon --ignore node_modules/ server.js",
-    "start": "node server.js"
+    "start": "node server.js",
+    "debug": "node --inspect=${DEBUG_PORT_PROJECT} server.js"
   }
 }

tests/helper/helper_podman.go

@@ -4,6 +4,8 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
+	"regexp"
+	"strings"
 
 	. "github.com/onsi/gomega"
 )
@@ -18,3 +20,25 @@ namespace=%q
 	Expect(err).ShouldNot(HaveOccurred())
 	os.Setenv("CONTAINERS_CONF", containersConfPath)
 }
+
+// ExtractK8sAndOcComponentsFromOutputOnPodman extracts the list of Kubernetes and OpenShift components from the "odo" output on Podman.
+func ExtractK8sAndOcComponentsFromOutputOnPodman(out string) []string {
+	lines, err := ExtractLines(out)
+	Expect(err).ShouldNot(HaveOccurred())
+
+	var handled []string
+	// Example lines to match:
+	// ⚠ Kubernetes components are not supported on Podman. Skipping: k8s-deploybydefault-true-and-referenced, k8s-deploybydefault-true-and-not-referenced.
+	// ⚠ OpenShift components are not supported on Podman. Skipping: ocp-deploybydefault-true-and-referenced.
+	// ⚠  Apply OpenShift components are not supported on Podman. Skipping: k8s-deploybydefault-true-and-referenced.
+	// ⚠  Apply OpenShift components are not supported on Podman. Skipping: k8s-deploybydefault-true-and-referenced.
+	re := regexp.MustCompile(`(?:Kubernetes|OpenShift) components are not supported on Podman\.\s*Skipping:\s*([^\n]+)\.`)
+	for _, l := range lines {
+		matches := re.FindStringSubmatch(l)
+		if len(matches) > 1 {
+			handled = append(handled, strings.Split(matches[1], ", ")...)
+		}
+	}
+
+	return handled
+}

tests/integration/cmd_dev_debug_test.go

@@ -456,8 +456,7 @@ var _ = Describe("odo dev debug command tests", func() {
 		When("starting with Devfile with autoBuild or deployByDefault components",
 			helper.LabelPodmanIf(podman, func() {
 				BeforeEach(func() {
-					helper.CopyExample(filepath.Join("source", "devfiles", "nodejs", "project"), commonVar.Context)
-					helper.CopyExample(filepath.Join("source", "nodejs", "Dockerfile"), filepath.Join(commonVar.Context, "Dockerfile"))
+					helper.CopyExample(filepath.Join("source", "nodejs"), commonVar.Context)
 					helper.CopyExampleDevFile(filepath.Join("source", "devfiles", "nodejs", "devfile-autobuild-deploybydefault.yaml"),
 						filepath.Join(commonVar.Context, "devfile.yaml"),
 						helper.DevfileMetadataNameSetter(cmpName))
@@ -499,43 +498,27 @@ var _ = Describe("odo dev debug command tests", func() {
 
 					It("should create the appropriate resources", func() {
 						if podman {
-							By("skipping Kubernetes/OpenShift components that would have been created automatically", func() {
-								linesErr, _ := helper.ExtractLines(stderr)
-								var skipped []string
-								for _, l := range linesErr {
-									if strings.Contains(l, "Kubernetes components are not supported on Podman. Skipping:") {
-										sl := strings.SplitN(l, ": ", 2)
-										if len(sl) < 2 {
-											break
-										}
-										for _, s := range strings.Split(sl[1], ", ") {
-											skipped = append(skipped, strings.TrimSuffix(s, "."))
-										}
-										break
-									}
-								}
-								expected := []string{
+							k8sOcComponents := helper.ExtractK8sAndOcComponentsFromOutputOnPodman(stderr)
+							By("handling Kubernetes/OpenShift components that would have been created automatically", func() {
+								Expect(k8sOcComponents).Should(ContainElements(
 									"k8s-deploybydefault-true-and-referenced",
 									"k8s-deploybydefault-true-and-not-referenced",
 									"k8s-deploybydefault-not-set-and-not-referenced",
 									"ocp-deploybydefault-true-and-referenced",
 									"ocp-deploybydefault-true-and-not-referenced",
 									"ocp-deploybydefault-not-set-and-not-referenced",
-								}
-								Expect(skipped).Should(ConsistOf(expected))
+								))
 							})
 							By("not handling Kubernetes/OpenShift components with deployByDefault=false", func() {
-								for _, l := range []string{
+								Expect(k8sOcComponents).ShouldNot(ContainElements(
 									"k8s-deploybydefault-false-and-referenced",
 									"k8s-deploybydefault-false-and-not-referenced",
 									"ocp-deploybydefault-false-and-referenced",
 									"ocp-deploybydefault-false-and-not-referenced",
-								} {
-									Expect(stderr).ShouldNot(ContainSubstring("Skipping: %s", l))
-								}
+								))
 							})
 							By("not handling referenced Kubernetes/OpenShift components with deployByDefault unset", func() {
-								Expect(stderr).ShouldNot(ContainSubstring("Skipping: k8s-deploybydefault-not-set-and-referenced"))
+								Expect(k8sOcComponents).ShouldNot(ContainElement("k8s-deploybydefault-not-set-and-referenced"))
 							})
 						} else {
 							By("automatically applying Kubernetes/OpenShift components with deployByDefault=true", func() {
@@ -649,52 +632,34 @@ var _ = Describe("odo dev debug command tests", func() {
 
 					It("should create the appropriate resources", func() {
 						if podman {
-							By("skipping Kubernetes/OpenShift components that would have been created automatically", func() {
-								linesErr, _ := helper.ExtractLines(stderr)
-								var skipped []string
-								for _, l := range linesErr {
-									if strings.Contains(l, "Kubernetes components are not supported on Podman. Skipping:") {
-										sl := strings.SplitN(l, ": ", 2)
-										if len(sl) < 2 {
-											break
-										}
-										for _, s := range strings.Split(sl[1], ", ") {
-											skipped = append(skipped, strings.TrimSuffix(s, "."))
-										}
-										break
-									}
-								}
-								expected := []string{
+							k8sOcComponents := helper.ExtractK8sAndOcComponentsFromOutputOnPodman(stderr)
+							By("handling Kubernetes/OpenShift components to create automatically", func() {
+								Expect(k8sOcComponents).Should(ContainElements(
 									"k8s-deploybydefault-true-and-referenced",
 									"k8s-deploybydefault-true-and-not-referenced",
 									"k8s-deploybydefault-not-set-and-not-referenced",
 									"ocp-deploybydefault-true-and-referenced",
 									"ocp-deploybydefault-true-and-not-referenced",
 									"ocp-deploybydefault-not-set-and-not-referenced",
-								}
-								Expect(skipped).Should(ConsistOf(expected))
+								))
 							})
 
-							By("skipping referenced Kubernetes/OpenShift components", func() {
-								for _, l := range []string{
+							By("handling referenced Kubernetes/OpenShift components", func() {
+								Expect(k8sOcComponents).Should(ContainElements(
 									"k8s-deploybydefault-true-and-referenced",
 									"k8s-deploybydefault-false-and-referenced",
 									"k8s-deploybydefault-not-set-and-referenced",
 									"ocp-deploybydefault-true-and-referenced",
 									"ocp-deploybydefault-false-and-referenced",
 									"ocp-deploybydefault-not-set-and-referenced",
-								} {
-									Expect(stderr).Should(ContainSubstring("Skipping: %s", l))
-								}
+								))
 							})
 
 							By("not handling non-referenced Kubernetes/OpenShift components with deployByDefault=false", func() {
-								for _, l := range []string{
+								Expect(k8sOcComponents).ShouldNot(ContainElements(
 									"k8s-deploybydefault-false-and-not-referenced",
 									"ocp-deploybydefault-false-and-not-referenced",
-								} {
-									Expect(stderr).ShouldNot(ContainSubstring("Skipping: %s", l))
-								}
+								))
 							})
 						} else {
 							By("applying referenced Kubernetes/OpenShift components", func() {