Jlopezbarb/fix depends on error

pkg/model/stack.go

@@ -349,6 +349,9 @@ func (s *Stack) Validate() error {
 		}
 		svc.IgnoreSyncVolumes(s)
 	}
+	if err := validateDependsOn(s); err != nil {
+		return err
+	}
 
 	return nil
 }
@@ -366,6 +369,29 @@ func validateStackName(name string) error {
 	return nil
 }
 
+func validateDependsOn(s *Stack) error {
+	for svcName, svc := range s.Services {
+		for dependentSvc, condition := range svc.DependsOn {
+			if svcName == dependentSvc {
+				return fmt.Errorf(" Service '%s' depends can not depend of itself.", svcName)
+			}
+			if _, ok := s.Services[dependentSvc]; !ok {
+				return fmt.Errorf(" Service '%s' depends on service '%s' which is undefined.", svcName, dependentSvc)
+			}
+			if condition.Condition == DependsOnServiceCompleted && !s.Services[dependentSvc].IsJob() {
+				return fmt.Errorf(" Service '%s' is not a job. Please make sure the 'restart_policy' is not set to 'always' in service '%s' ", dependentSvc, dependentSvc)
+			}
+		}
+	}
+
+	dependencyCycle := getDependentCyclic(s)
+	if len(dependencyCycle) > 0 {
+		svcsDependents := fmt.Sprintf("%s and %s", strings.Join(dependencyCycle[:len(dependencyCycle)-1], ", "), dependencyCycle[len(dependencyCycle)-1])
+		return fmt.Errorf(" There was a cyclic dependendecy between %s.", svcsDependents)
+	}
+	return nil
+}
+
 //GetLabelSelector returns the label selector for the stack name
 func (s *Stack) GetLabelSelector() string {
 	return fmt.Sprintf("%s=%s", StackNameLabel, s.Name)

pkg/model/stack_serializer.go

@@ -270,9 +270,6 @@ func (s *Stack) UnmarshalYAML(unmarshal func(interface{}) error) error {
 			return err
 		}
 	}
-	if err := validateDependsOn(s); err != nil {
-		return err
-	}
 
 	s.Warnings.NotSupportedFields = getNotSupportedFields(&stackRaw)
 	s.Warnings.SanitizedServices = sanitizedServicesNames
@@ -1103,29 +1100,6 @@ func sanitizeName(name string) string {
 	return name
 }
 
-func validateDependsOn(s *Stack) error {
-	for svcName, svc := range s.Services {
-		for dependentSvc, condition := range svc.DependsOn {
-			if svcName == dependentSvc {
-				return fmt.Errorf(" Service '%s' depends can not depend of itself.", svcName)
-			}
-			if _, ok := s.Services[dependentSvc]; !ok {
-				return fmt.Errorf(" Service '%s' depends on service '%s' which is undefined.", svcName, dependentSvc)
-			}
-			if condition.Condition == DependsOnServiceCompleted && !s.Services[dependentSvc].IsJob() {
-				return fmt.Errorf(" Service '%s' is not a job. Please change the reset policy so that it is not always in service '%s' ", dependentSvc, dependentSvc)
-			}
-		}
-	}
-
-	dependencyCycle := getDependentCyclic(s)
-	if len(dependencyCycle) > 0 {
-		svcsDependents := fmt.Sprintf("%s and %s", strings.Join(dependencyCycle[:len(dependencyCycle)-1], ", "), dependencyCycle[len(dependencyCycle)-1])
-		return fmt.Errorf(" There was a cyclic dependendecy between %s.", svcsDependents)
-	}
-	return nil
-}
-
 func getNotSupportedFields(s *StackRaw) []string {
 	notSupportedFields := make([]string, 0)
 	notSupportedFields = append(notSupportedFields, getTopLevelNotSupportedFields(s)...)

pkg/model/stack_serializer_test.go

@@ -1669,97 +1669,6 @@ func Test_MultipleEndpoints(t *testing.T) {
 	}
 }
 
-func Test_validateDependsOn(t *testing.T) {
-	tests := []struct {
-		name       string
-		manifest   []byte
-		throwError bool
-		dependsOn  DependsOn
-	}{
-		{
-			name:       "defined dependent service",
-			manifest:   []byte("services:\n  app:\n    image: okteto/vote:1\n    depends_on:\n      - test\n  test:\n    image: okteto/vote:1"),
-			throwError: false,
-			dependsOn: DependsOn{
-				"test": DependsOnConditionSpec{Condition: DependsOnServiceRunning},
-			},
-		},
-		{
-			name:       "defined dependent service needs to be sanitized",
-			manifest:   []byte("services:\n  app:\n    image: okteto/vote:1\n    depends_on:\n      - test_db\n  test_db:\n    image: okteto/vote:1"),
-			throwError: false,
-			dependsOn: DependsOn{
-				"test-db": DependsOnConditionSpec{Condition: DependsOnServiceRunning},
-			},
-		},
-		{
-			name:       "defined dependent service started",
-			manifest:   []byte("services:\n  app:\n    image: okteto/vote:1\n    depends_on:\n      test:\n        condition: service_started\n  test:\n    image: okteto/vote:1"),
-			throwError: false,
-			dependsOn: DependsOn{
-				"test": DependsOnConditionSpec{Condition: DependsOnServiceRunning},
-			},
-		},
-		{
-			name:       "defined dependent service healthy",
-			manifest:   []byte("services:\n  app:\n    image: okteto/vote:1\n    depends_on:\n      test:\n        condition: service_healthy\n  test:\n    image: okteto/vote:1"),
-			throwError: false,
-			dependsOn: DependsOn{
-				"test": DependsOnConditionSpec{Condition: DependsOnServiceHealthy},
-			},
-		},
-		{
-			name:       "defined dependent service completed",
-			manifest:   []byte("services:\n  app:\n    image: okteto/vote:1\n    depends_on:\n      test:\n        condition: service_completed_successfully\n  test:\n    image: okteto/vote:1\n    restart: never"),
-			throwError: false,
-			dependsOn: DependsOn{
-				"test": DependsOnConditionSpec{Condition: DependsOnServiceCompleted},
-			},
-		},
-		{
-			name:       "not defined dependent service",
-			manifest:   []byte("services:\n  app:\n    image: okteto/vote:1\n    depends_on:\n      - ads"),
-			throwError: true,
-		},
-		{
-			name:       "self dependent service",
-			manifest:   []byte("services:\n  app:\n    image: okteto/vote:1\n    depends_on:\n      - app"),
-			throwError: true,
-		},
-		{
-			name:       "circular dependency",
-			manifest:   []byte("services:\n  app:\n    image: okteto/vote:1\n    depends_on:\n      - test\n  test:\n    image: okteto/vote:1\n    depends_on:\n      - app"),
-			throwError: true,
-		},
-		{
-			name:       "circular dependency difficult",
-			manifest:   []byte("services:\n  app:\n    image: okteto/vote:1\n    depends_on:\n      - test\n  test:\n    image: okteto/vote:1\n    depends_on:\n      - test2\n  test1:\n    image: okteto/vote:1\n    depends_on:\n      - test2\n  test2:\n    image: okteto/vote:1\n    depends_on:\n      - app"),
-			throwError: true,
-		},
-		{
-			name:       "circular dependency not first",
-			manifest:   []byte("services:\n  test:\n    image: okteto/vote:1\n    depends_on:\n      - test2\n  app:\n    image: okteto/vote:1\n    depends_on:\n      - test\n  test1:\n    image: okteto/vote:1\n    depends_on:\n      - test2\n  test2:\n    image: okteto/vote:1\n    depends_on:\n      - app"),
-			throwError: true,
-		},
-	}
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			s, err := ReadStack(tt.manifest, false)
-			if err == nil && tt.throwError {
-				t.Fatal("Expected error but not thrown")
-			}
-			if err != nil && !tt.throwError {
-				t.Fatal(err)
-			}
-			if err == nil && !tt.throwError {
-				if !reflect.DeepEqual(s.Services["app"].DependsOn, tt.dependsOn) {
-					t.Fatalf("Expected %v but got %v", tt.dependsOn, s.Services["app"].DependsOn)
-				}
-			}
-		})
-	}
-}
-
 func Test_CreateJobPVCs(t *testing.T) {
 	manifest := []byte(`name: test
 services:

pkg/model/stack_test.go

@@ -964,3 +964,99 @@ func TestStack_ExpandEnvsAtFileLevel(t *testing.T) {
 		})
 	}
 }
+
+func Test_validateDependsOn(t *testing.T) {
+	tests := []struct {
+		name       string
+		manifest   []byte
+		throwError bool
+		dependsOn  DependsOn
+	}{
+		{
+			name:       "defined dependent service",
+			manifest:   []byte("services:\n  app:\n    image: okteto/vote:1\n    depends_on:\n      - test\n  test:\n    image: okteto/vote:1"),
+			throwError: false,
+			dependsOn: DependsOn{
+				"test": DependsOnConditionSpec{Condition: DependsOnServiceRunning},
+			},
+		},
+		{
+			name:       "defined dependent service needs to be sanitized",
+			manifest:   []byte("services:\n  app:\n    image: okteto/vote:1\n    depends_on:\n      - test_db\n  test_db:\n    image: okteto/vote:1"),
+			throwError: false,
+			dependsOn: DependsOn{
+				"test-db": DependsOnConditionSpec{Condition: DependsOnServiceRunning},
+			},
+		},
+		{
+			name:       "defined dependent service started",
+			manifest:   []byte("services:\n  app:\n    image: okteto/vote:1\n    depends_on:\n      test:\n        condition: service_started\n  test:\n    image: okteto/vote:1"),
+			throwError: false,
+			dependsOn: DependsOn{
+				"test": DependsOnConditionSpec{Condition: DependsOnServiceRunning},
+			},
+		},
+		{
+			name:       "defined dependent service healthy",
+			manifest:   []byte("services:\n  app:\n    image: okteto/vote:1\n    depends_on:\n      test:\n        condition: service_healthy\n  test:\n    image: okteto/vote:1"),
+			throwError: false,
+			dependsOn: DependsOn{
+				"test": DependsOnConditionSpec{Condition: DependsOnServiceHealthy},
+			},
+		},
+		{
+			name:       "defined dependent service completed",
+			manifest:   []byte("services:\n  app:\n    image: okteto/vote:1\n    depends_on:\n      test:\n        condition: service_completed_successfully\n  test:\n    image: okteto/vote:1\n    restart: never"),
+			throwError: false,
+			dependsOn: DependsOn{
+				"test": DependsOnConditionSpec{Condition: DependsOnServiceCompleted},
+			},
+		},
+		{
+			name:       "not defined dependent service",
+			manifest:   []byte("services:\n  app:\n    image: okteto/vote:1\n    depends_on:\n      - ads"),
+			throwError: true,
+		},
+		{
+			name:       "self dependent service",
+			manifest:   []byte("services:\n  app:\n    image: okteto/vote:1\n    depends_on:\n      - app"),
+			throwError: true,
+		},
+		{
+			name:       "circular dependency",
+			manifest:   []byte("services:\n  app:\n    image: okteto/vote:1\n    depends_on:\n      - test\n  test:\n    image: okteto/vote:1\n    depends_on:\n      - app"),
+			throwError: true,
+		},
+		{
+			name:       "circular dependency difficult",
+			manifest:   []byte("services:\n  app:\n    image: okteto/vote:1\n    depends_on:\n      - test\n  test:\n    image: okteto/vote:1\n    depends_on:\n      - test2\n  test1:\n    image: okteto/vote:1\n    depends_on:\n      - test2\n  test2:\n    image: okteto/vote:1\n    depends_on:\n      - app"),
+			throwError: true,
+		},
+		{
+			name:       "circular dependency not first",
+			manifest:   []byte("services:\n  test:\n    image: okteto/vote:1\n    depends_on:\n      - test2\n  app:\n    image: okteto/vote:1\n    depends_on:\n      - test\n  test1:\n    image: okteto/vote:1\n    depends_on:\n      - test2\n  test2:\n    image: okteto/vote:1\n    depends_on:\n      - app"),
+			throwError: true,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			s, err := ReadStack(tt.manifest, false)
+			if err != nil {
+				t.Fatal("Wrong stack readiness")
+			}
+			s.Name = "test"
+			err = s.Validate()
+			if err == nil && tt.throwError {
+				t.Fatal("Expected error but not thrown")
+			}
+			if err != nil && !tt.throwError {
+				t.Fatal(err)
+			}
+			if err == nil && !tt.throwError {
+				if !reflect.DeepEqual(s.Services["app"].DependsOn, tt.dependsOn) {
+					t.Fatalf("Expected %v but got %v", tt.dependsOn, s.Services["app"].DependsOn)
+				}
+			}
+		})
+	}
+}