Skip to content

Commit

Permalink
Fix regression from #3446
Browse files Browse the repository at this point in the history
purported to fix scenarios when people commit simultaneously.
However, the lock required a stateDir, sourced from the container label, which does not exist if the container
was not created by nerdctl (eg: Kube).
This commit fix this, by making sure we have a stateDir label.

Signed-off-by: apostasie <[email protected]>
  • Loading branch information
apostasie committed Sep 25, 2024
1 parent 2defdb9 commit d991d97
Show file tree
Hide file tree
Showing 4 changed files with 44 additions and 25 deletions.
2 changes: 1 addition & 1 deletion pkg/cmd/container/commit.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ func Commit(ctx context.Context, client *containerd.Client, rawRef string, req s
if found.MatchCount > 1 {
return fmt.Errorf("multiple IDs found with provided prefix: %s", found.Req)
}
imageID, err := commit.Commit(ctx, client, found.Container, opts)
imageID, err := commit.Commit(ctx, client, found.Container, opts, options.GOptions)
if err != nil {
return err
}
Expand Down
23 changes: 17 additions & 6 deletions pkg/cmd/container/remove.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,8 +107,23 @@ func RemoveContainer(ctx context.Context, c containerd.Container, globalOptions
return err
}

// Get datastore
dataStore, err := clientutil.DataStore(globalOptions.DataRoot, globalOptions.Address)
if err != nil {
return err
}

// Ensure we do have a stateDir label
stateDir := containerLabels[labels.StateDir]
if stateDir == "" {
stateDir, err = containerutil.ContainerStateDirPath(globalOptions.Namespace, dataStore, c.ID())
if err != nil {
return err
}
}

// Lock the container state
lf, err := containerutil.Lock(ctx, c)
lf, err := containerutil.Lock(stateDir)
if err != nil {
return err
}
Expand All @@ -132,11 +147,7 @@ func RemoveContainer(ctx context.Context, c containerd.Container, globalOptions
if err != nil {
return err
}
// Get datastore
dataStore, err := clientutil.DataStore(globalOptions.DataRoot, globalOptions.Address)
if err != nil {
return err
}

// Get namestore
nameStore, err := namestore.New(dataStore, containerNamespace)
if err != nil {
Expand Down
17 changes: 1 addition & 16 deletions pkg/containerutil/lock.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,27 +17,12 @@
package containerutil

import (
"context"
"errors"
"path/filepath"

"github.com/containerd/containerd/v2/client"

"github.com/containerd/nerdctl/v2/pkg/labels"
"github.com/containerd/nerdctl/v2/pkg/store"
)

func Lock(ctx context.Context, c client.Container) (store.Store, error) {
containerLabels, err := c.Labels(ctx)
if err != nil {
return nil, err
}

stateDir := containerLabels[labels.StateDir]
if stateDir == "" {
return nil, errors.New("container is missing statedir label")
}

func Lock(stateDir string) (store.Store, error) {
stor, err := store.New(filepath.Join(stateDir, "oplock"), 0, 0)
if err != nil {
return nil, err
Expand Down
27 changes: 25 additions & 2 deletions pkg/imgutil/commit/commit.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ import (
"github.com/containerd/log"
"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/containerutil"
imgutil "github.com/containerd/nerdctl/v2/pkg/imgutil"
"github.com/containerd/nerdctl/v2/pkg/labels"
Expand All @@ -66,8 +68,29 @@ var (
emptyDigest = digest.Digest("")
)

func Commit(ctx context.Context, client *containerd.Client, container containerd.Container, opts *Opts) (digest.Digest, error) {
lf, err := containerutil.Lock(ctx, container)
func Commit(ctx context.Context, client *containerd.Client, container containerd.Container, opts *Opts, globalOptions types.GlobalCommandOptions) (digest.Digest, error) {
// Get labels
containerLabels, err := container.Labels(ctx)
if err != nil {
return emptyDigest, err
}

// Get datastore
dataStore, err := clientutil.DataStore(globalOptions.DataRoot, globalOptions.Address)
if err != nil {
return emptyDigest, err
}

// Ensure we do have a stateDir label
stateDir := containerLabels[labels.StateDir]
if stateDir == "" {
stateDir, err = containerutil.ContainerStateDirPath(globalOptions.Namespace, dataStore, container.ID())
if err != nil {
return emptyDigest, err
}
}

lf, err := containerutil.Lock(stateDir)
if err != nil {
return emptyDigest, err
}
Expand Down

0 comments on commit d991d97

Please sign in to comment.