Merge pull request #429 from Joffref/sugate/fix-stop-vmm

Wait for cleanup to be done after stop VMM

Author: ginglis13

go.mod

@@ -18,6 +18,7 @@ require (
 	github.com/sirupsen/logrus v1.8.1
 	github.com/stretchr/testify v1.8.0
 	github.com/vishvananda/netlink v1.1.1-0.20210330154013-f5de75959ad5
+	github.com/vishvananda/netns v0.0.0-20210104183010-2eb08e3e575f
 	golang.org/x/crypto v0.0.0-20220622213112-05595931fe9d
 	golang.org/x/sys v0.0.0-20220204135822-1c1b9b1eba6a
 )

machine.go

@@ -270,6 +270,8 @@ type Machine struct {
 	// callbacks that should be run when the machine is being torn down
 	cleanupOnce  sync.Once
 	cleanupFuncs []func() error
+	// cleanupCh is a channel that gets closed to notify cleanup cleanupFuncs has been called totally
+	cleanupCh chan struct{}
 }
 
 // Logger returns a logrus logger appropriate for logging hypervisor messages
@@ -354,7 +356,8 @@ func configureBuilder(builder VMCommandBuilder, cfg Config) VMCommandBuilder {
 // provided Config.
 func NewMachine(ctx context.Context, cfg Config, opts ...Opt) (*Machine, error) {
 	m := &Machine{
-		exitCh: make(chan struct{}),
+		exitCh:    make(chan struct{}),
+		cleanupCh: make(chan struct{}),
 	}
 
 	if cfg.VMID == "" {
@@ -594,6 +597,8 @@ func (m *Machine) startVMM(ctx context.Context) error {
 		// When err is nil, two reads are performed (waitForSocket and close exitCh goroutine),
 		// second one never ends as it tries to read from empty channel.
 		close(errCh)
+		close(m.cleanupCh)
+
 	}()
 
 	m.setupSignals()
@@ -642,6 +647,8 @@ func (m *Machine) stopVMM() error {
 		if err != nil && !strings.Contains(err.Error(), "os: process already finished") {
 			return err
 		}
+		// Wait for the cleanup to finish.
+		<-m.cleanupCh
 		return nil
 	}
 	m.logger.Debug("stopVMM(): no firecracker process running, not sending a signal")

machine_test.go

@@ -19,6 +19,7 @@ import (
 	"errors"
 	"flag"
 	"fmt"
+	"github.com/vishvananda/netns"
 	"io"
 	"io/ioutil"
 	"net"
@@ -799,6 +800,71 @@ func testStopVMM(ctx context.Context, t *testing.T, m *Machine) {
 	}
 }
 
+func TestStopVMMCleanup(t *testing.T) {
+	fctesting.RequiresKVM(t)
+	fctesting.RequiresRoot(t)
+
+	socketPath, cleanup := makeSocketPath(t)
+	defer cleanup()
+
+	dir, err := ioutil.TempDir("", t.Name())
+	require.NoError(t, err)
+	defer os.RemoveAll(dir)
+
+	cniConfDir := filepath.Join(dir, "cni.conf")
+	err = os.MkdirAll(cniConfDir, 0777)
+	require.NoError(t, err)
+
+	cniBinPath := []string{testDataBin}
+
+	const networkName = "fcnet"
+	const ifName = "veth0"
+
+	networkMask := "/24"
+	subnet := "10.168.0.0" + networkMask
+
+	cniConfPath := fmt.Sprintf("%s/%s.conflist", cniConfDir, networkName)
+	err = writeCNIConfWithHostLocalSubnet(cniConfPath, networkName, subnet)
+	require.NoError(t, err)
+	defer os.Remove(cniConfPath)
+
+	networkInterface := NetworkInterface{
+		CNIConfiguration: &CNIConfiguration{
+			NetworkName: networkName,
+			IfName:      ifName,
+			ConfDir:     cniConfDir,
+			BinPath:     cniBinPath,
+			VMIfName:    "eth0",
+		},
+	}
+
+	cfg := Config{
+		SocketPath:        socketPath,
+		DisableValidation: true,
+		KernelImagePath:   getVmlinuxPath(t),
+		NetworkInterfaces: []NetworkInterface{networkInterface},
+		MachineCfg: models.MachineConfiguration{
+			VcpuCount:   Int64(1),
+			MemSizeMib:  Int64(64),
+			CPUTemplate: models.CPUTemplate(models.CPUTemplateT2),
+			Smt:         Bool(false),
+		},
+	}
+	ctx := context.Background()
+	cmd := VMCommandBuilder{}.
+		WithSocketPath(cfg.SocketPath).
+		WithBin(getFirecrackerBinaryPath()).
+		Build(ctx)
+	m, err := NewMachine(ctx, cfg, WithProcessRunner(cmd), WithLogger(fctesting.NewLogEntry(t)))
+	require.NoError(t, err)
+	err = m.Start(ctx)
+	require.NoError(t, err)
+	err = m.stopVMM()
+	require.NoError(t, err)
+	_, err = netns.GetFromName(m.Cfg.VMID)
+	require.Error(t, err)
+}
+
 func testShutdown(ctx context.Context, t *testing.T, m *Machine) {
 	err := m.Shutdown(ctx)
 	if err != nil {