Merge pull request containerd#3715 from apostasie/fix-compose-locking

Relax compose concurrency lock

Author: AkihiroSuda

pkg/cmd/compose/compose.go

@@ -30,13 +30,11 @@ import (
 	"github.com/containerd/platforms"
 
 	"github.com/containerd/nerdctl/v2/pkg/api/types"
-	"github.com/containerd/nerdctl/v2/pkg/clientutil"
 	"github.com/containerd/nerdctl/v2/pkg/cmd/volume"
 	"github.com/containerd/nerdctl/v2/pkg/composer"
 	"github.com/containerd/nerdctl/v2/pkg/composer/serviceparser"
 	"github.com/containerd/nerdctl/v2/pkg/imgutil"
 	"github.com/containerd/nerdctl/v2/pkg/ipfs"
-	"github.com/containerd/nerdctl/v2/pkg/lockutil"
 	"github.com/containerd/nerdctl/v2/pkg/netutil"
 	"github.com/containerd/nerdctl/v2/pkg/referenceutil"
 	"github.com/containerd/nerdctl/v2/pkg/signutil"
@@ -48,20 +46,7 @@ var locked *os.File
 
 // New returns a new *composer.Composer.
 func New(client *containerd.Client, globalOptions types.GlobalCommandOptions, options composer.Options, stdout, stderr io.Writer) (*composer.Composer, error) {
-	// Compose right now cannot be made safe to use concurrently, as we shell out to nerdctl for multiple operations,
-	// preventing us from using the lock mechanisms from the API.
-	// This here imposes a global lock, effectively preventing multiple compose commands from being run in parallel and
-	// preventing some of the problems with concurrent execution.
-	// This should be removed once we have better, in-depth solutions to make this concurrency safe.
-	// Note that we do not close the lock explicitly. Instead, the lock will get released when the `locked` global
-	// variable will get collected and the file descriptor closed (eg: when the binary exits).
-	var err error
-	dataStore, err := clientutil.DataStore(globalOptions.DataRoot, globalOptions.Address)
-	if err != nil {
-		return nil, err
-	}
-	locked, err = lockutil.Lock(dataStore)
-	if err != nil {
+	if err := composer.Lock(globalOptions.DataRoot, globalOptions.Address); err != nil {
 		return nil, err
 	}
 

pkg/composer/lock.go

@@ -0,0 +1,48 @@
+/*
+   Copyright The containerd 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 composer
+
+import (
+	"os"
+
+	"github.com/containerd/nerdctl/v2/pkg/clientutil"
+	"github.com/containerd/nerdctl/v2/pkg/lockutil"
+)
+
+//nolint:unused
+var locked *os.File
+
+func Lock(dataRoot string, address string) error {
+	// Compose right now cannot be made safe to use concurrently, as we shell out to nerdctl for multiple operations,
+	// preventing us from using the lock mechanisms from the API.
+	// This here allows to impose a global lock, effectively preventing multiple compose commands from being run in parallel and
+	// preventing some of the problems with concurrent execution.
+	// This should be removed once we have better, in-depth solutions to make compose concurrency safe.
+	// Note that in most cases we do not close the lock explicitly. Instead, the lock will get released when the `locked` global
+	// variable will get collected and the file descriptor closed (eg: when the binary exits).
+	var err error
+	dataStore, err := clientutil.DataStore(dataRoot, address)
+	if err != nil {
+		return err
+	}
+	locked, err = lockutil.Lock(dataStore)
+	return err
+}
+
+func Unlock() error {
+	return lockutil.Unlock(locked)
+}

pkg/composer/logs.go

@@ -44,6 +44,14 @@ type LogsOptions struct {
 }
 
 func (c *Composer) Logs(ctx context.Context, lo LogsOptions, services []string) error {
+	// Whether we called `compose logs`, or we are showing logs at the end of `up`, while in non detach mode, we need
+	// to release the lock. At this point, no operation will be performed that needs exclusive locking anymore, and
+	// not releasing the lock would otherwise unduly prevent further compose operations.
+	// See https://github.com/containerd/nerdctl/issues/3678
+	if err := Unlock(); err != nil {
+		return err
+	}
+
 	var serviceNames []string
 	err := c.project.ForEachService(services, func(name string, svc *types.ServiceConfig) error {
 		serviceNames = append(serviceNames, svc.Name)