From 3371227794f5f3645f4f19829c60a741635ed329 Mon Sep 17 00:00:00 2001 From: Milas Bowman Date: Fri, 22 Mar 2024 11:21:01 -0400 Subject: [PATCH] chore(desktop): revised feature detection for file shares Unfortunately, the feature flag mechanism for experimental features isn't adequate. To avoid some edge cases where Compose might try to use Synchronized file shares with Desktop when the feature isn't available, we need to query a new endpoint. Before we move any of this out of experimental, we need to improve the integration here with Desktop & Compose so that we get all the necessary feature/experiment state up-front to reduce the quantity of IPC calls needed up-front. For now, there's some intentional redundancy to avoid making this extra call if we can avoid it. The actual endpoint is very cheap/ fast, but every IPC call is a potential point of of failure, so it's worth it. Signed-off-by: Milas Bowman --- internal/desktop/client.go | 31 +++++++++++++++++++++++++++++++ pkg/compose/create.go | 2 +- pkg/compose/desktop.go | 18 ++++++++++++++++++ pkg/compose/down.go | 2 +- 4 files changed, 51 insertions(+), 2 deletions(-) diff --git a/internal/desktop/client.go b/internal/desktop/client.go index 9d99040bf..77836e227 100644 --- a/internal/desktop/client.go +++ b/internal/desktop/client.go @@ -123,6 +123,37 @@ func (c *Client) FeatureFlags(ctx context.Context) (FeatureFlagResponse, error) return ret, nil } +type GetFileSharesConfigResponse struct { + Active bool `json:"active"` + Compose struct { + ManageBindMounts bool `json:"manageBindMounts"` + } +} + +func (c *Client) GetFileSharesConfig(ctx context.Context) (*GetFileSharesConfigResponse, error) { + req, err := http.NewRequestWithContext(ctx, http.MethodGet, backendURL("/mutagen/file-shares/config"), 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, newHTTPStatusCodeError(resp) + } + + var ret GetFileSharesConfigResponse + if err := json.NewDecoder(resp.Body).Decode(&ret); err != nil { + return nil, err + } + return &ret, nil +} + type CreateFileShareRequest struct { HostPath string `json:"hostPath"` Labels map[string]string `json:"labels,omitempty"` diff --git a/pkg/compose/create.go b/pkg/compose/create.go index b35c9814f..7dc3b115b 100644 --- a/pkg/compose/create.go +++ b/pkg/compose/create.go @@ -152,7 +152,7 @@ func (s *composeService) ensureProjectVolumes(ctx context.Context, project *type } err := func() error { - if s.experiments.AutoFileShares() && s.isDesktopIntegrationActive() { + if s.manageDesktopFileSharesEnabled(ctx) { // collect all the bind mount paths and try to set up file shares in // Docker Desktop for them var paths []string diff --git a/pkg/compose/desktop.go b/pkg/compose/desktop.go index a50566aed..5e7fb8fea 100644 --- a/pkg/compose/desktop.go +++ b/pkg/compose/desktop.go @@ -17,8 +17,11 @@ package compose import ( + "context" + "github.com/docker/compose/v2/internal/desktop" "github.com/docker/compose/v2/internal/experimental" + "github.com/sirupsen/logrus" ) func (s *composeService) SetDesktopClient(cli *desktop.Client) { @@ -28,3 +31,18 @@ func (s *composeService) SetDesktopClient(cli *desktop.Client) { func (s *composeService) SetExperiments(experiments *experimental.State) { s.experiments = experiments } + +func (s *composeService) manageDesktopFileSharesEnabled(ctx context.Context) bool { + // there's some slightly redundancy here to avoid fetching the config if + // we can already tell the feature state - in practice, we + if !s.isDesktopIntegrationActive() || !s.experiments.AutoFileShares() { + return false + } + + fileSharesConfig, err := s.desktopCli.GetFileSharesConfig(ctx) + if err != nil { + logrus.Debugf("Failed to retrieve file shares config: %v", err) + return false + } + return fileSharesConfig.Active && fileSharesConfig.Compose.ManageBindMounts +} diff --git a/pkg/compose/down.go b/pkg/compose/down.go index ca1f58fe9..b7887c5a0 100644 --- a/pkg/compose/down.go +++ b/pkg/compose/down.go @@ -145,7 +145,7 @@ func (s *composeService) ensureVolumesDown(ctx context.Context, project *types.P }) } - if s.experiments.AutoFileShares() && s.isDesktopIntegrationActive() { + if s.manageDesktopFileSharesEnabled(ctx) { ops = append(ops, func() error { desktop.RemoveFileSharesForProject(ctx, s.desktopCli, project.Name) return nil