Skip to content

Commit

Permalink
Merge pull request #3522 from apostasie/cni-lock
Browse files Browse the repository at this point in the history
Enforce global lock in oci hooks
  • Loading branch information
AkihiroSuda authored Oct 10, 2024
2 parents 7eaaecb + 93fb53b commit 61b96a8
Showing 1 changed file with 21 additions and 0 deletions.
21 changes: 21 additions & 0 deletions pkg/ocihook/ocihook.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ import (
"github.com/containerd/nerdctl/v2/pkg/bypass4netnsutil"
"github.com/containerd/nerdctl/v2/pkg/dnsutil/hostsstore"
"github.com/containerd/nerdctl/v2/pkg/labels"
"github.com/containerd/nerdctl/v2/pkg/lockutil"
"github.com/containerd/nerdctl/v2/pkg/namestore"
"github.com/containerd/nerdctl/v2/pkg/netutil"
"github.com/containerd/nerdctl/v2/pkg/netutil/nettype"
Expand Down Expand Up @@ -92,6 +93,26 @@ func Run(stdin io.Reader, stderr io.Writer, event, dataStore, cniPath, cniNetcon
}
}()

// FIXME: CNI plugins are not safe to use concurrently
// See
// https://github.com/containerd/nerdctl/issues/3518
// https://github.com/containerd/nerdctl/issues/2908
// and likely others
// Fixing these issues would require a lot of work, possibly even stopping using individual cni binaries altogether
// or at least being very mindful in what operation we call inside CNIEnv at what point, with filesystem locking.
// This below is a stopgap solution that just enforces a global lock
// Note this here is probably not enough, as concurrent CNI operations may happen outside of the scope of ocihooks
// through explicit calls to Remove, etc.
err = os.MkdirAll(cniNetconfPath, 0o700)
if err != nil {
return err
}
lock, err := lockutil.Lock(cniNetconfPath)
if err != nil {
return err
}
defer lockutil.Unlock(lock)

opts, err := newHandlerOpts(&state, dataStore, cniPath, cniNetconfPath)
if err != nil {
return err
Expand Down

0 comments on commit 61b96a8

Please sign in to comment.