Skip to content

Commit

Permalink
Jlopezbarb/fix depends on error (#1928)
Browse files Browse the repository at this point in the history 
* Fix depends on multiple files error

Signed-off-by: Javier López Barba <javier@okteto.com>

* Add username

Signed-off-by: Javier López Barba <javier@okteto.com>

* Revert username

Signed-off-by: Javier López Barba <javier@okteto.com>

* Fix error message

Signed-off-by: Javier López Barba <javier@okteto.com>
  • Loading branch information
@jLopezbarb
jLopezbarb committed Nov 16, 2021
1 parent 240724d commit c118937
Show file tree
Hide file tree
Showing 4 changed files with 122 additions and 117 deletions.
26 changes: 26 additions & 0 deletions pkg/model/stack.go
View file
Expand Up @@ -349,6 +349,9 @@ func (s *Stack) Validate() error {
}
svc.IgnoreSyncVolumes(s)
}
if err := validateDependsOn(s); err != nil {
return err
}

return nil
}
Expand All @@ -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)
Expand Down
26 changes: 0 additions & 26 deletions pkg/model/stack_serializer.go
View file
Expand Up @@ -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
Expand Down Expand Up @@ -1122,29 +1119,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)...)
Expand Down
91 changes: 0 additions & 91 deletions pkg/model/stack_serializer_test.go
View file
Expand Up @@ -1755,97 +1755,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:
Expand Down
96 changes: 96 additions & 0 deletions pkg/model/stack_test.go
View file
Expand Up @@ -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)
}
}
})
}
}

0 comments on commit c118937

Please sign in to comment.