From 5a063b7510d336723cb6af5c77df3a0746ad4f90 Mon Sep 17 00:00:00 2001 From: Guillaume Lours <705411+glours@users.noreply.github.com> Date: Mon, 30 Jun 2025 16:48:42 +0200 Subject: [PATCH] fix provider concurrent environment map accesses Signed-off-by: Guillaume Lours <705411+glours@users.noreply.github.com> --- .github/workflows/ci.yml | 3 + Makefile | 6 +- docs/examples/provider.go | 24 +++++-- pkg/compose/plugins.go | 14 ++-- .../depends-on-multiple-providers.yaml | 21 ++++++ pkg/e2e/providers_test.go | 64 +++++++++++++++++++ 6 files changed, 120 insertions(+), 12 deletions(-) create mode 100644 pkg/e2e/fixtures/providers/depends-on-multiple-providers.yaml create mode 100644 pkg/e2e/providers_test.go diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index fdf252786..10f89dc72 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -190,6 +190,9 @@ jobs: check-latest: true cache: true + - name: Build example provider + run: make example-provider + - name: Build uses: docker/bake-action@v6 with: diff --git a/Makefile b/Makefile index 946947d0e..34004f111 100644 --- a/Makefile +++ b/Makefile @@ -74,7 +74,7 @@ install: binary install $(or $(DESTDIR),./bin/build)/docker-compose ~/.docker/cli-plugins/docker-compose .PHONY: e2e-compose -e2e-compose: ## Run end to end local tests in plugin mode. Set E2E_TEST=TestName to run a single test +e2e-compose: example-provider ## Run end to end local tests in plugin mode. Set E2E_TEST=TestName to run a single test go run gotest.tools/gotestsum@latest --format testname --junitfile "/tmp/report/report.xml" -- -v $(TEST_FLAGS) -count=1 ./pkg/e2e .PHONY: e2e-compose-standalone @@ -87,6 +87,10 @@ build-and-e2e-compose: build e2e-compose ## Compile the compose cli-plugin and r .PHONY: build-and-e2e-compose-standalone build-and-e2e-compose-standalone: build e2e-compose-standalone ## Compile the compose cli-plugin and run End to end local tests in standalone mode. Set E2E_TEST=TestName to run a single test +.PHONY: example-provider +example-provider: ## build example provider for e2e tests + go build -o bin/build/example-provider docs/examples/provider.go + .PHONY: mocks mocks: mockgen --version >/dev/null 2>&1 || go install go.uber.org/mock/mockgen@v0.4.0 diff --git a/docs/examples/provider.go b/docs/examples/provider.go index e500e8d5c..79fd3256e 100644 --- a/docs/examples/provider.go +++ b/docs/examples/provider.go @@ -39,20 +39,30 @@ func main() { } } +type options struct { + db string + size int +} + func composeCommand() *cobra.Command { c := &cobra.Command{ Use: "compose EVENT", TraverseChildren: true, } c.PersistentFlags().String("project-name", "", "compose project name") // unused + + var options options upCmd := &cobra.Command{ - Use: "up", - Run: up, + Use: "up", + Run: func(_ *cobra.Command, args []string) { + up(options, args) + }, Args: cobra.ExactArgs(1), } - upCmd.Flags().String("type", "", "Database type (mysql, postgres, etc.)") + + upCmd.Flags().StringVar(&options.db, "type", "", "Database type (mysql, postgres, etc.)") _ = upCmd.MarkFlagRequired("type") - upCmd.Flags().Int("size", 10, "Database size in GB") + upCmd.Flags().IntVar(&options.size, "size", 10, "Database size in GB") upCmd.Flags().String("name", "", "Name of the database to be created") _ = upCmd.MarkFlagRequired("name") @@ -71,13 +81,13 @@ func composeCommand() *cobra.Command { const lineSeparator = "\n" -func up(_ *cobra.Command, args []string) { +func up(options options, args []string) { servicename := args[0] fmt.Printf(`{ "type": "debug", "message": "Starting %s" }%s`, servicename, lineSeparator) - for i := 0; i < 100; i += 10 { + for i := 0; i < options.size; i += 10 { time.Sleep(1 * time.Second) - fmt.Printf(`{ "type": "info", "message": "Processing ... %d%%" }%s`, i, lineSeparator) + fmt.Printf(`{ "type": "info", "message": "Processing ... %d%%" }%s`, i*100/options.size, lineSeparator) } fmt.Printf(`{ "type": "setenv", "message": "URL=https://magic.cloud/%s" }%s`, servicename, lineSeparator) } diff --git a/pkg/compose/plugins.go b/pkg/compose/plugins.go index 66cfc53fc..7ddf0b1f2 100644 --- a/pkg/compose/plugins.go +++ b/pkg/compose/plugins.go @@ -27,16 +27,18 @@ import ( "os/exec" "path/filepath" "strings" + "sync" - "github.com/compose-spec/compose-go/v2/types" - "github.com/docker/cli/cli-plugins/manager" - "github.com/docker/cli/cli-plugins/socket" - "github.com/docker/cli/cli/config" "github.com/docker/compose/v2/pkg/progress" "github.com/sirupsen/logrus" "github.com/spf13/cobra" "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/propagation" + + "github.com/compose-spec/compose-go/v2/types" + "github.com/docker/cli/cli-plugins/manager" + "github.com/docker/cli/cli-plugins/socket" + "github.com/docker/cli/cli/config" ) type JsonMessage struct { @@ -52,6 +54,8 @@ const ( providerMetadataDirectory = "compose/providers" ) +var mux sync.Mutex + func (s *composeService) runPlugin(ctx context.Context, project *types.Project, service types.ServiceConfig, command string) error { provider := *service.Provider @@ -70,6 +74,8 @@ func (s *composeService) runPlugin(ctx context.Context, project *types.Project, return err } + mux.Lock() + defer mux.Unlock() for name, s := range project.Services { if _, ok := s.DependsOn[service.Name]; ok { prefix := strings.ToUpper(service.Name) + "_" diff --git a/pkg/e2e/fixtures/providers/depends-on-multiple-providers.yaml b/pkg/e2e/fixtures/providers/depends-on-multiple-providers.yaml new file mode 100644 index 000000000..6faec7cdc --- /dev/null +++ b/pkg/e2e/fixtures/providers/depends-on-multiple-providers.yaml @@ -0,0 +1,21 @@ +services: + test: + image: alpine + command: env + depends_on: + - provider1 + - provider2 + provider1: + provider: + type: example-provider + options: + name: provider1 + type: test1 + size: 1 + provider2: + provider: + type: example-provider + options: + name: provider2 + type: test2 + size: 2 diff --git a/pkg/e2e/providers_test.go b/pkg/e2e/providers_test.go new file mode 100644 index 000000000..b026f1f14 --- /dev/null +++ b/pkg/e2e/providers_test.go @@ -0,0 +1,64 @@ +/* + Copyright 2020 Docker Compose CLI authors + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package e2e + +import ( + "bufio" + "fmt" + "os" + "path/filepath" + "slices" + "strings" + "testing" + + "gotest.tools/v3/assert" + "gotest.tools/v3/icmd" +) + +func TestDependsOnMultipleProviders(t *testing.T) { + provider, err := findExecutable("example-provider") + assert.NilError(t, err) + + path := fmt.Sprintf("%s%s%s", os.Getenv("PATH"), string(os.PathListSeparator), filepath.Dir(provider)) + c := NewParallelCLI(t, WithEnv("PATH="+path)) + const projectName = "depends-on-multiple-providers" + t.Cleanup(func() { + c.cleanupWithDown(t, projectName) + }) + + res := c.RunDockerComposeCmd(t, "-f", "fixtures/providers/depends-on-multiple-providers.yaml", "--project-name", projectName, "up") + res.Assert(t, icmd.Success) + env := getEnv(res.Combined(), false) + assert.Check(t, slices.Contains(env, "PROVIDER1_URL=https://magic.cloud/provider1"), env) + assert.Check(t, slices.Contains(env, "PROVIDER2_URL=https://magic.cloud/provider2"), env) +} + +func getEnv(out string, run bool) []string { + var env []string + scanner := bufio.NewScanner(strings.NewReader(out)) + for scanner.Scan() { + line := scanner.Text() + if !run && strings.HasPrefix(line, "test-1 | ") { + env = append(env, line[10:]) + } + if run && strings.Contains(line, "=") && len(strings.Split(line, "=")) == 2 { + env = append(env, line) + } + } + slices.Sort(env) + return env +}