From 1b5fa3b93ff4488c2d14a1195b7ca1efd1c2bf79 Mon Sep 17 00:00:00 2001 From: Milas Bowman Date: Wed, 20 Mar 2024 07:44:27 -0600 Subject: [PATCH] feat(experiments): add experimental feature state (#11633) Use environment variable for global opt-out and Docker Desktop (if available) to determine specific experiment states. In the future, we'll allow per-feature opt-in/opt-out via env vars as well, but currently there is a single `COMPOSE_EXPERIMENTAL` env var that can be used to opt-out of all experimental features independently of Docker Desktop configuration. --- cmd/compose/compose.go | 30 ++++++++-- cmd/compose/up.go | 6 +- cmd/main.go | 5 +- internal/desktop/client.go | 39 +++++++++++- internal/desktop/discovery.go | 62 +++++++++++++++++++ internal/desktop/integration.go | 25 -------- internal/experimental/experimental.go | 85 +++++++++++++++++++++++++++ internal/locker/pidfile_windows.go | 3 +- pkg/compose/compose.go | 6 +- pkg/compose/desktop.go | 61 +++---------------- 10 files changed, 231 insertions(+), 91 deletions(-) create mode 100644 internal/desktop/discovery.go delete mode 100644 internal/desktop/integration.go create mode 100644 internal/experimental/experimental.go diff --git a/cmd/compose/compose.go b/cmd/compose/compose.go index 8af123e51..0ecb19b60 100644 --- a/cmd/compose/compose.go +++ b/cmd/compose/compose.go @@ -38,6 +38,7 @@ import ( "github.com/docker/cli/cli/command" "github.com/docker/compose/v2/cmd/formatter" "github.com/docker/compose/v2/internal/desktop" + "github.com/docker/compose/v2/internal/experimental" "github.com/docker/compose/v2/internal/tracing" "github.com/docker/compose/v2/pkg/api" "github.com/docker/compose/v2/pkg/compose" @@ -66,6 +67,14 @@ const ( ComposeEnvFiles = "COMPOSE_ENV_FILES" ) +type Backend interface { + api.Service + + SetDesktopClient(cli *desktop.Client) + + SetExperiments(experiments *experimental.State) +} + // Command defines a compose CLI command as a func with args type Command func(context.Context, []string) error @@ -326,7 +335,7 @@ func RunningAsStandalone() bool { } // RootCommand returns the compose command with its child commands -func RootCommand(dockerCli command.Cli, backend api.Service) *cobra.Command { //nolint:gocyclo +func RootCommand(dockerCli command.Cli, backend Backend) *cobra.Command { //nolint:gocyclo // filter out useless commandConn.CloseWrite warning message that can occur // when using a remote context that is unreachable: "commandConn.CloseWrite: commandconn: failed to wait: signal: killed" // https://github.com/docker/cli/blob/e1f24d3c93df6752d3c27c8d61d18260f141310c/cli/connhelper/commandconn/commandconn.go#L203-L215 @@ -337,6 +346,7 @@ func RootCommand(dockerCli command.Cli, backend api.Service) *cobra.Command { // "commandConn.CloseRead:", )) + experiments := experimental.NewState() opts := ProjectOptions{} var ( ansi string @@ -486,20 +496,32 @@ func RootCommand(dockerCli command.Cli, backend api.Service) *cobra.Command { // cmd.SetContext(ctx) // (6) Desktop integration - if db, ok := backend.(desktop.IntegrationService); ok { - if err := db.MaybeEnableDesktopIntegration(ctx); err != nil { + var desktopCli *desktop.Client + if !dryRun { + if desktopCli, err = desktop.NewFromDockerClient(ctx, dockerCli); desktopCli != nil { + logrus.Debugf("Enabled Docker Desktop integration (experimental) @ %s", desktopCli.Endpoint()) + backend.SetDesktopClient(desktopCli) + } else if err != nil { // not fatal, Compose will still work but behave as though // it's not running as part of Docker Desktop logrus.Debugf("failed to enable Docker Desktop integration: %v", err) + } else { + logrus.Trace("Docker Desktop integration not enabled") } } + // (7) experimental features + if err := experiments.Load(ctx, desktopCli); err != nil { + logrus.Debugf("Failed to query feature flags from Desktop: %v", err) + } + backend.SetExperiments(experiments) + return nil }, } c.AddCommand( - upCommand(&opts, dockerCli, backend), + upCommand(&opts, dockerCli, backend, experiments), downCommand(&opts, dockerCli, backend), startCommand(&opts, dockerCli, backend), restartCommand(&opts, dockerCli, backend), diff --git a/cmd/compose/up.go b/cmd/compose/up.go index 084e010fa..25382f9b7 100644 --- a/cmd/compose/up.go +++ b/cmd/compose/up.go @@ -27,6 +27,7 @@ import ( "github.com/compose-spec/compose-go/v2/types" "github.com/docker/cli/cli/command" "github.com/docker/compose/v2/cmd/formatter" + "github.com/docker/compose/v2/internal/experimental" xprogress "github.com/moby/buildkit/util/progress/progressui" "github.com/spf13/cobra" @@ -76,7 +77,7 @@ func (opts upOptions) apply(project *types.Project, services []string) (*types.P return project, nil } -func upCommand(p *ProjectOptions, dockerCli command.Cli, backend api.Service) *cobra.Command { +func upCommand(p *ProjectOptions, dockerCli command.Cli, backend api.Service, experiments *experimental.State) *cobra.Command { up := upOptions{} create := createOptions{} build := buildOptions{ProjectOptions: p} @@ -96,7 +97,7 @@ func upCommand(p *ProjectOptions, dockerCli command.Cli, backend api.Service) *c if len(up.attach) != 0 && up.attachDependencies { return errors.New("cannot combine --attach and --attach-dependencies") } - return runUp(ctx, dockerCli, backend, create, up, build, project, services) + return runUp(ctx, dockerCli, backend, experiments, create, up, build, project, services) }), ValidArgsFunction: completeServiceNames(dockerCli, p), } @@ -160,6 +161,7 @@ func runUp( ctx context.Context, dockerCli command.Cli, backend api.Service, + _ *experimental.State, createOptions createOptions, upOptions upOptions, buildOptions buildOptions, diff --git a/cmd/main.go b/cmd/main.go index 038d07cf5..af797eb6b 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -36,7 +36,10 @@ import ( func pluginMain() { plugin.Run(func(dockerCli command.Cli) *cobra.Command { - backend := compose.NewComposeService(dockerCli) + // TODO(milas): this cast is safe but we should not need to do this, + // we should expose the concrete service type so that we do not need + // to rely on the `api.Service` interface internally + backend := compose.NewComposeService(dockerCli).(commands.Backend) cmd := commands.RootCommand(dockerCli, backend) originalPreRunE := cmd.PersistentPreRunE cmd.PersistentPreRunE = func(cmd *cobra.Command, args []string) error { diff --git a/internal/desktop/client.go b/internal/desktop/client.go index e43a10a09..718c7889e 100644 --- a/internal/desktop/client.go +++ b/internal/desktop/client.go @@ -30,7 +30,8 @@ import ( // Client for integration with Docker Desktop features. type Client struct { - client *http.Client + apiEndpoint string + client *http.Client } // NewClient creates a Desktop integration client for the provided in-memory @@ -45,11 +46,16 @@ func NewClient(apiEndpoint string) *Client { transport = otelhttp.NewTransport(transport) c := &Client{ - client: &http.Client{Transport: transport}, + apiEndpoint: apiEndpoint, + client: &http.Client{Transport: transport}, } return c } +func (c *Client) Endpoint() string { + return c.apiEndpoint +} + // Close releases any open connections. func (c *Client) Close() error { c.client.CloseIdleConnections() @@ -84,6 +90,35 @@ func (c *Client) Ping(ctx context.Context) (*PingResponse, error) { return &ret, nil } +type FeatureFlagResponse map[string]FeatureFlagValue + +type FeatureFlagValue struct { + Enabled bool `json:"enabled"` +} + +func (c *Client) FeatureFlags(ctx context.Context) (FeatureFlagResponse, error) { + req, err := http.NewRequestWithContext(ctx, http.MethodGet, backendURL("/features"), http.NoBody) + if err != nil { + return nil, err + } + resp, err := c.client.Do(req) + if err != nil { + return nil, err + } + defer func() { + _ = resp.Body.Close() + }() + if resp.StatusCode != http.StatusOK { + return nil, fmt.Errorf("unexpected status code: %d", resp.StatusCode) + } + + var ret FeatureFlagResponse + if err := json.NewDecoder(resp.Body).Decode(&ret); err != nil { + return nil, err + } + return ret, nil +} + // backendURL generates a URL for the given API path. // // NOTE: Custom transport handles communication. The host is to create a valid diff --git a/internal/desktop/discovery.go b/internal/desktop/discovery.go new file mode 100644 index 000000000..6ec48265a --- /dev/null +++ b/internal/desktop/discovery.go @@ -0,0 +1,62 @@ +/* + Copyright 2024 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 desktop + +import ( + "context" + "fmt" + "strings" + "time" + + "github.com/docker/cli/cli/command" +) + +// engineLabelDesktopAddress is used to detect that Compose is running with a +// Docker Desktop context. When this label is present, the value is an endpoint +// address for an in-memory socket (AF_UNIX or named pipe). +const engineLabelDesktopAddress = "com.docker.desktop.address" + +// NewFromDockerClient creates a Desktop Client using the Docker CLI client to +// auto-discover the Desktop CLI socket endpoint (if available). +// +// An error is returned if there is a failure communicating with Docker Desktop, +// but even on success, a nil Client can be returned if the active Docker Engine +// is not a Desktop instance. +func NewFromDockerClient(ctx context.Context, dockerCli command.Cli) (*Client, error) { + // safeguard to make sure this doesn't get stuck indefinitely + ctx, cancel := context.WithTimeout(ctx, time.Second) + defer cancel() + + info, err := dockerCli.Client().Info(ctx) + if err != nil { + return nil, fmt.Errorf("querying server info: %w", err) + } + for _, l := range info.Labels { + k, v, ok := strings.Cut(l, "=") + if !ok || k != engineLabelDesktopAddress { + continue + } + + desktopCli := NewClient(v) + _, err := desktopCli.Ping(ctx) + if err != nil { + return nil, fmt.Errorf("pinging Desktop API: %w", err) + } + return desktopCli, nil + } + return nil, nil +} diff --git a/internal/desktop/integration.go b/internal/desktop/integration.go deleted file mode 100644 index 62dd4b931..000000000 --- a/internal/desktop/integration.go +++ /dev/null @@ -1,25 +0,0 @@ -/* - Copyright 2024 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 desktop - -import ( - "context" -) - -type IntegrationService interface { - MaybeEnableDesktopIntegration(ctx context.Context) error -} diff --git a/internal/experimental/experimental.go b/internal/experimental/experimental.go new file mode 100644 index 000000000..878757b6d --- /dev/null +++ b/internal/experimental/experimental.go @@ -0,0 +1,85 @@ +/* + Copyright 2024 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 experimental + +import ( + "context" + "os" + "strconv" + + "github.com/docker/compose/v2/internal/desktop" +) + +// envComposeExperimentalGlobal can be set to a falsy value (e.g. 0, false) to +// globally opt-out of any experimental features in Compose. +const envComposeExperimentalGlobal = "COMPOSE_EXPERIMENTAL" + +// State of experiments (enabled/disabled) based on environment and local config. +type State struct { + // active is false if experiments have been opted-out of globally. + active bool + desktopValues desktop.FeatureFlagResponse +} + +func NewState() *State { + // experimental features have individual controls, but users can opt out + // of ALL experiments easily if desired + experimentsActive := true + if v := os.Getenv(envComposeExperimentalGlobal); v != "" { + experimentsActive, _ = strconv.ParseBool(v) + } + return &State{ + active: experimentsActive, + } +} + +func (s *State) Load(ctx context.Context, client *desktop.Client) error { + if !s.active { + // user opted out of experiments globally, no need to load state from + // Desktop + return nil + } + + if client == nil { + // not running under Docker Desktop + return nil + } + + desktopValues, err := client.FeatureFlags(ctx) + if err != nil { + return err + } + s.desktopValues = desktopValues + return nil +} + +func (s *State) NavBar() bool { + return s.determineFeatureState("ComposeNav") +} + +func (s *State) AutoFileShares() bool { + return s.determineFeatureState("ComposeAutoFileShares") +} + +func (s *State) determineFeatureState(name string) bool { + if !s.active || s.desktopValues == nil { + return false + } + // TODO(milas): we should add individual environment variable overrides + // per-experiment in a generic way here + return s.desktopValues[name].Enabled +} diff --git a/internal/locker/pidfile_windows.go b/internal/locker/pidfile_windows.go index 9f8d4c3ee..adc151827 100644 --- a/internal/locker/pidfile_windows.go +++ b/internal/locker/pidfile_windows.go @@ -19,9 +19,10 @@ package locker import ( + "os" + "github.com/docker/docker/pkg/pidfile" "github.com/mitchellh/go-ps" - "os" ) func (f *Pidfile) Lock() error { diff --git a/pkg/compose/compose.go b/pkg/compose/compose.go index fa631bfad..e01ab931e 100644 --- a/pkg/compose/compose.go +++ b/pkg/compose/compose.go @@ -27,6 +27,7 @@ import ( "sync" "github.com/docker/compose/v2/internal/desktop" + "github.com/docker/compose/v2/internal/experimental" "github.com/docker/docker/api/types/volume" "github.com/jonboulle/clockwork" @@ -62,8 +63,9 @@ func NewComposeService(dockerCli command.Cli) api.Service { } type composeService struct { - dockerCli command.Cli - desktopCli *desktop.Client + dockerCli command.Cli + desktopCli *desktop.Client + experiments *experimental.State clock clockwork.Clock maxConcurrency int diff --git a/pkg/compose/desktop.go b/pkg/compose/desktop.go index 9af977fde..a50566aed 100644 --- a/pkg/compose/desktop.go +++ b/pkg/compose/desktop.go @@ -17,61 +17,14 @@ package compose import ( - "context" - "fmt" - "os" - "strconv" - "strings" - "time" - "github.com/docker/compose/v2/internal/desktop" - "github.com/sirupsen/logrus" + "github.com/docker/compose/v2/internal/experimental" ) -// engineLabelDesktopAddress is used to detect that Compose is running with a -// Docker Desktop context. When this label is present, the value is an endpoint -// address for an in-memory socket (AF_UNIX or named pipe). -const engineLabelDesktopAddress = "com.docker.desktop.address" - -var _ desktop.IntegrationService = &composeService{} - -// MaybeEnableDesktopIntegration initializes the desktop.Client instance if -// the server info from the Docker Engine is a Docker Desktop instance. -// -// EXPERIMENTAL: Requires `COMPOSE_EXPERIMENTAL_DESKTOP=1` env var set. -func (s *composeService) MaybeEnableDesktopIntegration(ctx context.Context) error { - if desktopEnabled, _ := strconv.ParseBool(os.Getenv("COMPOSE_EXPERIMENTAL_DESKTOP")); !desktopEnabled { - return nil - } - - if s.dryRun { - return nil - } - - // safeguard to make sure this doesn't get stuck indefinitely - ctx, cancel := context.WithTimeout(ctx, time.Second) - defer cancel() - - info, err := s.dockerCli.Client().Info(ctx) - if err != nil { - return fmt.Errorf("querying server info: %w", err) - } - for _, l := range info.Labels { - k, v, ok := strings.Cut(l, "=") - if !ok || k != engineLabelDesktopAddress { - continue - } - - desktopCli := desktop.NewClient(v) - _, err := desktopCli.Ping(ctx) - if err != nil { - return fmt.Errorf("pinging Desktop API: %w", err) - } - logrus.Debugf("Enabling Docker Desktop integration (experimental): %s", v) - s.desktopCli = desktopCli - return nil - } - - logrus.Trace("Docker Desktop not detected, no integration enabled") - return nil +func (s *composeService) SetDesktopClient(cli *desktop.Client) { + s.desktopCli = cli +} + +func (s *composeService) SetExperiments(experiments *experimental.State) { + s.experiments = experiments }