From bb94ea034e9908ca02c8c6c5c173b41f47dad0e3 Mon Sep 17 00:00:00 2001 From: Guillaume Lours <705411+glours@users.noreply.github.com> Date: Mon, 10 Jul 2023 18:27:59 +0200 Subject: [PATCH 1/2] add support of depends_on.required attribute Signed-off-by: Guillaume Lours <705411+glours@users.noreply.github.com> --- go.mod | 5 +++-- go.sum | 10 ++++++---- pkg/compose/compose.go | 3 ++- pkg/compose/convergence.go | 3 +++ pkg/compose/convergence_test.go | 4 ++-- pkg/compose/create.go | 1 + pkg/compose/create_test.go | 2 +- pkg/compose/dependencies.go | 7 ++++++- pkg/compose/restart.go | 2 +- pkg/compose/start.go | 1 + pkg/e2e/compose_run_test.go | 8 ++++++++ .../fixtures/dependencies/deps-not-required.yaml | 11 +++++++++++ pkg/e2e/up_test.go | 13 +++++++++++++ 13 files changed, 58 insertions(+), 12 deletions(-) create mode 100644 pkg/e2e/fixtures/dependencies/deps-not-required.yaml diff --git a/go.mod b/go.mod index e458aa42..bb4eb864 100644 --- 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 index 9c6aeb63..9a2662d1 100644 --- 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 index 3d6835c0..aa0ca9f2 100644 --- 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 index 5243e89f..78a5fdb6 100644 --- a/pkg/compose/convergence.go +++ b/pkg/compose/convergence.go @@ -362,6 +362,9 @@ func shouldWaitForDependency(serviceName string, dependencyConfig types.ServiceD return false, nil } } + if !dependencyConfig.Required { + return false, nil + } return false, err } else if service.Scale == 0 { // don't wait for the dependency which configured to have 0 containers running diff --git a/pkg/compose/convergence_test.go b/pkg/compose/convergence_test.go index ab9454dc..1fddbf5e 100644 --- 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 index 57e1ca65..ffc9f178 100644 --- 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 index 9939d51f..b3a6b4fe 100644 --- 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 index 331a8001..5205b30e 100644 --- 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 index e091acb6..72cf0a81 100644 --- 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 index 66520cd8..9dacb7a8 100644 --- 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 index 40ce272f..e330bd43 100644 --- 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 new file mode 100644 index 00000000..44286846 --- /dev/null +++ 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] \ No newline at end of file diff --git a/pkg/e2e/up_test.go b/pkg/e2e/up_test.go index f2217297..d1e4fe21 100644 --- 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, + "up", "-d") + assert.Assert(t, strings.Contains(res.Combined(), "foo"), res.Combined()) + assert.Assert(t, !strings.Contains(res.Combined(), "bar"), res.Combined()) +} From 2d16a05afa99d460defa8edc4f12e2fd80cd7c84 Mon Sep 17 00:00:00 2001 From: Guillaume Lours <705411+glours@users.noreply.github.com> Date: Tue, 18 Jul 2023 18:24:55 +0200 Subject: [PATCH 2/2] only check if a dependency is required when something unexpected happens Signed-off-by: Guillaume Lours <705411+glours@users.noreply.github.com> --- pkg/compose/convergence.go | 28 +++++++++++++++++++++++++--- pkg/e2e/up_test.go | 4 ++-- 2 files changed, 27 insertions(+), 5 deletions(-) diff --git a/pkg/compose/convergence.go b/pkg/compose/convergence.go index 78a5fdb6..046119c5 100644 --- 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) @@ -362,9 +387,6 @@ func shouldWaitForDependency(serviceName string, dependencyConfig types.ServiceD return false, nil } } - if !dependencyConfig.Required { - return false, nil - } return false, err } else if service.Scale == 0 { // don't wait for the dependency which configured to have 0 containers running diff --git a/pkg/e2e/up_test.go b/pkg/e2e/up_test.go index d1e4fe21..b70cd169 100644 --- a/pkg/e2e/up_test.go +++ b/pkg/e2e/up_test.go @@ -162,7 +162,7 @@ func TestUpWithDependencyNotRequired(t *testing.T) { }) res := c.RunDockerComposeCmd(t, "-f", "./fixtures/dependencies/deps-not-required.yaml", "--project-name", projectName, - "up", "-d") + "--profile", "not-required", "up", "-d") assert.Assert(t, strings.Contains(res.Combined(), "foo"), res.Combined()) - assert.Assert(t, !strings.Contains(res.Combined(), "bar"), res.Combined()) + assert.Assert(t, strings.Contains(res.Combined(), " optional dependency \"bar\" failed to start"), res.Combined()) }