Merge pull request docker#10792 from glours/add-depends_on-required

glours · web-flow · commit 5a072b1ad52a · 2023-07-19T09:53:49.000+02:00
add support of depends_on.required attribute
diff --git a/go.mod b/go.mod
@@ -6,7 +6,7 @@ require (
 	github.com/AlecAivazis/survey/v2 v2.3.7
 	github.com/Microsoft/go-winio v0.6.1
 	github.com/buger/goterm v1.0.4
-	github.com/compose-spec/compose-go v1.16.0
+	github.com/compose-spec/compose-go v1.17.0
 	github.com/containerd/console v1.0.3
 	github.com/containerd/containerd v1.7.2
 	github.com/cucumber/godog v0.0.0-00010101000000-000000000000 // replaced; see replace for the actual version used
@@ -162,7 +162,8 @@ require (
 	go.opentelemetry.io/otel/metric v0.37.0 // indirect
 	go.opentelemetry.io/proto/otlp v0.19.0 // indirect
 	golang.org/x/crypto v0.7.0 // indirect
-	golang.org/x/mod v0.9.0 // indirect
+	golang.org/x/exp v0.0.0-20230713183714-613f0c0eb8a1 // indirect
+	golang.org/x/mod v0.11.0 // indirect
 	golang.org/x/net v0.9.0 // indirect
 	golang.org/x/oauth2 v0.7.0 // indirect
 	golang.org/x/sys v0.7.0 // indirect
diff --git a/go.sum b/go.sum
@@ -131,8 +131,8 @@ github.com/cncf/xds/go v0.0.0-20210922020428-25de7278fc84/go.mod h1:eXthEFrGJvWH
 github.com/cncf/xds/go v0.0.0-20211011173535-cb28da3451f1/go.mod h1:eXthEFrGJvWHgFFCl3hGmgk+/aYT6PnTQLykKQRLhEs=
 github.com/cncf/xds/go v0.0.0-20230607035331-e9ce68804cb4 h1:/inchEIKaYC1Akx+H+gqO04wryn5h75LSazbRlnya1k=
 github.com/codahale/rfc6979 v0.0.0-20141003034818-6a90f24967eb h1:EDmT6Q9Zs+SbUoc7Ik9EfrFqcylYqgPZ9ANSbTAntnE=
-github.com/compose-spec/compose-go v1.16.0 h1:HYk4uYWXgArHh6NG+WE4yGYayOXw+hjqJ+eJxpjWWjk=
-github.com/compose-spec/compose-go v1.16.0/go.mod h1:3yngGBGfls6FHGQsg4B1z6gz8ej9SOvmAJtxCwgbcnc=
+github.com/compose-spec/compose-go v1.17.0 h1:cvje90CU94dQyTnJoHJYjx9yE4Iggse1XmGcO3Qi5ts=
+github.com/compose-spec/compose-go v1.17.0/go.mod h1:zR2tP1+kZHi5vJz7PjpW6oMoDji/Js3GHjP+hfjf70Q=
 github.com/containerd/cgroups v1.1.0 h1:v8rEWFl6EoqHB+swVNjVoCJE8o3jX7e8nqBGPLaDFBM=
 github.com/containerd/console v1.0.3 h1:lIr7SlA5PxZyMV30bDW0MGbiOPXwc63yRuCP0ARubLw=
 github.com/containerd/console v1.0.3/go.mod h1:7LqA/THxQ86k76b8c/EMSiaJ3h1eZkMkXar0TQ1gf3U=
@@ -713,6 +713,8 @@ golang.org/x/exp v0.0.0-20191227195350-da58074b4299/go.mod h1:2RIsYlXP63K8oxa1u0
 golang.org/x/exp v0.0.0-20200119233911-0405dc783f0a/go.mod h1:2RIsYlXP63K8oxa1u096TMicItID8zy7Y6sNkU49FU4=
 golang.org/x/exp v0.0.0-20200207192155-f17229e696bd/go.mod h1:J/WKrq2StrnmMY6+EHIKF9dgMWnmCNThgcyBT1FY9mM=
 golang.org/x/exp v0.0.0-20200224162631-6cc2880d07d6/go.mod h1:3jZMyOhIsHpP37uCMkUooju7aAi5cS1Q23tOzKc+0MU=
+golang.org/x/exp v0.0.0-20230713183714-613f0c0eb8a1 h1:MGwJjxBy0HJshjDNfLsYO8xppfqWlA5ZT9OhtUUhTNw=
+golang.org/x/exp v0.0.0-20230713183714-613f0c0eb8a1/go.mod h1:FXUEEKJgO7OQYeo8N01OfiKP8RXMtf6e8aTskBGqWdc=
 golang.org/x/image v0.0.0-20190227222117-0694c2d4d067/go.mod h1:kZ7UVZpmo3dzQBMxlp+ypCbDeSB+sBbTgSJuh5dn5js=
 golang.org/x/image v0.0.0-20190802002840-cff245a6509b/go.mod h1:FeLwcggjj3mMvU+oOTbSwawSJRM1uh48EjtB4UJZlP0=
 golang.org/x/lint v0.0.0-20181026193005-c67002cb31c3/go.mod h1:UVdnD1Gm6xHRNCYTkRU2/jEulfH38KcIWyp/GAMgvoE=
@@ -738,8 +740,8 @@ golang.org/x/mod v0.4.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=
 golang.org/x/mod v0.4.1/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=
 golang.org/x/mod v0.4.2/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=
 golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4/go.mod h1:jJ57K6gSWd91VN4djpZkiMVwK6gcyfeH4XE8wZrZaV4=
-golang.org/x/mod v0.9.0 h1:KENHtAZL2y3NLMYZeHY9DW8HW8V+kQyJsY/V9JlKvCs=
-golang.org/x/mod v0.9.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs=
+golang.org/x/mod v0.11.0 h1:bUO06HqtnRcc/7l71XBe4WcqTZ+3AH1J59zWDDwLKgU=
+golang.org/x/mod v0.11.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs=
 golang.org/x/net v0.0.0-20180724234803-3673e40ba225/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
 golang.org/x/net v0.0.0-20180826012351-8a410e7b638d/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
 golang.org/x/net v0.0.0-20180906233101-161cd47e91fd/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
diff --git a/pkg/compose/compose.go b/pkg/compose/compose.go
@@ -203,6 +203,7 @@ func (s *composeService) projectFromName(containers Containers, projectName stri
 				condition := ServiceConditionRunningOrHealthy
 				// Let's restart the dependency by default if we don't have the info stored in the label
 				restart := true
+				required := true
 				dependency := dcArr[0]
 
 				// backward compatibility
@@ -212,7 +213,7 @@ func (s *composeService) projectFromName(containers Containers, projectName stri
 						restart, _ = strconv.ParseBool(dcArr[2])
 					}
 				}
-				service.DependsOn[dependency] = types.ServiceDependency{Condition: condition, Restart: restart}
+				service.DependsOn[dependency] = types.ServiceDependency{Condition: condition, Restart: restart, Required: required}
 			}
 		}
 		project.Services = append(project.Services, *service)
diff --git a/pkg/compose/convergence.go b/pkg/compose/convergence.go
@@ -286,9 +286,18 @@ func containerEvents(containers Containers, eventFunc func(string) progress.Even
 	return events
 }
 
+func containerSkippedEvents(containers Containers, eventFunc func(string, string) progress.Event, reason string) []progress.Event {
+	events := []progress.Event{}
+	for _, container := range containers {
+		events = append(events, eventFunc(getContainerProgressName(container), reason))
+	}
+	return events
+}
+
 // ServiceConditionRunningOrHealthy is a service condition on status running or healthy
 const ServiceConditionRunningOrHealthy = "running_or_healthy"
 
+//nolint:gocyclo
 func (s *composeService) waitDependencies(ctx context.Context, project *types.Project, dependencies types.DependsOnConfig, containers Containers) error {
 	eg, _ := errgroup.WithContext(ctx)
 	w := progress.ContextWriter(ctx)
@@ -312,6 +321,11 @@ func (s *composeService) waitDependencies(ctx context.Context, project *types.Pr
 				case ServiceConditionRunningOrHealthy:
 					healthy, err := s.isServiceHealthy(ctx, waitingFor, true)
 					if err != nil {
+						if !config.Required {
+							w.Events(containerSkippedEvents(waitingFor, progress.SkippedEvent, fmt.Sprintf("optional dependency %q is not running or is unhealthy", dep)))
+							logrus.Warnf("optional dependency %q is not running or is unhealthy: %s", dep, err.Error())
+							return nil
+						}
 						return err
 					}
 					if healthy {
@@ -321,6 +335,11 @@ func (s *composeService) waitDependencies(ctx context.Context, project *types.Pr
 				case types.ServiceConditionHealthy:
 					healthy, err := s.isServiceHealthy(ctx, waitingFor, false)
 					if err != nil {
+						if !config.Required {
+							w.Events(containerSkippedEvents(waitingFor, progress.SkippedEvent, fmt.Sprintf("optional dependency %q failed to start", dep)))
+							logrus.Warnf("optional dependency %q failed to start: %s", dep, err.Error())
+							return nil
+						}
 						w.Events(containerEvents(waitingFor, progress.ErrorEvent))
 						return errors.Wrap(err, "dependency failed to start")
 					}
@@ -334,6 +353,12 @@ func (s *composeService) waitDependencies(ctx context.Context, project *types.Pr
 						return err
 					}
 					if exited {
+						logMessageSuffix := fmt.Sprintf("%q didn't complete successfully: exit %d", dep, code)
+						if !config.Required {
+							w.Events(containerSkippedEvents(waitingFor, progress.SkippedEvent, fmt.Sprintf("optional dependency %s", logMessageSuffix)))
+							logrus.Warnf("optional dependency %s", logMessageSuffix)
+							return nil
+						}
 						w.Events(containerEvents(waitingFor, progress.Exited))
 						if code != 0 {
 							return fmt.Errorf("service %q didn't complete successfully: exit %d", dep, code)
diff --git a/pkg/compose/convergence_test.go b/pkg/compose/convergence_test.go
@@ -236,8 +236,8 @@ func TestWaitDependencies(t *testing.T) {
 		redisService := types.ServiceConfig{Name: "redis", Scale: 1}
 		project := types.Project{Name: strings.ToLower(testProject), Services: []types.ServiceConfig{dbService, redisService}}
 		dependencies := types.DependsOnConfig{
-			"db":    {Condition: types.ServiceConditionStarted},
-			"redis": {Condition: types.ServiceConditionStarted},
+			"db":    {Condition: types.ServiceConditionStarted, Required: true},
+			"redis": {Condition: types.ServiceConditionStarted, Required: true},
 		}
 		assert.NilError(t, tested.waitDependencies(context.Background(), &project, dependencies, nil))
 	})
diff --git a/pkg/compose/create.go b/pkg/compose/create.go
@@ -139,6 +139,7 @@ func prepareVolumes(p *types.Project) error {
 					p.Services[i].DependsOn[service.Name].Condition == "" {
 					p.Services[i].DependsOn[service.Name] = types.ServiceDependency{
 						Condition: types.ServiceConditionStarted,
+						Required:  true,
 					}
 				}
 			}
diff --git a/pkg/compose/create_test.go b/pkg/compose/create_test.go
@@ -124,7 +124,7 @@ func TestPrepareVolumes(t *testing.T) {
 					Name:        "aService",
 					VolumesFrom: []string{"anotherService"},
 					DependsOn: map[string]composetypes.ServiceDependency{
-						"anotherService": {Condition: composetypes.ServiceConditionHealthy},
+						"anotherService": {Condition: composetypes.ServiceConditionHealthy, Required: true},
 					},
 				},
 				{
diff --git a/pkg/compose/dependencies.go b/pkg/compose/dependencies.go
@@ -268,10 +268,15 @@ func NewGraph(project *types.Project, initialStatus ServiceStatus) (*Graph, erro
 		graph.AddVertex(s.Name, s.Name, initialStatus)
 	}
 
-	for _, s := range project.Services {
+	for index, s := range project.Services {
 		for _, name := range s.GetDependencies() {
 			err := graph.AddEdge(s.Name, name)
 			if err != nil {
+				if !s.DependsOn[name].Required {
+					delete(s.DependsOn, name)
+					project.Services[index] = s
+					continue
+				}
 				if api.IsNotFoundError(err) {
 					ds, err := project.GetDisabledService(name)
 					if err == nil {
diff --git a/pkg/compose/restart.go b/pkg/compose/restart.go
@@ -48,7 +48,7 @@ func (s *composeService) restart(ctx context.Context, projectName string, option
 		}
 	}
 
-	// ignore depends_on relations which are not impacted by restarting service
+	// ignore depends_on relations which are not impacted by restarting service or not required
 	for i, service := range project.Services {
 		for name, r := range service.DependsOn {
 			if !r.Restart {
diff --git a/pkg/compose/start.go b/pkg/compose/start.go
@@ -108,6 +108,7 @@ func (s *composeService) start(ctx context.Context, projectName string, options
 		for _, s := range project.Services {
 			depends[s.Name] = types.ServiceDependency{
 				Condition: getDependencyCondition(s, project),
+				Required:  true,
 			}
 		}
 		if options.WaitTimeout > 0 {
diff --git a/pkg/e2e/compose_run_test.go b/pkg/e2e/compose_run_test.go
@@ -144,4 +144,12 @@ func TestLocalComposeRun(t *testing.T) {
 
 		c.RunDockerComposeCmd(t, "-f", "./fixtures/run-test/deps.yaml", "down", "--remove-orphans")
 	})
+
+	t.Run("run with not required dependency", func(t *testing.T) {
+		res := c.RunDockerComposeCmd(t, "-f", "./fixtures/dependencies/deps-not-required.yaml", "run", "foo")
+		assert.Assert(t, strings.Contains(res.Combined(), "foo"), res.Combined())
+		assert.Assert(t, !strings.Contains(res.Combined(), "bar"), res.Combined())
+
+		c.RunDockerComposeCmd(t, "-f", "./fixtures/dependencies/deps-not-required.yaml", "down", "--remove-orphans")
+	})
 }
diff --git a/pkg/e2e/fixtures/dependencies/deps-not-required.yaml b/pkg/e2e/fixtures/dependencies/deps-not-required.yaml
@@ -0,0 +1,11 @@
+services:
+  foo:
+    image: bash
+    command: echo "foo"
+    depends_on:
+      bar:
+        required: false
+        condition: service_healthy
+  bar:
+    image: nginx:alpine
+    profiles: [not-required]
diff --git a/pkg/e2e/up_test.go b/pkg/e2e/up_test.go
@@ -153,3 +153,16 @@ func TestScaleDoesntRecreate(t *testing.T) {
 	assert.Check(t, !strings.Contains(res.Combined(), "Recreated"))
 
 }
+
+func TestUpWithDependencyNotRequired(t *testing.T) {
+	c := NewCLI(t)
+	const projectName = "compose-e2e-dependency-not-required"
+	t.Cleanup(func() {
+		c.RunDockerComposeCmd(t, "--project-name", projectName, "down")
+	})
+
+	res := c.RunDockerComposeCmd(t, "-f", "./fixtures/dependencies/deps-not-required.yaml", "--project-name", projectName,
+		"--profile", "not-required", "up", "-d")
+	assert.Assert(t, strings.Contains(res.Combined(), "foo"), res.Combined())
+	assert.Assert(t, strings.Contains(res.Combined(), " optional dependency \"bar\" failed to start"), res.Combined())
+}

Original file line number	Diff line number	Diff line change
`@@ -203,6 +203,7 @@ func (s *composeService) projectFromName(containers Containers, projectName stri`
`203`	`203`	`condition := ServiceConditionRunningOrHealthy`
`204`	`204`	`// Let's restart the dependency by default if we don't have the info stored in the label`
`205`	`205`	`restart := true`
	`206`	`+ required := true`
`206`	`207`	`dependency := dcArr[0]`
`207`	`208`
`208`	`209`	`// backward compatibility`
`@@ -212,7 +213,7 @@ func (s *composeService) projectFromName(containers Containers, projectName stri`
`212`	`213`	`restart, _ = strconv.ParseBool(dcArr[2])`
`213`	`214`	`}`
`214`	`215`	`}`
`215`		`- service.DependsOn[dependency] = types.ServiceDependency{Condition: condition, Restart: restart}`
	`216`	`+ service.DependsOn[dependency] = types.ServiceDependency{Condition: condition, Restart: restart, Required: required}`
`216`	`217`	`}`
`217`	`218`	`}`
`218`	`219`	`project.Services = append(project.Services, *service)`
Original file line number	Diff line number	Diff line change
`@@ -139,6 +139,7 @@ func prepareVolumes(p *types.Project) error {`
`139`	`139`	`p.Services[i].DependsOn[service.Name].Condition == "" {`
`140`	`140`	`p.Services[i].DependsOn[service.Name] = types.ServiceDependency{`
`141`	`141`	`Condition: types.ServiceConditionStarted,`
	`142`	`+ Required: true,`
`142`	`143`	`}`
`143`	`144`	`}`
`144`	`145`	`}`
Original file line number	Diff line number	Diff line change
`@@ -124,7 +124,7 @@ func TestPrepareVolumes(t *testing.T) {`
`124`	`124`	`Name: "aService",`
`125`	`125`	`VolumesFrom: []string{"anotherService"},`
`126`	`126`	`DependsOn: map[string]composetypes.ServiceDependency{`
`127`		`- "anotherService": {Condition: composetypes.ServiceConditionHealthy},`
	`127`	`+ "anotherService": {Condition: composetypes.ServiceConditionHealthy, Required: true},`
`128`	`128`	`},`
`129`	`129`	`},`
`130`	`130`	`{`
Original file line number	Diff line number	Diff line change
`@@ -48,7 +48,7 @@ func (s *composeService) restart(ctx context.Context, projectName string, option`
`48`	`48`	`}`
`49`	`49`	`}`
`50`	`50`
`51`		`- // ignore depends_on relations which are not impacted by restarting service`
	`51`	`+ // ignore depends_on relations which are not impacted by restarting service or not required`
`52`	`52`	`for i, service := range project.Services {`
`53`	`53`	`for name, r := range service.DependsOn {`
`54`	`54`	`if !r.Restart {`
Original file line number	Diff line number	Diff line change
`@@ -108,6 +108,7 @@ func (s *composeService) start(ctx context.Context, projectName string, options`
`108`	`108`	`for _, s := range project.Services {`
`109`	`109`	`depends[s.Name] = types.ServiceDependency{`
`110`	`110`	`Condition: getDependencyCondition(s, project),`
	`111`	`+ Required: true,`
`111`	`112`	`}`
`112`	`113`	`}`
`113`	`114`	`if options.WaitTimeout > 0 {`