Wait for deployment rollout only when binding created / modified (red…

…hat-developer#5785)

* Wait for deployment rollout only when binding created / modified

* Add integration test

pkg/kclient/dynamic.go

@@ -16,27 +16,40 @@ import (
 	"k8s.io/klog"
 )
 
-// CreateDynamicResource creates a dynamic custom resource
-func (c *Client) CreateDynamicResource(resource unstructured.Unstructured) error {
+// PatchDynamicResource patches a dynamic custom resource and returns true
+// if the generation of the resource increased or the resource is created
+func (c *Client) PatchDynamicResource(resource unstructured.Unstructured) (bool, error) {
 	klog.V(5).Infoln("Applying resource via server-side apply:")
 	klog.V(5).Infoln(resourceAsJson(resource.Object))
 	data, err := json.Marshal(resource.Object)
 	if err != nil {
-		return fmt.Errorf("unable to marshal resource: %w", err)
+		return false, fmt.Errorf("unable to marshal resource: %w", err)
 	}
 
 	gvr, err := c.GetRestMappingFromUnstructured(resource)
 	if err != nil {
-		return err
+		return false, err
+	}
+
+	var previousGeneration int64 = -1
+	// Get the generation of the current resource
+	previous, err := c.DynamicClient.Resource(gvr.Resource).Namespace(c.Namespace).Get(context.TODO(), resource.GetName(), metav1.GetOptions{})
+	if err != nil {
+		if !kerrors.IsNotFound(err) {
+			return false, err
+		}
+	} else {
+		previousGeneration = previous.GetGeneration()
 	}
 
 	// Patch the dynamic resource
-	_, err = c.DynamicClient.Resource(gvr.Resource).Namespace(c.Namespace).Patch(context.TODO(), resource.GetName(), types.ApplyPatchType, data, metav1.PatchOptions{FieldManager: FieldManager, Force: boolPtr(true)})
+	current, err := c.DynamicClient.Resource(gvr.Resource).Namespace(c.Namespace).Patch(context.TODO(), resource.GetName(), types.ApplyPatchType, data, metav1.PatchOptions{FieldManager: FieldManager, Force: boolPtr(true)})
 	if err != nil {
-		return err
+		return false, err
 	}
+	newGeneration := current.GetGeneration()
 
-	return nil
+	return newGeneration > previousGeneration, nil
 }
 
 // ListDynamicResource returns an unstructured list of instances of a Custom

pkg/kclient/interface.go

@@ -51,7 +51,7 @@ type ClientInterface interface {
 	IsDeploymentExtensionsV1Beta1() (bool, error)
 
 	// dynamic.go
-	CreateDynamicResource(exampleCustomResource unstructured.Unstructured) error
+	PatchDynamicResource(exampleCustomResource unstructured.Unstructured) (bool, error)
 	ListDynamicResources(gvr schema.GroupVersionResource) (*unstructured.UnstructuredList, error)
 	GetDynamicResource(gvr schema.GroupVersionResource, name string) (*unstructured.Unstructured, error)
 	UpdateDynamicResource(gvr schema.GroupVersionResource, name string, u *unstructured.Unstructured) error

pkg/service/link.go

@@ -90,7 +90,8 @@ func pushLinksWithOperator(client kclient.ClientInterface, devfileObj parser.Dev
 		u.SetOwnerReferences([]metav1.OwnerReference{ownerReference})
 		u.SetLabels(labels)
 
-		err = createOperatorService(client, u)
+		var updated bool
+		updated, err = updateOperatorService(client, u)
 		delete(deployed, u.GetKind()+"/"+crdName)
 		if err != nil {
 			if strings.Contains(err.Error(), "already exists") {
@@ -104,7 +105,7 @@ func pushLinksWithOperator(client kclient.ClientInterface, devfileObj parser.Dev
 		// uncomment/modify when service linking is enabled in v3
 		// name := u.GetName()
 		// log.Successf("Created link %q using Service Binding Operator on the cluster; component will be restarted", name)
-		restartNeeded = true
+		restartNeeded = restartNeeded || updated
 	}
 
 	for key, val := range deployed {

pkg/service/service.go

@@ -257,7 +257,7 @@ func PushKubernetesResource(client kclient.ClientInterface, u unstructured.Unstr
 	// Pass in all annotations to the k8s resource
 	u.SetAnnotations(mergeMaps(u.GetAnnotations(), annotations))
 
-	err = createOperatorService(client, u)
+	_, err = updateOperatorService(client, u)
 	return isOp, err
 }
 
@@ -374,27 +374,21 @@ func isLinkResource(kind string) bool {
 	return kind == "ServiceBinding"
 }
 
-// createOperatorService creates the given operator on the cluster
-// it returns the CR,Kind and errors
-func createOperatorService(client kclient.ClientInterface, u unstructured.Unstructured) error {
-
-	// Check resource
-	checkSpinner := log.Spinner("Searching resource in cluster")
-	defer checkSpinner.End(false)
-
-	checkSpinner.End(true)
+// updateOperatorService creates the given operator on the cluster
+// it returns true if the generation of the resource increased or the resource is created
+func updateOperatorService(client kclient.ClientInterface, u unstructured.Unstructured) (bool, error) {
 
 	// Create the service on cluster
 	createSpinner := log.Spinnerf("Creating kind %s", u.GetKind())
 	defer createSpinner.End(false)
 
-	err := client.CreateDynamicResource(u)
+	updated, err := client.PatchDynamicResource(u)
 	if err != nil {
-		return err
+		return false, err
 	}
 
 	createSpinner.End(true)
-	return err
+	return updated, err
 }
 
 // ValidateResourcesExist validates if the Kubernetes inlined components are installed on the cluster

tests/integration/devfile/cmd_dev_test.go

@@ -660,6 +660,74 @@ var _ = Describe("odo dev command tests", func() {
 		})
 	})
 
+	When("Starting a PostgreSQL service", func() {
+		BeforeEach(func() {
+			if helper.IsKubernetesCluster() {
+				Skip("Operators have not been setup on Kubernetes cluster yet. Remove this once the issue has been fixed.")
+			}
+			// Ensure that the operators are installed
+			commonVar.CliRunner.EnsureOperatorIsInstalled("service-binding-operator")
+			commonVar.CliRunner.EnsureOperatorIsInstalled("cloud-native-postgresql")
+			Eventually(func() string {
+				out, _ := commonVar.CliRunner.GetBindableKinds()
+				return out
+			}, 120, 3).Should(ContainSubstring("Cluster"))
+			addBindableKind := commonVar.CliRunner.Run("apply", "-f", helper.GetExamplePath("manifests", "bindablekind-instance.yaml"))
+			Expect(addBindableKind.ExitCode()).To(BeEquivalentTo(0))
+		})
+
+		When("creating local files and dir and running odo dev", func() {
+			var newDirPath, newFilePath, stdOut, podName string
+			var session helper.DevSession
+			// from devfile
+			devfileCmpName := "my-nodejs-app"
+			BeforeEach(func() {
+				newFilePath = filepath.Join(commonVar.Context, "foobar.txt")
+				newDirPath = filepath.Join(commonVar.Context, "testdir")
+				helper.CopyExampleDevFile(filepath.Join("source", "devfiles", "nodejs", "devfile-with-service-binding-files.yaml"), filepath.Join(commonVar.Context, "devfile.yaml"))
+				// Create a new file that we plan on deleting later...
+				if err := helper.CreateFileWithContent(newFilePath, "hello world"); err != nil {
+					fmt.Printf("the foobar.txt file was not created, reason %v", err.Error())
+				}
+				// Create a new directory
+				helper.MakeDir(newDirPath)
+				var err error
+				session, _, _, _, err = helper.StartDevMode()
+				Expect(err).ToNot(HaveOccurred())
+			})
+
+			AfterEach(func() {
+				session.Stop()
+				session.WaitEnd()
+			})
+
+			It("should correctly propagate changes to the container", func() {
+
+				// Check to see if it's been pushed (foobar.txt abd directory testdir)
+				podName = commonVar.CliRunner.GetRunningPodNameByComponent(devfileCmpName, commonVar.Project)
+
+				stdOut = commonVar.CliRunner.ExecListDir(podName, commonVar.Project, "/projects")
+				helper.MatchAllInOutput(stdOut, []string{"foobar.txt", "testdir"})
+			})
+
+			When("deleting local files and dir and waiting for sync", func() {
+				BeforeEach(func() {
+					// Now we delete the file and dir and push
+					helper.DeleteDir(newFilePath)
+					helper.DeleteDir(newDirPath)
+					_, _, err := session.WaitSync()
+					Expect(err).ToNot(HaveOccurred())
+				})
+				It("should not list deleted dir and file in container", func() {
+					podName = commonVar.CliRunner.GetRunningPodNameByComponent(devfileCmpName, commonVar.Project)
+					// Then check to see if it's truly been deleted
+					stdOut = commonVar.CliRunner.ExecListDir(podName, commonVar.Project, "/projects")
+					helper.DontMatchAllInOutput(stdOut, []string{"foobar.txt", "testdir"})
+				})
+			})
+		})
+	})
+
 	When("adding local files to gitignore and running odo dev", func() {
 		var gitignorePath, newDirPath, newFilePath1, newFilePath2, newFilePath3, stdOut, podName string
 		var session helper.DevSession