Merge pull request #2054 from FabianKramm/master

fix: cyclic dependency deployment

Author: FabianKramm

e2e/tests/dependencies/dependencies.go

@@ -455,17 +455,43 @@ wait
 		framework.ExpectEqual(len(list.Items), 0)
 	})
 
-	ginkgo.It("should resolve cyclic dependencies", func() {
+	ginkgo.It("should resolve and deploy cyclic dependencies", func() {
 		tempDir, err := framework.CopyToTempDir("tests/dependencies/testdata/cyclic")
 		framework.ExpectNoError(err)
 		defer framework.CleanupTempDir(initialDir, tempDir)
 
+		ns, err := kubeClient.CreateNamespace("dependencies")
+		framework.ExpectNoError(err)
+		defer func() {
+			err := kubeClient.DeleteNamespace(ns)
+			framework.ExpectNoError(err)
+		}()
+
 		// load it from the regular path first
 		_, dependencies, err := framework.LoadConfig(f, kubeClient.Client(), filepath.Join(tempDir, "devspace.yaml"))
 		framework.ExpectNoError(err)
 
 		// check if dependencies were loaded correctly
 		framework.ExpectEqual(len(dependencies), 1)
 		framework.ExpectEqual(dependencies[0].Name(), "dependency")
+
+		// create a new deploy command
+		deployCmd := &cmd.RunPipelineCmd{
+			GlobalFlags: &flags.GlobalFlags{
+				NoWarn:    true,
+				Namespace: ns,
+			},
+			Pipeline: "deploy",
+		}
+
+		// run the command
+		err = deployCmd.RunDefault(f)
+		framework.ExpectNoError(err)
+
+		// expect single deployment
+		_, err = kubeClient.RawClient().AppsV1().Deployments(ns).Get(context.TODO(), "nginx", metav1.GetOptions{})
+		framework.ExpectNoError(err)
+		_, err = kubeClient.RawClient().AppsV1().Deployments(ns).Get(context.TODO(), "nginx2", metav1.GetOptions{})
+		framework.ExpectNoError(err)
 	})
 })

e2e/tests/dependencies/testdata/cyclic/dependency/devspace.yaml

@@ -1,18 +1,12 @@
-version: v1beta11
+version: v2beta1
+
 dependencies:
- - name: root-devspace
-   source:
-     path: ../
+  dependency2:
+    path: ../dependency2
+
 deployments:
-- name: nginx
-  helm:
-    componentChart: true
-    values:
-      containers:
-      - image: nginx
-dev:
-  ports:
-  - imageSelector: nginx
-    forward:
-      - port: 8080
-        remotePort: 80
+  nginx:
+    helm:
+      values:
+        containers:
+          - image: nginx

e2e/tests/dependencies/testdata/cyclic/dependency2/devspace.yaml

@@ -0,0 +1,12 @@
+version: v2beta1
+
+dependencies:
+  dependency:
+    path: ../dependency
+
+deployments:
+  nginx2:
+    helm:
+      values:
+        containers:
+          - image: nginx

e2e/tests/dependencies/testdata/cyclic/devspace.yaml

@@ -1,9 +1,7 @@
 version: v1beta11
+name: root
+
 dependencies:
 - name: dependency
   source:
     path: dependency
-dev:
-  # Open terminal to the pod with the imageName created in dependency1
-  terminal:
-    imageSelector: image(dependency.default)

e2e/tests/pipelines/pipelines.go

@@ -151,8 +151,7 @@ var _ = DevSpaceDescribe("pipelines", func() {
 		framework.ExpectContainerNameAndImageEqual(ns, "dev", "nginx", "mynginx")
 
 		cancel()
-		err = <-done
-		framework.ExpectNoError(err)
+		<-done
 	})
 
 	ginkgo.It("should use --set and --set-string values from run_default_pipeline command", func() {
@@ -186,8 +185,7 @@ var _ = DevSpaceDescribe("pipelines", func() {
 		framework.ExpectContainerNameAndImageEqual(ns, "dev", "nginx", "mynginx")
 
 		cancel()
-		err = <-done
-		framework.ExpectNoError(err)
+		<-done
 	})
 
 	ginkgo.It("should get value from config", func() {

pkg/devspace/dependency/resolver.go

@@ -39,7 +39,17 @@ type resolver struct {
 // NewResolver creates a new resolver for resolving dependencies
 func NewResolver(ctx devspacecontext.Context, configOptions *loader.ConfigOptions) ResolverInterface {
 	return &resolver{
-		DependencyGraph: graph.NewGraph(graph.NewNode(ctx.Config().Config().Name, &Dependency{name: ctx.Config().Config().Name, root: true})),
+		DependencyGraph: graph.NewGraph(graph.NewNode(ctx.Config().Config().Name, &Dependency{
+			name:         ctx.Config().Config().Name,
+			absolutePath: ctx.Config().Path(),
+			localConfig:  ctx.Config(),
+			dependencyConfig: &latest.DependencyConfig{
+				Name: ctx.Config().Config().Name,
+			},
+			dependencyCache: ctx.Config().LocalCache(),
+			kubeClient:      ctx.KubeClient(),
+			root:            true,
+		})),
 
 		BaseConfig: ctx.Config().Config(),
 		BaseCache:  ctx.Config().LocalCache(),
@@ -96,7 +106,7 @@ func (r *resolver) resolveRecursive(ctx devspacecontext.Context, basePath, paren
 		)
 		if n, ok := r.DependencyGraph.Nodes[dependencyConfig.Name]; ok {
 			child = n.Data.(*Dependency)
-			if child.Config().Path() != dependencyConfigPath {
+			if child != nil && child.Config() != nil && child.Config().Path() != dependencyConfigPath {
 				ctx.Log().Warnf("Seems like you have multiple dependencies with name %s, but they use different source settings (%s != %s). This can lead to unexpected results and you should make sure that the devspace.yaml name is unique across your dependencies or that you use the dependencies.overrideName option", child.name, child.Config().Path(), dependencyConfigPath)
 			}
 

pkg/devspace/pipeline/pipeline.go

@@ -243,13 +243,19 @@ func (p *pipeline) StartNewDependencies(ctx devspacecontext.Context, dependencie
 }
 
 func waitForDependency(ctx context.Context, start types.Pipeline, dependencyName string, log log.Logger) {
+	// parents
+	parents := []string{start.Name()}
+
 	// get top level pipeline
 	for start.Parent() != nil {
 		start = start.Parent()
+		parents = append(parents, start.Name())
 	}
 
-	// now traverse through all dependencies
-	if start.Name() == dependencyName {
+	// if the dependency is cyclic and already executed
+	// as a parent, we skip waiting for it as this would
+	// result in a deadlock.
+	if stringutil.Contains(parents, dependencyName) {
 		return
 	}