Merge pull request #5972 from bergwolf/github/hook

fix moby prestart hook handling

src/runtime/pkg/containerd-shim-v2/create_test.go

@@ -41,7 +41,7 @@ func TestCreateSandboxSuccess(t *testing.T) {
 		},
 	}
 
-	testingImpl.CreateSandboxFunc = func(ctx context.Context, sandboxConfig vc.SandboxConfig) (vc.VCSandbox, error) {
+	testingImpl.CreateSandboxFunc = func(ctx context.Context, sandboxConfig vc.SandboxConfig, hookFunc func(context.Context) error) (vc.VCSandbox, error) {
 		return sandbox, nil
 	}
 

src/runtime/pkg/katautils/create.go

@@ -162,17 +162,19 @@ func CreateSandbox(ctx context.Context, vci vc.VC, ociSpec specs.Spec, runtimeCo
 	ociSpec.Annotations["nerdctl/network-namespace"] = sandboxConfig.NetworkConfig.NetworkID
 	sandboxConfig.Annotations["nerdctl/network-namespace"] = ociSpec.Annotations["nerdctl/network-namespace"]
 
-	// Run pre-start OCI hooks, in the runtime namespace.
+	sandbox, err := vci.CreateSandbox(ctx, sandboxConfig, func(ctx context.Context) error {
-	if err := PreStartHooks(ctx, ociSpec, containerID, bundlePath); err != nil {
+		// Run pre-start OCI hooks, in the runtime namespace.
-		return nil, vc.Process{}, err
+		if err := PreStartHooks(ctx, ociSpec, containerID, bundlePath); err != nil {
-	}
+			return err
+		}
 
-	// Run create runtime OCI hooks, in the runtime namespace.
+		// Run create runtime OCI hooks, in the runtime namespace.
-	if err := CreateRuntimeHooks(ctx, ociSpec, containerID, bundlePath); err != nil {
+		if err := CreateRuntimeHooks(ctx, ociSpec, containerID, bundlePath); err != nil {
-		return nil, vc.Process{}, err
+			return err
-	}
+		}
 
-	sandbox, err := vci.CreateSandbox(ctx, sandboxConfig)
+		return nil
+	})
 	if err != nil {
 		return nil, vc.Process{}, err
 	}
@@ -255,6 +257,12 @@ func CreateContainer(ctx context.Context, sandbox vc.VCSandbox, ociSpec specs.Sp
 		return vc.Process{}, err
 	}
 
+	hid, err := sandbox.GetHypervisorPid()
+	if err != nil {
+		return vc.Process{}, err
+	}
+	ctx = context.WithValue(ctx, vc.HypervisorPidKey{}, hid)
+
 	// Run pre-start OCI hooks.
 	err = EnterNetNS(sandbox.GetNetNs(), func() error {
 		return PreStartHooks(ctx, ociSpec, containerID, bundlePath)

src/runtime/pkg/katautils/create_test.go

@@ -274,7 +274,7 @@ func TestCreateSandboxAnnotations(t *testing.T) {
 
 	rootFs := vc.RootFs{Mounted: true}
 
-	testingImpl.CreateSandboxFunc = func(ctx context.Context, sandboxConfig vc.SandboxConfig) (vc.VCSandbox, error) {
+	testingImpl.CreateSandboxFunc = func(ctx context.Context, sandboxConfig vc.SandboxConfig, hookFunc func(context.Context) error) (vc.VCSandbox, error) {
 		return &vcmock.Sandbox{
 			MockID: testSandboxID,
 			MockContainers: []*vcmock.Container{

src/runtime/pkg/katautils/hook.go

@@ -17,6 +17,7 @@ import (
 
 	"github.com/kata-containers/kata-containers/src/runtime/pkg/katautils/katatrace"
 	syscallWrapper "github.com/kata-containers/kata-containers/src/runtime/pkg/syscall"
+	vc "github.com/kata-containers/kata-containers/src/runtime/virtcontainers"
 	"github.com/opencontainers/runtime-spec/specs-go"
 	"github.com/sirupsen/logrus"
 )
@@ -38,8 +39,16 @@ func runHook(ctx context.Context, spec specs.Spec, hook specs.Hook, cid, bundleP
 	defer span.End()
 	katatrace.AddTags(span, "path", hook.Path, "args", hook.Args)
 
+	pid, ok := ctx.Value(vc.HypervisorPidKey{}).(int)
+	if !ok || pid == 0 {
+		hookLogger().Info("no hypervisor pid")
+
+		pid = syscallWrapper.Gettid()
+	}
+	hookLogger().Infof("hypervisor pid %v", pid)
+
 	state := specs.State{
-		Pid:         syscallWrapper.Gettid(),
+		Pid:         pid,
 		Bundle:      bundlePath,
 		ID:          cid,
 		Annotations: spec.Annotations,

src/runtime/virtcontainers/api.go

@@ -44,16 +44,16 @@ func SetLogger(ctx context.Context, logger *logrus.Entry) {
 
 // CreateSandbox is the virtcontainers sandbox creation entry point.
 // CreateSandbox creates a sandbox and its containers. It does not start them.
-func CreateSandbox(ctx context.Context, sandboxConfig SandboxConfig, factory Factory) (VCSandbox, error) {
+func CreateSandbox(ctx context.Context, sandboxConfig SandboxConfig, factory Factory, prestartHookFunc func(context.Context) error) (VCSandbox, error) {
 	span, ctx := katatrace.Trace(ctx, virtLog, "CreateSandbox", apiTracingTags)
 	defer span.End()
 
-	s, err := createSandboxFromConfig(ctx, sandboxConfig, factory)
+	s, err := createSandboxFromConfig(ctx, sandboxConfig, factory, prestartHookFunc)
 
 	return s, err
 }
 
-func createSandboxFromConfig(ctx context.Context, sandboxConfig SandboxConfig, factory Factory) (_ *Sandbox, err error) {
+func createSandboxFromConfig(ctx context.Context, sandboxConfig SandboxConfig, factory Factory, prestartHookFunc func(context.Context) error) (_ *Sandbox, err error) {
 	span, ctx := katatrace.Trace(ctx, virtLog, "createSandboxFromConfig", apiTracingTags)
 	defer span.End()
 
@@ -88,7 +88,7 @@ func createSandboxFromConfig(ctx context.Context, sandboxConfig SandboxConfig, f
 	}
 
 	// Start the VM
-	if err = s.startVM(ctx); err != nil {
+	if err = s.startVM(ctx, prestartHookFunc); err != nil {
 		return nil, err
 	}
 

src/runtime/virtcontainers/api_test.go

@@ -145,7 +145,7 @@ func TestCreateSandboxNoopAgentSuccessful(t *testing.T) {
 	config := newTestSandboxConfigNoop()
 
 	ctx := WithNewAgentFunc(context.Background(), newMockAgent)
-	p, err := CreateSandbox(ctx, config, nil)
+	p, err := CreateSandbox(ctx, config, nil, nil)
 	assert.NoError(err)
 	assert.NotNil(p)
 
@@ -178,7 +178,7 @@ func TestCreateSandboxKataAgentSuccessful(t *testing.T) {
 	defer hybridVSockTTRPCMock.Stop()
 
 	ctx := WithNewAgentFunc(context.Background(), newMockAgent)
-	p, err := CreateSandbox(ctx, config, nil)
+	p, err := CreateSandbox(ctx, config, nil, nil)
 	assert.NoError(err)
 	assert.NotNil(p)
 
@@ -199,7 +199,7 @@ func TestCreateSandboxFailing(t *testing.T) {
 	config := SandboxConfig{}
 
 	ctx := WithNewAgentFunc(context.Background(), newMockAgent)
-	p, err := CreateSandbox(ctx, config, nil)
+	p, err := CreateSandbox(ctx, config, nil, nil)
 	assert.Error(err)
 	assert.Nil(p.(*Sandbox))
 }
@@ -227,7 +227,7 @@ func createAndStartSandbox(ctx context.Context, config SandboxConfig) (sandbox V
 	err error) {
 
 	// Create sandbox
-	sandbox, err = CreateSandbox(ctx, config, nil)
+	sandbox, err = CreateSandbox(ctx, config, nil, nil)
 	if sandbox == nil || err != nil {
 		return nil, "", err
 	}
@@ -260,7 +260,7 @@ func TestReleaseSandbox(t *testing.T) {
 	config := newTestSandboxConfigNoop()
 
 	ctx := WithNewAgentFunc(context.Background(), newMockAgent)
-	s, err := CreateSandbox(ctx, config, nil)
+	s, err := CreateSandbox(ctx, config, nil, nil)
 	assert.NoError(t, err)
 	assert.NotNil(t, s)
 

src/runtime/virtcontainers/example_pod_run_test.go

@@ -64,7 +64,7 @@ func Example_createAndStartSandbox() {
 	}
 
 	// Create the sandbox
-	s, err := vc.CreateSandbox(context.Background(), sandboxConfig, nil)
+	s, err := vc.CreateSandbox(context.Background(), sandboxConfig, nil, nil)
 	if err != nil {
 		fmt.Printf("Could not create sandbox: %s", err)
 		return

src/runtime/virtcontainers/implementation.go

@@ -31,8 +31,8 @@ func (impl *VCImpl) SetFactory(ctx context.Context, factory Factory) {
 }
 
 // CreateSandbox implements the VC function of the same name.
-func (impl *VCImpl) CreateSandbox(ctx context.Context, sandboxConfig SandboxConfig) (VCSandbox, error) {
+func (impl *VCImpl) CreateSandbox(ctx context.Context, sandboxConfig SandboxConfig, hookFunc func(context.Context) error) (VCSandbox, error) {
-	return CreateSandbox(ctx, sandboxConfig, impl.factory)
+	return CreateSandbox(ctx, sandboxConfig, impl.factory, hookFunc)
 }
 
 // CleanupContainer is used by shimv2 to stop and delete a container exclusively, once there is no container

src/runtime/virtcontainers/interfaces.go

@@ -23,7 +23,7 @@ type VC interface {
 	SetLogger(ctx context.Context, logger *logrus.Entry)
 	SetFactory(ctx context.Context, factory Factory)
 
-	CreateSandbox(ctx context.Context, sandboxConfig SandboxConfig) (VCSandbox, error)
+	CreateSandbox(ctx context.Context, sandboxConfig SandboxConfig, hookFunc func(context.Context) error) (VCSandbox, error)
 	CleanupContainer(ctx context.Context, sandboxID, containerID string, force bool) error
 }
 

src/runtime/virtcontainers/network_linux.go

@@ -252,6 +252,22 @@ func (n *LinuxNetwork) removeSingleEndpoint(ctx context.Context, s *Sandbox, idx
 	return nil
 }
 
+func (n *LinuxNetwork) endpointAlreadyAdded(netInfo *NetworkInfo) bool {
+	for _, ep := range n.eps {
+		// Existing endpoint
+		if ep.Name() == netInfo.Iface.Name {
+			return true
+		}
+		pair := ep.NetworkPair()
+		// Existing virtual endpoints
+		if pair != nil && (pair.TapInterface.Name == netInfo.Iface.Name || pair.TapInterface.TAPIface.Name == netInfo.Iface.Name || pair.VirtIface.Name == netInfo.Iface.Name) {
+			return true
+		}
+	}
+
+	return false
+}
+
 // Scan the networking namespace through netlink and then:
 // 1. Create the endpoints for the relevant interfaces found there.
 // 2. Attach them to the VM.
@@ -292,6 +308,12 @@ func (n *LinuxNetwork) addAllEndpoints(ctx context.Context, s *Sandbox, hotplug
 			continue
 		}
 
+		// Skip any interfaces that are already added
+		if n.endpointAlreadyAdded(&netInfo) {
+			networkLogger().WithField("endpoint", netInfo.Iface.Name).Info("already added")
+			continue
+		}
+
 		if err := doNetNS(n.netNSPath, func(_ ns.NetNS) error {
 			_, err = n.addSingleEndpoint(ctx, s, netInfo, hotplug)
 			return err

src/runtime/virtcontainers/pkg/vcmock/mock.go

@@ -42,9 +42,9 @@ func (m *VCMock) SetFactory(ctx context.Context, factory vc.Factory) {
 }
 
 // CreateSandbox implements the VC function of the same name.
-func (m *VCMock) CreateSandbox(ctx context.Context, sandboxConfig vc.SandboxConfig) (vc.VCSandbox, error) {
+func (m *VCMock) CreateSandbox(ctx context.Context, sandboxConfig vc.SandboxConfig, hookFunc func(context.Context) error) (vc.VCSandbox, error) {
 	if m.CreateSandboxFunc != nil {
-		return m.CreateSandboxFunc(ctx, sandboxConfig)
+		return m.CreateSandboxFunc(ctx, sandboxConfig, hookFunc)
 	}
 
 	return nil, fmt.Errorf("%s: %s (%+v): sandboxConfig: %v", mockErrorPrefix, getSelf(), m, sandboxConfig)

src/runtime/virtcontainers/pkg/vcmock/mock_test.go

@@ -120,22 +120,22 @@ func TestVCMockCreateSandbox(t *testing.T) {
 	assert.Nil(m.CreateSandboxFunc)
 
 	ctx := context.Background()
-	_, err := m.CreateSandbox(ctx, vc.SandboxConfig{})
+	_, err := m.CreateSandbox(ctx, vc.SandboxConfig{}, nil)
 	assert.Error(err)
 	assert.True(IsMockError(err))
 
-	m.CreateSandboxFunc = func(ctx context.Context, sandboxConfig vc.SandboxConfig) (vc.VCSandbox, error) {
+	m.CreateSandboxFunc = func(ctx context.Context, sandboxConfig vc.SandboxConfig, hookFunc func(context.Context) error) (vc.VCSandbox, error) {
 		return &Sandbox{}, nil
 	}
 
-	sandbox, err := m.CreateSandbox(ctx, vc.SandboxConfig{})
+	sandbox, err := m.CreateSandbox(ctx, vc.SandboxConfig{}, nil)
 	assert.NoError(err)
 	assert.Equal(sandbox, &Sandbox{})
 
 	// reset
 	m.CreateSandboxFunc = nil
 
-	_, err = m.CreateSandbox(ctx, vc.SandboxConfig{})
+	_, err = m.CreateSandbox(ctx, vc.SandboxConfig{}, nil)
 	assert.Error(err)
 	assert.True(IsMockError(err))
 }

src/runtime/virtcontainers/pkg/vcmock/types.go

@@ -88,6 +88,6 @@ type VCMock struct {
 	SetLoggerFunc  func(ctx context.Context, logger *logrus.Entry)
 	SetFactoryFunc func(ctx context.Context, factory vc.Factory)
 
-	CreateSandboxFunc    func(ctx context.Context, sandboxConfig vc.SandboxConfig) (vc.VCSandbox, error)
+	CreateSandboxFunc    func(ctx context.Context, sandboxConfig vc.SandboxConfig, hookFunc func(context.Context) error) (vc.VCSandbox, error)
 	CleanupContainerFunc func(ctx context.Context, sandboxID, containerID string, force bool) error
 }

src/runtime/virtcontainers/sandbox.go

@@ -92,6 +92,9 @@ var (
 	errSandboxNotRunning = errors.New("Sandbox not running")
 )
 
+// HypervisorPidKey is the context key for hypervisor pid
+type HypervisorPidKey struct{}
+
 // SandboxStatus describes a sandbox status.
 type SandboxStatus struct {
 	ContainersStatus []ContainerStatus
@@ -1194,7 +1197,7 @@ func (s *Sandbox) cleanSwap(ctx context.Context) {
 }
 
 // startVM starts the VM.
-func (s *Sandbox) startVM(ctx context.Context) (err error) {
+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})
 	defer span.End()
 
@@ -1234,9 +1237,24 @@ func (s *Sandbox) startVM(ctx context.Context) (err error) {
 		return err
 	}
 
+	if prestartHookFunc != nil {
+		hid, err := s.GetHypervisorPid()
+		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
+		}
+	}
+
 	// In case of vm factory, network interfaces are hotplugged
 	// after vm is started.
-	if s.factory != nil {
+	// In case of prestartHookFunc, network config might have been changed.
+	// We need to rescan and handle the change.
+	if s.factory != nil || prestartHookFunc != nil {
 		if _, err := s.network.AddEndpoints(ctx, s, nil, true); err != nil {
 			return err
 		}

src/runtime/virtcontainers/sandbox_test.go

@@ -1348,7 +1348,7 @@ func TestSandboxCreationFromConfigRollbackFromCreateSandbox(t *testing.T) {
 	// Ensure hypervisor doesn't exist
 	assert.NoError(os.Remove(hConf.HypervisorPath))
 
-	_, err := createSandboxFromConfig(ctx, sConf, nil)
+	_, err := createSandboxFromConfig(ctx, sConf, nil, nil)
 	// Fail at createSandbox: QEMU path does not exist, it is expected. Then rollback is called
 	assert.Error(err)