Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 42 additions & 0 deletions pkg/dev/podmandev/podmandev.go
Original file line number Diff line number Diff line change
@@ -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"
Expand Down Expand Up @@ -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,
Expand All @@ -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")
}
}
83 changes: 55 additions & 28 deletions tests/integration/cmd_dev_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
})
})
}
})
})
})

Expand Down Expand Up @@ -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() {
Expand Down