diff --git a/virtcontainers/clh.go b/virtcontainers/clh.go index 53d60e777a..a90e48461e 100644 --- a/virtcontainers/clh.go +++ b/virtcontainers/clh.go @@ -446,7 +446,9 @@ func (clh *cloudHypervisor) addDevice(devInfo interface{}, devType deviceType) e switch v := devInfo.(type) { case Endpoint: - clh.addNet(v) + if err := clh.addNet(v); err != nil { + return err + } case types.HybridVSock: clh.addVSock(defaultGuestVSockCID, v.UdsPath) case types.Volume: @@ -522,7 +524,7 @@ func (clh *cloudHypervisor) terminate() (err error) { } } - clh.cleanupVM() + _ = clh.cleanupVM(true) }() pid := clh.state.PID @@ -1049,16 +1051,29 @@ func (clh *cloudHypervisor) addVSock(cid int64, path string) { clh.vmconfig.Vsock = append(clh.vmconfig.Vsock, chclient.VsockConfig{Cid: cid, Sock: path}) } -func (clh *cloudHypervisor) addNet(e Endpoint) { +func (clh *cloudHypervisor) addNet(e Endpoint) error { clh.Logger().WithField("endpoint-type", e).Debugf("Adding Endpoint of type %v", e) + mac := e.HardwareAddr() - tapPath := e.NetworkPair().TapInterface.TAPIface.Name + netPair := e.NetworkPair() + + if netPair == nil { + return errors.New("net Pair to be added is nil, needed to get TAP path") + } + + tapPath := netPair.TapInterface.TAPIface.Name + + if tapPath == "" { + return errors.New("TAP path in network pair is empty") + } + clh.Logger().WithFields(log.Fields{ "mac": mac, "tap": tapPath, }).Info("Adding Net") clh.vmconfig.Net = append(clh.vmconfig.Net, chclient.NetConfig{Mac: mac, Tap: tapPath}) + return nil } // Add shared Volume using virtiofs @@ -1095,25 +1110,37 @@ func (clh *cloudHypervisor) addVolume(volume types.Volume) error { } // cleanupVM will remove generated files and directories related with the virtual machine -func (clh *cloudHypervisor) cleanupVM() error { +func (clh *cloudHypervisor) cleanupVM(force bool) error { + + if clh.id == "" { + return errors.New("Hypervisor ID is empty") + } // cleanup vm path dir := filepath.Join(store.RunVMStoragePath(), clh.id) // If it's a symlink, remove both dir and the target. - // This can happen when vm template links a sandbox to a vm. link, err := filepath.EvalSymlinks(dir) if err != nil { - // Well, it's just cleanup failure. Let's ignore it. clh.Logger().WithError(err).WithField("dir", dir).Warn("failed to resolve vm path") } - clh.Logger().WithField("link", link).WithField("dir", dir).Infof("cleanup vm path") + + clh.Logger().WithFields(log.Fields{ + "link": link, + "dir": dir, + }).Infof("cleanup vm path") if err := os.RemoveAll(dir); err != nil { + if !force { + return err + } clh.Logger().WithError(err).Warnf("failed to remove vm path %s", dir) } if link != dir && link != "" { if err := os.RemoveAll(link); err != nil { + if !force { + return err + } clh.Logger().WithError(err).WithField("link", link).Warn("failed to remove resolved vm path") } } @@ -1121,10 +1148,16 @@ func (clh *cloudHypervisor) cleanupVM() error { if clh.config.VMid != "" { dir = store.SandboxConfigurationRootPath(clh.config.VMid) if err := os.RemoveAll(dir); err != nil { + if !force { + return err + } clh.Logger().WithError(err).WithField("path", dir).Warnf("failed to remove vm path") } dir = store.SandboxRuntimeRootPath(clh.config.VMid) if err := os.RemoveAll(dir); err != nil { + if !force { + return err + } clh.Logger().WithError(err).WithField("path", dir).Warnf("failed to remove vm path") } } diff --git a/virtcontainers/clh_test.go b/virtcontainers/clh_test.go index 9a7be249a0..8bd9870dad 100644 --- a/virtcontainers/clh_test.go +++ b/virtcontainers/clh_test.go @@ -8,12 +8,52 @@ package virtcontainers import ( "context" "net/http" + "os" + "path/filepath" "testing" + "github.com/kata-containers/runtime/virtcontainers/device/config" chclient "github.com/kata-containers/runtime/virtcontainers/pkg/cloud-hypervisor/client" + "github.com/kata-containers/runtime/virtcontainers/store" + "github.com/pkg/errors" "github.com/stretchr/testify/assert" ) +func newClhConfig() (HypervisorConfig, error) { + + setupClh() + + if testClhPath == "" { + return HypervisorConfig{}, errors.New("hypervisor fake path is empty") + } + + if testVirtiofsdPath == "" { + return HypervisorConfig{}, errors.New("hypervisor fake path is empty") + } + + if _, err := os.Stat(testClhPath); os.IsNotExist(err) { + return HypervisorConfig{}, err + } + + if _, err := os.Stat(testVirtiofsdPath); os.IsNotExist(err) { + return HypervisorConfig{}, err + } + + return HypervisorConfig{ + KernelPath: testClhKernelPath, + ImagePath: testClhImagePath, + HypervisorPath: testClhPath, + NumVCPUs: defaultVCPUs, + BlockDeviceDriver: config.VirtioBlock, + MemorySize: defaultMemSzMiB, + DefaultBridges: defaultBridges, + DefaultMaxVCPUs: MaxClhVCPUs(), + SharedFS: config.VirtioFS, + VirtioFSCache: virtioFsCacheAlways, + VirtioFSDaemon: testVirtiofsdPath, + }, nil +} + type clhClientMock struct { vmInfo chclient.VmInfo } @@ -54,6 +94,74 @@ func TestCloudHypervisorAddVSock(t *testing.T) { assert.Equal(clh.vmconfig.Vsock[1].Sock, "path2") } +// Check addNet appends to the network config list new configurations. +// Check that the elements in the list has the correct values +func TestCloudHypervisorAddNetCheckNetConfigListValues(t *testing.T) { + macTest := "00:00:00:00:00" + tapPath := "/path/to/tap" + + assert := assert.New(t) + + clh := cloudHypervisor{} + + e := &VethEndpoint{} + e.NetPair.TAPIface.HardAddr = macTest + e.NetPair.TapInterface.TAPIface.Name = tapPath + + err := clh.addNet(e) + assert.Nil(err) + + assert.Equal(len(clh.vmconfig.Net), 1) + if err == nil { + assert.Equal(clh.vmconfig.Net[0].Mac, macTest) + assert.Equal(clh.vmconfig.Net[0].Tap, tapPath) + } + + err = clh.addNet(e) + assert.Nil(err) + + assert.Equal(len(clh.vmconfig.Net), 2) + if err == nil { + assert.Equal(clh.vmconfig.Net[1].Mac, macTest) + assert.Equal(clh.vmconfig.Net[1].Tap, tapPath) + } +} + +// Check addNet with valid values, and fail with invalid values +// For Cloud Hypervisor only tap is be required +func TestCloudHypervisorAddNetCheckEnpointTypes(t *testing.T) { + assert := assert.New(t) + + tapPath := "/path/to/tap" + + validVeth := &VethEndpoint{} + validVeth.NetPair.TapInterface.TAPIface.Name = tapPath + + type args struct { + e Endpoint + } + tests := []struct { + name string + args args + wantErr bool + }{ + {"TapEndpoint", args{e: &TapEndpoint{}}, true}, + {"Empty VethEndpoint", args{e: &VethEndpoint{}}, true}, + {"Valid VethEndpoint", args{e: validVeth}, false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + clh := &cloudHypervisor{} + if err := clh.addNet(tt.args.e); (err != nil) != tt.wantErr { + t.Errorf("cloudHypervisor.addNet() error = %v, wantErr %v", err, tt.wantErr) + + } else if err == nil { + assert.Equal(clh.vmconfig.Net[0].Tap, tapPath) + } + }) + } +} + func TestCloudHypervisorBootVM(t *testing.T) { clh := &cloudHypervisor{} clh.APIClient = &clhClientMock{} @@ -62,3 +170,66 @@ func TestCloudHypervisorBootVM(t *testing.T) { t.Errorf("cloudHypervisor.bootVM() error = %v", err) } } + +func TestCloudHypervisorCleanupVM(t *testing.T) { + clh := &cloudHypervisor{} + + if err := clh.cleanupVM(true); err == nil { + t.Errorf("cloudHypervisor.cleanupVM() expected error != %v", err) + } + + clh.id = "cleanVMID" + + if err := clh.cleanupVM(true); err != nil { + t.Errorf("cloudHypervisor.cleanupVM() expected error != %v", err) + } + + dir := filepath.Join(store.RunVMStoragePath(), clh.id) + os.MkdirAll(dir, os.ModePerm) + + if err := clh.cleanupVM(false); err != nil { + t.Errorf("cloudHypervisor.cleanupVM() expected error != %v", err) + } + _, err := os.Stat(dir) + + if err == nil { + t.Errorf("dir should not exist %s", dir) + } + + if !os.IsNotExist(err) { + t.Errorf("Unexpected error = %v", err) + } +} + +func TestClhCreateSandbox(t *testing.T) { + assert := assert.New(t) + + clhConfig, err := newClhConfig() + assert.NoError(err) + + clh := &cloudHypervisor{ + config: clhConfig, + } + + sandbox := &Sandbox{ + ctx: context.Background(), + id: "testSandbox", + config: &SandboxConfig{ + HypervisorConfig: clhConfig, + }, + } + + vcStore, err := store.NewVCSandboxStore(sandbox.ctx, sandbox.id) + assert.NoError(err) + + sandbox.store = vcStore + + // Create parent dir path for hypervisor.json + parentDir := store.SandboxConfigurationRootPath(sandbox.id) + assert.NoError(os.MkdirAll(parentDir, store.DirMode)) + + err = clh.createSandbox(context.Background(), sandbox.id, NetworkNamespace{}, &sandbox.config.HypervisorConfig, sandbox.store, false) + assert.NoError(err) + assert.NoError(os.RemoveAll(parentDir)) + assert.Exactly(clhConfig, clh.config) +} diff --git a/virtcontainers/virtcontainers_test.go b/virtcontainers/virtcontainers_test.go index 185da52ab7..289516104b 100644 --- a/virtcontainers/virtcontainers_test.go +++ b/virtcontainers/virtcontainers_test.go @@ -25,6 +25,7 @@ const testKernel = "kernel" const testInitrd = "initrd" const testImage = "image" const testHypervisor = "hypervisor" +const testVirtiofsd = "virtiofsd" const testHypervisorCtl = "hypervisorctl" const testBundle = "bundle" @@ -49,6 +50,7 @@ var testAcrnKernelPath = "" var testAcrnImagePath = "" var testAcrnPath = "" var testAcrnCtlPath = "" +var testVirtiofsdPath = "" var testHyperstartCtlSocket = "" var testHyperstartTtySocket = "" @@ -91,7 +93,7 @@ func setupAcrn() { func setupClh() { os.Mkdir(filepath.Join(testDir, testBundle), store.DirMode) - for _, filename := range []string{testClhKernelPath, testClhImagePath, testClhPath} { + for _, filename := range []string{testClhKernelPath, testClhImagePath, testClhPath, testVirtiofsdPath} { _, err := os.Create(filename) if err != nil { fmt.Printf("Could not recreate %s:%v", filename, err) @@ -142,9 +144,10 @@ func TestMain(m *testing.M) { setupAcrn() - testClhKernelPath = filepath.Join(testDir, testKernel) - testClhImagePath = filepath.Join(testDir, testImage) - testClhPath = filepath.Join(testDir, testHypervisor) + testVirtiofsdPath = filepath.Join(testDir, testBundle, testVirtiofsd) + testClhKernelPath = filepath.Join(testDir, testBundle, testKernel) + testClhImagePath = filepath.Join(testDir, testBundle, testImage) + testClhPath = filepath.Join(testDir, testBundle, testHypervisor) setupClh()