Skip to content

Commit

Permalink
Delete previous pod and creates new ond when modifying devfile on pod…
Browse files Browse the repository at this point in the history 
…man (#6802)

* Delete previous pod and creates new ond when modifying devfile on podman

* Add integration tests
  • Loading branch information
@feloy
feloy authored May 10, 2023
1 parent 5459326 commit da28e7d
Show file tree
Hide file tree
Showing 5 changed files with 56 additions and 87 deletions.
10 changes: 5 additions & 5 deletions pkg/dev/podmandev/pod.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,15 +43,15 @@ func createPodFromComponent(
customForwardedPorts []api.ForwardedPort,
usedPorts []int,
customAddress string,
devfileObj parser.DevfileObj,
) (*corev1.Pod, []api.ForwardedPort, error) {
var (
appName = odocontext.GetApplication(ctx)
componentName = odocontext.GetComponentName(ctx)
devfileObj = odocontext.GetDevfileObj(ctx)
workingDir = odocontext.GetWorkingDirectory(ctx)
)

podTemplate, err := generator.GetPodTemplateSpec(*devfileObj, generator.PodTemplateParams{})
podTemplate, err := generator.GetPodTemplateSpec(devfileObj, generator.PodTemplateParams{})
if err != nil {
return nil, nil, err
}
Expand All @@ -61,7 +61,7 @@ func createPodFromComponent(
}

var fwPorts []api.ForwardedPort
fwPorts, err = getPortMapping(*devfileObj, debug, randomPorts, usedPorts, customForwardedPorts, customAddress)
fwPorts, err = getPortMapping(devfileObj, debug, randomPorts, usedPorts, customForwardedPorts, customAddress)
if err != nil {
return nil, nil, err
}
Expand All @@ -88,7 +88,7 @@ func createPodFromComponent(
},
}

devfileVolumes, err := storage.ListStorage(*devfileObj)
devfileVolumes, err := storage.ListStorage(devfileObj)
if err != nil {
return nil, nil, err
}
Expand All @@ -108,7 +108,7 @@ func createPodFromComponent(
}
}

containers, err = utils.UpdateContainersEntrypointsIfNeeded(*devfileObj, containers, buildCommand, runCommand, debugCommand)
containers, err = utils.UpdateContainersEntrypointsIfNeeded(devfileObj, containers, buildCommand, runCommand, debugCommand)
if err != nil {
return nil, nil, err
}
Expand Down
1 change: 1 addition & 0 deletions pkg/dev/podmandev/pod_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1423,6 +1423,7 @@ func Test_createPodFromComponent(t *testing.T) {
tt.args.customForwardedPorts,
[]int{20001, 20002, 20003, 20004, 20005},
tt.args.customAddress,
devfileObj,
)
if (err != nil) != tt.wantErr {
t.Errorf("createPodFromComponent() error = %v, wantErr %v", err, tt.wantErr)
Expand Down
41 changes: 5 additions & 36 deletions pkg/dev/podmandev/podmandev.go
View file
Original file line number Diff line number Diff line change
@@ -1,14 +1,11 @@
package podmandev

import (
"bytes"
"context"
"encoding/json"
"fmt"
"strings"

devfilev1 "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2"
"github.com/devfile/library/v2/pkg/devfile/parser"
"k8s.io/klog"

"github.com/redhat-developer/odo/pkg/dev"
Expand All @@ -17,7 +14,6 @@ import (
"github.com/redhat-developer/odo/pkg/devfile/location"
"github.com/redhat-developer/odo/pkg/exec"
"github.com/redhat-developer/odo/pkg/libdevfile"
"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/portForward"
Expand Down Expand Up @@ -164,39 +160,12 @@ func (o *DevClient) checkVolumesFree(pod *corev1.Pod) error {
}

func (o *DevClient) watchHandler(ctx context.Context, pushParams common.PushParameters, componentStatus *watch.ComponentStatus) error {
pushParams.Devfile = *odocontext.GetDevfileObj(ctx) // TOO reload devfile from disk
printWarningsOnDevfileChanges(ctx, pushParams.StartOptions)
return o.reconcile(ctx, pushParams, componentStatus)
}

func printWarningsOnDevfileChanges(ctx context.Context, options dev.StartOptions) {
var warning string
currentDevfile := odocontext.GetDevfileObj(ctx)
newDevfile, err := devfile.ParseAndValidateFromFileWithVariables(location.DevfileLocation(""), options.Variables)
devObj, err := devfile.ParseAndValidateFromFileWithVariables(location.DevfileLocation(""), pushParams.StartOptions.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(options.Out, warning+"\n")
return fmt.Errorf("unable to read devfile: %w", err)
}
pushParams.Devfile = devObj

return o.reconcile(ctx, pushParams, componentStatus)
}
13 changes: 11 additions & 2 deletions pkg/dev/podmandev/reconcile.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ func (o *DevClient) reconcile(
return err
}

pod, fwPorts, err := o.deployPod(ctx, options)
pod, fwPorts, err := o.deployPod(ctx, options, devfileObj)
if err != nil {
return err
}
Expand Down Expand Up @@ -197,7 +197,7 @@ func (o *DevClient) buildPushAutoImageComponents(ctx context.Context, devfileObj
}

// deployPod deploys the component as a Pod in podman
func (o *DevClient) deployPod(ctx context.Context, options dev.StartOptions) (*corev1.Pod, []api.ForwardedPort, error) {
func (o *DevClient) deployPod(ctx context.Context, options dev.StartOptions, devfileObj parser.DevfileObj) (*corev1.Pod, []api.ForwardedPort, error) {

spinner := log.Spinner("Deploying pod")
defer spinner.End(false)
Expand All @@ -213,6 +213,7 @@ func (o *DevClient) deployPod(ctx context.Context, options dev.StartOptions) (*c
options.CustomForwardedPorts,
o.usedPorts,
options.CustomAddress,
devfileObj,
)
if err != nil {
return nil, nil, err
Expand All @@ -225,6 +226,14 @@ func (o *DevClient) deployPod(ctx context.Context, options dev.StartOptions) (*c
return o.deployedPod, fwPorts, nil
}

// Delete previous volumes and pod, if running
if o.deployedPod != nil {
err = o.podmanClient.CleanupPodResources(o.deployedPod)
if err != nil {
return nil, nil, err
}
}

err = o.checkVolumesFree(pod)
if err != nil {
return nil, nil, err
Expand Down
78 changes: 34 additions & 44 deletions tests/integration/cmd_dev_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1148,7 +1148,7 @@ ComponentSettings:
Expect(err).ToNot(HaveOccurred())
})

When("modifying memoryLimit for container in Devfile", func() {
When("modifying name for container in Devfile", func() {
var stdout string
var stderr string
BeforeEach(func() {
Expand All @@ -1171,8 +1171,8 @@ ComponentSettings:
stdout = string(stdoutBytes)
stderr = string(stderrBytes)
}()
src := "memoryLimit: 1024Mi"
dst := "memoryLimit: 1023Mi"
src := "runtime"
dst := "other"
helper.ReplaceString("devfile.yaml", src, dst)
if manual {
devSession.PressKey('p')
Expand All @@ -1181,45 +1181,38 @@ ComponentSettings:
})

It(fmt.Sprintf("should react on the Devfile modification (podman=%v, manual=%v, customPortForwarding=%v, customAddress=%v)", podman, manual, customPortForwarding, customAddress), 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("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() {
component := helper.NewComponent(cmpName, "app", labels.ComponentDevMode, commonVar.Project, commonVar.CliRunner)
podDef := component.GetPodDef()
containerName := podDef.Spec.Containers[0].Name
Expect(containerName).To(ContainSubstring("other"))
})

By("exposing the endpoint", func() {
Eventually(func(g Gomega) {
url := fmt.Sprintf("http://%s", ports[containerPort])
if customPortForwarding {
Expect(url).To(ContainSubstring(strconv.Itoa(localPort)))
}
if customAddress {
Expect(url).To(ContainSubstring(localAddress))
}
resp, err := http.Get(url)
g.Expect(err).ToNot(HaveOccurred())
defer resp.Body.Close()

body, _ := io.ReadAll(resp.Body)
for _, i := range []string{"Hello from Node.js Starter Application!"} {
g.Expect(string(body)).To(ContainSubstring(i))
}
g.Expect(err).ToNot(HaveOccurred())
}).WithPolling(1 * time.Second).WithTimeout(20 * time.Second).Should(Succeed())
})
}
By("exposing the endpoint", func() {
Eventually(func(g Gomega) {
url := fmt.Sprintf("http://%s", ports[containerPort])
if customPortForwarding {
Expect(url).To(ContainSubstring(strconv.Itoa(localPort)))
}
if customAddress {
Expect(url).To(ContainSubstring(localAddress))
}
resp, err := http.Get(url)
g.Expect(err).ToNot(HaveOccurred())
defer resp.Body.Close()

body, _ := io.ReadAll(resp.Body)
for _, i := range []string{"Hello from Node.js Starter Application!"} {
g.Expect(string(body)).To(ContainSubstring(i))
}
g.Expect(err).ToNot(HaveOccurred())
}).WithPolling(1 * time.Second).WithTimeout(20 * time.Second).Should(Succeed())
})
})
})
})
Expand Down Expand Up @@ -1327,9 +1320,6 @@ ComponentSettings:

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))
})
Expand Down

0 comments on commit da28e7d

Please sign in to comment.