Skip to content

Commit

Permalink
chore(desktop): revised feature detection for file shares
Browse files Browse the repository at this point in the history
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 <milas.bowman@docker.com>
  • Loading branch information
milas authored and ndeloof committed Mar 22, 2024
1 parent e9dc820 commit 3371227
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 2 deletions.
31 changes: 31 additions & 0 deletions internal/desktop/client.go
Expand Up @@ -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"`
Expand Down
2 changes: 1 addition & 1 deletion pkg/compose/create.go
Expand Up @@ -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
Expand Down
18 changes: 18 additions & 0 deletions pkg/compose/desktop.go
Expand Up @@ -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) {
Expand All @@ -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
}
2 changes: 1 addition & 1 deletion pkg/compose/down.go
Expand Up @@ -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
Expand Down

0 comments on commit 3371227

Please sign in to comment.