Skip to content

Commit 5fa9f12

Browse files
committed
Relax compose concurrency lock
Since compose implementation is currently unsafe to use concurrently, containerd#3543 did introduce a blanket locking mechanism for any and all compose operations. However, an unintended side-effect is that a call to `compose logs`, or other operations being called in no-detach mode, would effectively prevent the user from performing any further operation until these are quitting. This commit does make it so a call to `composer.Logs` does release the lock. Note the lock logic has also been moved to `pkg/composer`. Signed-off-by: apostasie <[email protected]>
1 parent 1f81225 commit 5fa9f12

File tree

3 files changed

+57
-16
lines changed

3 files changed

+57
-16
lines changed

pkg/cmd/compose/compose.go

Lines changed: 1 addition & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -30,13 +30,11 @@ import (
3030
"github.com/containerd/platforms"
3131

3232
"github.com/containerd/nerdctl/v2/pkg/api/types"
33-
"github.com/containerd/nerdctl/v2/pkg/clientutil"
3433
"github.com/containerd/nerdctl/v2/pkg/cmd/volume"
3534
"github.com/containerd/nerdctl/v2/pkg/composer"
3635
"github.com/containerd/nerdctl/v2/pkg/composer/serviceparser"
3736
"github.com/containerd/nerdctl/v2/pkg/imgutil"
3837
"github.com/containerd/nerdctl/v2/pkg/ipfs"
39-
"github.com/containerd/nerdctl/v2/pkg/lockutil"
4038
"github.com/containerd/nerdctl/v2/pkg/netutil"
4139
"github.com/containerd/nerdctl/v2/pkg/referenceutil"
4240
"github.com/containerd/nerdctl/v2/pkg/signutil"
@@ -48,20 +46,7 @@ var locked *os.File
4846

4947
// New returns a new *composer.Composer.
5048
func New(client *containerd.Client, globalOptions types.GlobalCommandOptions, options composer.Options, stdout, stderr io.Writer) (*composer.Composer, error) {
51-
// Compose right now cannot be made safe to use concurrently, as we shell out to nerdctl for multiple operations,
52-
// preventing us from using the lock mechanisms from the API.
53-
// This here imposes a global lock, effectively preventing multiple compose commands from being run in parallel and
54-
// preventing some of the problems with concurrent execution.
55-
// This should be removed once we have better, in-depth solutions to make this concurrency safe.
56-
// Note that we do not close the lock explicitly. Instead, the lock will get released when the `locked` global
57-
// variable will get collected and the file descriptor closed (eg: when the binary exits).
58-
var err error
59-
dataStore, err := clientutil.DataStore(globalOptions.DataRoot, globalOptions.Address)
60-
if err != nil {
61-
return nil, err
62-
}
63-
locked, err = lockutil.Lock(dataStore)
64-
if err != nil {
49+
if err := composer.Lock(globalOptions.DataRoot, globalOptions.Address); err != nil {
6550
return nil, err
6651
}
6752

pkg/composer/lock.go

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
/*
2+
Copyright The containerd Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package composer
18+
19+
import (
20+
"os"
21+
22+
"github.com/containerd/nerdctl/v2/pkg/clientutil"
23+
"github.com/containerd/nerdctl/v2/pkg/lockutil"
24+
)
25+
26+
//nolint:unused
27+
var locked *os.File
28+
29+
func Lock(dataRoot string, address string) error {
30+
// Compose right now cannot be made safe to use concurrently, as we shell out to nerdctl for multiple operations,
31+
// preventing us from using the lock mechanisms from the API.
32+
// This here allows to impose a global lock, effectively preventing multiple compose commands from being run in parallel and
33+
// preventing some of the problems with concurrent execution.
34+
// This should be removed once we have better, in-depth solutions to make compose concurrency safe.
35+
// Note that in most cases we do not close the lock explicitly. Instead, the lock will get released when the `locked` global
36+
// variable will get collected and the file descriptor closed (eg: when the binary exits).
37+
var err error
38+
dataStore, err := clientutil.DataStore(dataRoot, address)
39+
if err != nil {
40+
return err
41+
}
42+
locked, err = lockutil.Lock(dataStore)
43+
return err
44+
}
45+
46+
func Unlock() error {
47+
return lockutil.Unlock(locked)
48+
}

pkg/composer/logs.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,14 @@ type LogsOptions struct {
4444
}
4545

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

0 commit comments

Comments
 (0)