Merge pull request #7703 from bergwolf/github/nerdctl-fc

runtime: run prestart hooks before starting VM for FC

src/runtime/virtcontainers/acrn_arch_base.go

@@ -366,6 +366,7 @@ func (a *acrnArchBase) capabilities(config HypervisorConfig) types.Capabilities
 
 	caps.SetBlockDeviceSupport()
 	caps.SetBlockDeviceHotplugSupport()
+	caps.SetNetworkDeviceHotplugSupported()
 
 	return caps
 }

src/runtime/virtcontainers/acrn_arch_base_test.go

@@ -89,6 +89,7 @@ func TestAcrnArchBaseCapabilities(t *testing.T) {
 	assert.True(c.IsBlockDeviceSupported())
 	assert.True(c.IsBlockDeviceHotplugSupported())
 	assert.False(c.IsFsSharingSupported())
+	assert.True(c.IsNetworkDeviceHotplugSupported())
 }
 
 func TestAcrnArchBaseMemoryTopology(t *testing.T) {

src/runtime/virtcontainers/acrn_test.go

@@ -82,6 +82,7 @@ func TestAcrnCapabilities(t *testing.T) {
 	caps := a.Capabilities(a.ctx)
 	assert.True(caps.IsBlockDeviceSupported())
 	assert.True(caps.IsBlockDeviceHotplugSupported())
+	assert.True(caps.IsNetworkDeviceHotplugSupported())
 }
 
 func testAcrnAddDevice(t *testing.T, devInfo interface{}, devType DeviceType, expected []Device) {

src/runtime/virtcontainers/clh.go

@@ -1206,6 +1206,7 @@ func (clh *cloudHypervisor) Capabilities(ctx context.Context) types.Capabilities
 		caps.SetFsSharingSupport()
 	}
 	caps.SetBlockDeviceHotplugSupport()
+	caps.SetNetworkDeviceHotplugSupported()
 	return caps
 }
 

src/runtime/virtcontainers/clh_test.go

@@ -752,4 +752,7 @@ func TestClhCapabilities(t *testing.T) {
 
 	c = clh.Capabilities(ctx)
 	assert.False(c.IsFsSharingSupported())
+
+	assert.True(c.IsNetworkDeviceHotplugSupported())
+	assert.True(c.IsBlockDeviceHotplugSupported())
 }

src/runtime/virtcontainers/qemu_amd64.go

@@ -162,6 +162,7 @@ func (q *qemuAmd64) capabilities(hConfig HypervisorConfig) types.Capabilities {
 	if q.qemuMachine.Type == QemuQ35 ||
 		q.qemuMachine.Type == QemuVirt {
 		caps.SetBlockDeviceHotplugSupport()
+		caps.SetNetworkDeviceHotplugSupported()
 	}
 
 	caps.SetMultiQueueSupport()

src/runtime/virtcontainers/qemu_amd64_test.go

@@ -47,10 +47,12 @@ func TestQemuAmd64Capabilities(t *testing.T) {
 	amd64 := newTestQemu(assert, QemuQ35)
 	caps := amd64.capabilities(config)
 	assert.True(caps.IsBlockDeviceHotplugSupported())
+	assert.True(caps.IsNetworkDeviceHotplugSupported())
 
 	amd64 = newTestQemu(assert, QemuMicrovm)
 	caps = amd64.capabilities(config)
 	assert.False(caps.IsBlockDeviceHotplugSupported())
+	assert.False(caps.IsNetworkDeviceHotplugSupported())
 }
 
 func TestQemuAmd64Bridges(t *testing.T) {

src/runtime/virtcontainers/qemu_arch_base.go

@@ -307,6 +307,7 @@ func (q *qemuArchBase) capabilities(hConfig HypervisorConfig) types.Capabilities
 	var caps types.Capabilities
 	caps.SetBlockDeviceHotplugSupport()
 	caps.SetMultiQueueSupport()
+	caps.SetNetworkDeviceHotplugSupported()
 	if hConfig.SharedFS != config.NoSharedFS {
 		caps.SetFsSharingSupport()
 	}

src/runtime/virtcontainers/qemu_arch_base_test.go

@@ -123,6 +123,7 @@ func TestQemuArchBaseCapabilities(t *testing.T) {
 	c := qemuArchBase.capabilities(hConfig)
 	assert.True(c.IsBlockDeviceHotplugSupported())
 	assert.True(c.IsFsSharingSupported())
+	assert.True(c.IsNetworkDeviceHotplugSupported())
 
 	hConfig.SharedFS = config.NoSharedFS
 	c = qemuArchBase.capabilities(hConfig)

src/runtime/virtcontainers/qemu_ppc64le.go

@@ -104,6 +104,7 @@ func (q *qemuPPC64le) capabilities(hConfig HypervisorConfig) types.Capabilities
 	// pseries machine type supports hotplugging drives
 	if q.qemuMachine.Type == QemuPseries {
 		caps.SetBlockDeviceHotplugSupport()
+		caps.SetNetworkDeviceHotplugSupported()
 	}
 
 	caps.SetMultiQueueSupport()

src/runtime/virtcontainers/qemu_test.go

@@ -475,6 +475,7 @@ func TestQemuCapabilities(t *testing.T) {
 
 	caps := q.Capabilities(q.ctx)
 	assert.True(caps.IsBlockDeviceHotplugSupported())
+	assert.True(caps.IsNetworkDeviceHotplugSupported())
 }
 
 func TestQemuQemuPath(t *testing.T) {

src/runtime/virtcontainers/sandbox.go

@@ -955,6 +955,17 @@ func (s *Sandbox) createNetwork(ctx context.Context) error {
 		return nil
 	}
 
+	// docker container needs the hypervisor process ID to find out the container netns,
+	// which means that the hypervisor has to support network device hotplug so that docker
+	// can use the prestart hooks to set up container netns.
+	caps := s.hypervisor.Capabilities(ctx)
+	if !caps.IsNetworkDeviceHotplugSupported() {
+		spec := s.GetPatchedOCISpec()
+		if utils.IsDockerContainer(spec) {
+			return errors.New("docker container needs network device hotplug but the configured hypervisor does not support it")
+		}
+	}
+
 	span, ctx := katatrace.Trace(ctx, s.Logger(), "createNetwork", sandboxTracingTags, map[string]string{"sandbox_id": s.id})
 	defer span.End()
 	katatrace.AddTags(span, "network", s.network, "NetworkConfig", s.config.NetworkConfig)
@@ -1310,6 +1321,22 @@ func (s *Sandbox) cleanSwap(ctx context.Context) {
 	}
 }
 
+func (s *Sandbox) runPrestartHooks(ctx context.Context, prestartHookFunc func(context.Context) error) error {
+	hid, _ := s.GetHypervisorPid()
+	// Ignore errors here as hypervisor might not have been started yet, likely in FC case.
+	if hid > 0 {
+		s.Logger().Infof("sandbox %s hypervisor pid is %v", s.id, hid)
+		ctx = context.WithValue(ctx, HypervisorPidKey{}, hid)
+	}
+
+	if err := prestartHookFunc(ctx); err != nil {
+		s.Logger().Errorf("fail to run prestartHook for sandbox %s: %s", s.id, err)
+		return err
+	}
+
+	return nil
+}
+
 // startVM starts the VM.
 func (s *Sandbox) startVM(ctx context.Context, prestartHookFunc func(context.Context) error) (err error) {
 	span, ctx := katatrace.Trace(ctx, s.Logger(), "startVM", sandboxTracingTags, map[string]string{"sandbox_id": s.id})
@@ -1332,6 +1359,17 @@ func (s *Sandbox) startVM(ctx context.Context, prestartHookFunc func(context.Con
 		}
 	}()
 
+	caps := s.hypervisor.Capabilities(ctx)
+	// If the hypervisor does not support device hotplug, run prestart hooks
+	// before spawning the VM so that it is possible to let the hooks set up
+	// netns and thus network devices are set up statically.
+	if !caps.IsNetworkDeviceHotplugSupported() && prestartHookFunc != nil {
+		err = s.runPrestartHooks(ctx, prestartHookFunc)
+		if err != nil {
+			return err
+		}
+	}
+
 	if err := s.network.Run(ctx, func() error {
 		if s.factory != nil {
 			vm, err := s.factory.GetVM(ctx, VMConfig{
@@ -1351,24 +1389,22 @@ func (s *Sandbox) startVM(ctx context.Context, prestartHookFunc func(context.Con
 		return err
 	}
 
-	if prestartHookFunc != nil {
-		hid, err := s.GetHypervisorPid()
+	if caps.IsNetworkDeviceHotplugSupported() && prestartHookFunc != nil {
+		err = s.runPrestartHooks(ctx, prestartHookFunc)
 		if err != nil {
 			return err
 		}
-		s.Logger().Infof("hypervisor pid is %v", hid)
-		ctx = context.WithValue(ctx, HypervisorPidKey{}, hid)
-
-		if err := prestartHookFunc(ctx); err != nil {
-			return err
-		}
 	}
 
-	// 1. Do not scan the netns if we want no network for the vmm.
-	// 2. In case of vm factory, scan the netns to hotplug interfaces after vm is started.
-	// 3. In case of prestartHookFunc, network config might have been changed. We need to
+	// 1. Do not scan the netns if we want no network for the vmm
+	// 2. Do not scan the netns if the vmm does not support device hotplug, in which case
+	//    the network is already set up statically
+	// 3. In case of vm factory, scan the netns to hotplug interfaces after vm is started.
+	// 4. In case of prestartHookFunc, network config might have been changed. We need to
 	//    rescan and handle the change.
-	if !s.config.NetworkConfig.DisableNewNetwork && (s.factory != nil || prestartHookFunc != nil) {
+	if !s.config.NetworkConfig.DisableNewNetwork &&
+		caps.IsNetworkDeviceHotplugSupported() &&
+		(s.factory != nil || prestartHookFunc != nil) {
 		if _, err := s.network.AddEndpoints(ctx, s, nil, true); err != nil {
 			return err
 		}

src/runtime/virtcontainers/types/capabilities.go

@@ -10,6 +10,7 @@ const (
 	blockDeviceHotplugSupport
 	multiQueueSupport
 	fsSharingSupported
+	networkDeviceHotplugSupport
 )
 
 // Capabilities describe a virtcontainers hypervisor capabilities
@@ -57,3 +58,13 @@ func (caps *Capabilities) IsFsSharingSupported() bool {
 func (caps *Capabilities) SetFsSharingSupport() {
 	caps.flags |= fsSharingSupported
 }
+
+// IsDeviceHotplugSupported tells if an hypervisor supports device hotplug.
+func (caps *Capabilities) IsNetworkDeviceHotplugSupported() bool {
+	return caps.flags&networkDeviceHotplugSupport != 0
+}
+
+// SetDeviceHotplugSupported sets the host filesystem sharing capability to true.
+func (caps *Capabilities) SetNetworkDeviceHotplugSupported() {
+	caps.flags |= networkDeviceHotplugSupport
+}

src/runtime/virtcontainers/utils/utils.go

@@ -12,9 +12,11 @@ import (
 	"os"
 	"os/exec"
 	"path/filepath"
+	"strings"
 	"syscall"
 	"time"
 
+	"github.com/opencontainers/runtime-spec/specs-go"
 	"github.com/sirupsen/logrus"
 	"golang.org/x/sys/unix"
 
@@ -494,3 +496,21 @@ func RevertBytes(num uint64) uint64 {
 	}
 	return 1024*RevertBytes(a) + b
 }
+
+// IsDockerContainer returns if the container is managed by docker
+// This is done by checking the prestart hook for `libnetwork` arguments.
+func IsDockerContainer(spec *specs.Spec) bool {
+	if spec == nil || spec.Hooks == nil {
+		return false
+	}
+
+	for _, hook := range spec.Hooks.Prestart {
+		for _, arg := range hook.Args {
+			if strings.HasPrefix(arg, "libnetwork") {
+				return true
+			}
+		}
+	}
+
+	return false
+}

src/runtime/virtcontainers/utils/utils_test.go

@@ -16,6 +16,7 @@ import (
 	"syscall"
 	"testing"
 
+	"github.com/opencontainers/runtime-spec/specs-go"
 	"github.com/sirupsen/logrus"
 	"github.com/stretchr/testify/assert"
 )
@@ -580,3 +581,25 @@ func TestRevertBytes(t *testing.T) {
 	num := RevertBytes(testNum)
 	assert.Equal(expectedNum, num)
 }
+
+func TestIsDockerContainer(t *testing.T) {
+	assert := assert.New(t)
+
+	ociSpec := &specs.Spec{
+		Hooks: &specs.Hooks{
+			Prestart: []specs.Hook{
+				{
+					Args: []string{
+						"haha",
+					},
+				},
+			},
+		},
+	}
+	assert.False(IsDockerContainer(ociSpec))
+
+	ociSpec.Hooks.Prestart = append(ociSpec.Hooks.Prestart, specs.Hook{
+		Args: []string{"libnetwork-xxx"},
+	})
+	assert.True(IsDockerContainer(ociSpec))
+}