Reuse same volumes when devfile is modified, on podman (#6810)

* Reuse same volumes when devfile is modified * Fix doc /comment
redhat-developer · May 11, 2023 · f491bcc · f491bcc
1 parent da28e7d
commit f491bcc
Show file tree

Hide file tree

Showing 8 changed files with 26 additions and 26 deletions.
diff --git a/docs/website/docs/development/architecture/podman-support-limitations.md b/docs/website/docs/development/architecture/podman-support-limitations.md
@@ -50,10 +50,6 @@ Podman is natively not able to port-forward to programs listening on localhost.
 - you can change your application to listen on `0.0.0.0`. This will be necessary for the ports giving access to the application or, in Production, this port would not be available (this port will most probably be exposed through an Ingress or a Route in Production, and these methods need the port to be bound to `0.0.0.0`),
 - you can keep the port bound to `localhost`. This is the best choice for the Debug port, to restrict access to this Debug port. In this case, you can use the flag `--forward-localhost` when running `odo dev` on Podman. This way, you keep the Debug port secure on cluster.
 
-## Pod not updated when Devfile changes
-
-When running `odo dev` on cluster, if you make changes to the Devfile affecting the definition of the deployed Pod (for example the memory or CPU requests or limits), the Pod will be recreated with its new definition. This behaviour is not supported yet for Podman.
-
 ## Pre-Stop events not supported
 
 Pre-Stop events defined in the Devfile are not triggered when running `odo dev` on Podman.

diff --git a/pkg/dev/podmandev/cleanup.go b/pkg/dev/podmandev/cleanup.go
@@ -11,5 +11,5 @@ func (o *DevClient) CleanupResources(ctx context.Context, out io.Writer) error {
 	if o.deployedPod == nil {
 		return nil
 	}
-	return o.podmanClient.CleanupPodResources(o.deployedPod)
+	return o.podmanClient.CleanupPodResources(o.deployedPod, true)
 }
diff --git a/pkg/dev/podmandev/reconcile.go b/pkg/dev/podmandev/reconcile.go
@@ -226,17 +226,17 @@ func (o *DevClient) deployPod(ctx context.Context, options dev.StartOptions, dev
 		return o.deployedPod, fwPorts, nil
 	}
 
-	// Delete previous volumes and pod, if running
+	// Delete previous pod, if running
 	if o.deployedPod != nil {
-		err = o.podmanClient.CleanupPodResources(o.deployedPod)
+		err = o.podmanClient.CleanupPodResources(o.deployedPod, false)
+		if err != nil {
+			return nil, nil, err
+		}
+	} else {
+		err = o.checkVolumesFree(pod)
 		if err != nil {
 			return nil, nil, err
 		}
-	}
-
-	err = o.checkVolumesFree(pod)
-	if err != nil {
-		return nil, nil, err
 	}
 
 	err = o.podmanClient.PlayKube(pod)

diff --git a/pkg/odo/cli/delete/component/component.go b/pkg/odo/cli/delete/component/component.go
@@ -199,7 +199,7 @@ func (o *ComponentOptions) deleteNamedComponent(ctx context.Context) error {
 		if len(podmanResources) > 0 {
 			spinner := log.Spinnerf("Deleting resources from podman")
 			for _, pod := range podmanResources {
-				err = o.clientset.PodmanClient.CleanupPodResources(pod)
+				err = o.clientset.PodmanClient.CleanupPodResources(pod, true)
 				if err != nil {
 					log.Warningf("Failed to delete the pod %q from podman: %s\n", pod.GetName(), err)
 				}
@@ -359,7 +359,7 @@ func (o *ComponentOptions) deleteDevfileComponent(ctx context.Context) ([]unstru
 				_ = isPodmanInnerLoopDeployed
 			}
 			for _, pod := range podmanPods {
-				err = o.clientset.PodmanClient.CleanupPodResources(pod)
+				err = o.clientset.PodmanClient.CleanupPodResources(pod, true)
 				if err != nil {
 					log.Warningf("Failed to delete the pod %q from podman: %s\n", pod.GetName(), err)
 				}

diff --git a/pkg/odo/cli/delete/component/component_test.go b/pkg/odo/cli/delete/component/component_test.go
@@ -211,7 +211,7 @@ func TestComponentOptions_deleteNamedComponent(t *testing.T) {
 				},
 				podmanClient: func(ctrl *gomock.Controller) podman.Client {
 					client := podman.NewMockClient(ctrl)
-					client.EXPECT().CleanupPodResources(&pod1).Times(1)
+					client.EXPECT().CleanupPodResources(&pod1, true).Times(1)
 					return client
 				},
 				deleteComponentClient: func(ctrl *gomock.Controller) _delete.Client {
@@ -233,7 +233,7 @@ func TestComponentOptions_deleteNamedComponent(t *testing.T) {
 				},
 				podmanClient: func(ctrl *gomock.Controller) podman.Client {
 					client := podman.NewMockClient(ctrl)
-					client.EXPECT().CleanupPodResources(&pod1).Times(1)
+					client.EXPECT().CleanupPodResources(&pod1, true).Times(1)
 					return client
 				},
 				deleteComponentClient: func(ctrl *gomock.Controller) _delete.Client {
@@ -255,7 +255,7 @@ func TestComponentOptions_deleteNamedComponent(t *testing.T) {
 				},
 				podmanClient: func(ctrl *gomock.Controller) podman.Client {
 					client := podman.NewMockClient(ctrl)
-					client.EXPECT().CleanupPodResources(&pod1).Times(0)
+					client.EXPECT().CleanupPodResources(&pod1, true).Times(0)
 					return client
 				},
 				deleteComponentClient: func(ctrl *gomock.Controller) _delete.Client {
@@ -278,7 +278,7 @@ func TestComponentOptions_deleteNamedComponent(t *testing.T) {
 				},
 				podmanClient: func(ctrl *gomock.Controller) podman.Client {
 					client := podman.NewMockClient(ctrl)
-					client.EXPECT().CleanupPodResources(&pod1).Times(1)
+					client.EXPECT().CleanupPodResources(&pod1, true).Times(1)
 					return client
 				},
 				deleteComponentClient: func(ctrl *gomock.Controller) _delete.Client {
@@ -309,7 +309,7 @@ func TestComponentOptions_deleteNamedComponent(t *testing.T) {
 				},
 				podmanClient: func(ctrl *gomock.Controller) podman.Client {
 					client := podman.NewMockClient(ctrl)
-					client.EXPECT().CleanupPodResources(&pod1).Times(1)
+					client.EXPECT().CleanupPodResources(&pod1, true).Times(1)
 					return client
 				},
 				deleteComponentClient: func(ctrl *gomock.Controller) _delete.Client {
@@ -344,7 +344,7 @@ func TestComponentOptions_deleteNamedComponent(t *testing.T) {
 				},
 				podmanClient: func(ctrl *gomock.Controller) podman.Client {
 					client := podman.NewMockClient(ctrl)
-					client.EXPECT().CleanupPodResources(&pod1).Times(0)
+					client.EXPECT().CleanupPodResources(&pod1, true).Times(0)
 					return client
 				},
 				deleteComponentClient: func(ctrl *gomock.Controller) _delete.Client {

diff --git a/pkg/podman/interface.go b/pkg/podman/interface.go
@@ -32,7 +32,7 @@ type Client interface {
 	VolumeRm(volumeName string) error
 
 	// CleanupPodResources stops and removes a pod and its associated resources (volumes)
-	CleanupPodResources(pod *corev1.Pod) error
+	CleanupPodResources(pod *corev1.Pod, cleanVolumes bool) error
 
 	ListAllComponents() ([]api.ComponentAbstract, error)
 

diff --git a/pkg/podman/mock.go b/pkg/podman/mock.go
diff --git a/pkg/podman/podman.go b/pkg/podman/podman.go
@@ -206,7 +206,7 @@ func (o *PodmanCli) VolumeLs() (map[string]bool, error) {
 	return SplitLinesAsSet(string(out)), nil
 }
 
-func (o *PodmanCli) CleanupPodResources(pod *corev1.Pod) error {
+func (o *PodmanCli) CleanupPodResources(pod *corev1.Pod, cleanupVolumes bool) error {
 	err := o.PodStop(pod.GetName())
 	if err != nil {
 		return err
@@ -216,6 +216,10 @@ func (o *PodmanCli) CleanupPodResources(pod *corev1.Pod) error {
 		return err
 	}
 
+	if !cleanupVolumes {
+		return nil
+	}
+
 	for _, volume := range pod.Spec.Volumes {
 		if volume.PersistentVolumeClaim == nil {
 			continue