Display a warning that "odo dev" on Podman needs to be restarted if the Devfile is changed (redhat-developer#6477)

* Display a warning that "odo dev" on Podman might need to be restarted when local changes are detected

The initial idea was to display such a warning only
when changes are detected in the Devfile,
which are not yet handled completely at this time.
But to take into account the manual synchronization case (where
there is no file watcher (and therefore no way to determine which
files have actually been modified)), a generic warning is being
displayed whenever the Podman-specific watch handler is called.

* Add integration test case

* fixup! Add integration test case

* amend! Display a warning that "odo dev" on Podman might need to be restarted when local changes are detected

Display a warning that "odo dev" on Podman might need to be restarted when the Devfile is modified

* fixup! Add integration test case

* Assert that no warning is displayed if the Devfile has not changed

Author: rm3l

pkg/dev/podmandev/podmandev.go

@@ -1,16 +1,24 @@
 package podmandev
 
 import (
+	"bytes"
 	"context"
+	"encoding/json"
 	"fmt"
 	"io"
 	"path/filepath"
 	"strings"
 
+	"github.com/devfile/library/pkg/devfile/parser"
+	"k8s.io/klog"
+
 	"github.com/redhat-developer/odo/pkg/dev"
 	"github.com/redhat-developer/odo/pkg/dev/common"
+	"github.com/redhat-developer/odo/pkg/devfile"
 	"github.com/redhat-developer/odo/pkg/devfile/adapters"
+	"github.com/redhat-developer/odo/pkg/devfile/location"
 	"github.com/redhat-developer/odo/pkg/exec"
+	"github.com/redhat-developer/odo/pkg/log"
 	odocontext "github.com/redhat-developer/odo/pkg/odo/context"
 	"github.com/redhat-developer/odo/pkg/podman"
 	"github.com/redhat-developer/odo/pkg/state"
@@ -158,6 +166,8 @@ func (o *DevClient) checkVolumesFree(pod *corev1.Pod) error {
 }
 
 func (o *DevClient) watchHandler(ctx context.Context, pushParams adapters.PushParameters, watchParams watch.WatchParameters, componentStatus *watch.ComponentStatus) error {
+	printWarningsOnDevfileChanges(ctx, watchParams)
+
 	startOptions := dev.StartOptions{
 		IgnorePaths:  watchParams.FileIgnores,
 		Debug:        watchParams.Debug,
@@ -169,3 +179,35 @@ func (o *DevClient) watchHandler(ctx context.Context, pushParams adapters.PushPa
 	}
 	return o.reconcile(ctx, watchParams.Out, watchParams.ErrOut, startOptions, componentStatus)
 }
+
+func printWarningsOnDevfileChanges(ctx context.Context, parameters watch.WatchParameters) {
+	var warning string
+	currentDevfile := odocontext.GetDevfileObj(ctx)
+	newDevfile, err := devfile.ParseAndValidateFromFileWithVariables(location.DevfileLocation(""), parameters.Variables)
+	if err != nil {
+		warning = fmt.Sprintf("error while reading the Devfile. Please restart 'odo dev' if you made any changes to the Devfile. Error message is: %v", err)
+	} else {
+		devfileEquals := func(d1, d2 parser.DevfileObj) (bool, error) {
+			// Compare two Devfile objects by comparing the result of their JSON encoding,
+			// because reflect.DeepEqual does not work properly with the parser.DevfileObj structure.
+			d1Json, jsonErr := json.Marshal(d1.Data)
+			if jsonErr != nil {
+				return false, jsonErr
+			}
+			d2Json, jsonErr := json.Marshal(d2.Data)
+			if jsonErr != nil {
+				return false, jsonErr
+			}
+			return bytes.Equal(d1Json, d2Json), nil
+		}
+		equal, eqErr := devfileEquals(*currentDevfile, newDevfile)
+		if eqErr != nil {
+			klog.V(5).Infof("error while checking if Devfile has changed: %v", eqErr)
+		} else if !equal {
+			warning = "Detected changes in the Devfile, but this is not supported yet on Podman. Please restart 'odo dev' for such changes to be applied."
+		}
+	}
+	if warning != "" {
+		log.Fwarning(parameters.Out, warning+"\n")
+	}
+}

tests/integration/cmd_dev_test.go

@@ -692,45 +692,61 @@ ComponentSettings:
 							Expect(err).ToNot(HaveOccurred())
 						})
 
-						if !podman {
-							When("modifying memoryLimit for container in Devfile", func() {
-								BeforeEach(func() {
-									src := "memoryLimit: 1024Mi"
-									dst := "memoryLimit: 1023Mi"
-									helper.ReplaceString("devfile.yaml", src, dst)
-									if manual {
-										if os.Getenv("SKIP_KEY_PRESS") == "true" {
-											Skip("This is a unix-terminal specific scenario, skipping")
-										}
-
-										devSession.PressKey('p')
+						When("modifying memoryLimit for container in Devfile", func() {
+							var stdout string
+							var stderr string
+							BeforeEach(func() {
+								src := "memoryLimit: 1024Mi"
+								dst := "memoryLimit: 1023Mi"
+								helper.ReplaceString("devfile.yaml", src, dst)
+								if manual {
+									if os.Getenv("SKIP_KEY_PRESS") == "true" {
+										Skip("This is a unix-terminal specific scenario, skipping")
 									}
-									var err error
-									_, _, ports, err = devSession.WaitSync()
-									Expect(err).Should(Succeed())
-								})
 
-								It("should expose the endpoint on localhost", func() {
+									devSession.PressKey('p')
+								}
+								var err error
+								var stdoutBytes []byte
+								var stderrBytes []byte
+								stdoutBytes, stderrBytes, ports, err = devSession.WaitSync()
+								Expect(err).Should(Succeed())
+								stdout = string(stdoutBytes)
+								stderr = string(stderrBytes)
+							})
+
+							It("should react on the Devfile modification", func() {
+								if podman {
+									By("warning users that odo dev needs to be restarted", func() {
+										Expect(stdout).To(ContainSubstring(
+											"Detected changes in the Devfile, but this is not supported yet on Podman. Please restart 'odo dev' for such changes to be applied."))
+									})
+								} else {
+									By("not warning users that odo dev needs to be restarted", func() {
+										warning := "Please restart 'odo dev'"
+										Expect(stdout).ShouldNot(ContainSubstring(warning))
+										Expect(stderr).ShouldNot(ContainSubstring(warning))
+									})
 									By("updating the pod", func() {
 										podName := commonVar.CliRunner.GetRunningPodNameByComponent(cmpName, commonVar.Project)
 										bufferOutput := commonVar.CliRunner.Run("get", "pods", podName, "-o", "jsonpath='{.spec.containers[0].resources.requests.memory}'").Out.Contents()
 										output := string(bufferOutput)
 										Expect(output).To(ContainSubstring("1023Mi"))
 									})
+								}
 
-									By("exposing the endpoint", func() {
-										url := fmt.Sprintf("http://%s", ports["3000"])
-										resp, err := http.Get(url)
-										Expect(err).ToNot(HaveOccurred())
-										defer resp.Body.Close()
+								By("exposing the endpoint", func() {
+									url := fmt.Sprintf("http://%s", ports["3000"])
+									resp, err := http.Get(url)
+									Expect(err).ToNot(HaveOccurred())
+									defer resp.Body.Close()
 
-										body, _ := io.ReadAll(resp.Body)
-										helper.MatchAllInOutput(string(body), []string{"Hello from Node.js Starter Application!"})
-										Expect(err).ToNot(HaveOccurred())
-									})
+									body, _ := io.ReadAll(resp.Body)
+									helper.MatchAllInOutput(string(body), []string{"Hello from Node.js Starter Application!"})
+									Expect(err).ToNot(HaveOccurred())
 								})
 							})
-						}
+						})
 					})
 				})
 
@@ -792,9 +808,20 @@ ComponentSettings:
 								devSession.PressKey('p')
 							}
 
-							_, _, _, err := devSession.WaitSync()
+							var stdout, stderr []byte
+							var err error
+							stdout, stderr, _, err = devSession.WaitSync()
 							Expect(err).Should(Succeed())
 
+							By("not warning users that odo dev needs to be restarted because the Devfile has not changed", func() {
+								warning := "Please restart 'odo dev'"
+								if podman {
+									warning = "Detected changes in the Devfile, but this is not supported yet on Podman. Please restart 'odo dev' for such changes to be applied."
+								}
+								Expect(stdout).ShouldNot(ContainSubstring(warning))
+								Expect(stderr).ShouldNot(ContainSubstring(warning))
+							})
+
 							for _, p := range containerPorts {
 								By(fmt.Sprintf("returning the right response when querying port forwarded for container port %d", p),
 									func() {