From 6b04814d96d2bae30a9e3aad4e9721eee9082723 Mon Sep 17 00:00:00 2001 From: David Ko Date: Sun, 16 Aug 2020 00:20:43 +0800 Subject: [PATCH] support removing networking from stopped vm --- cmd/ignite/run/stop.go | 7 ----- go.mod | 2 +- pkg/network/cni/cni.go | 15 ++++----- pkg/network/docker/docker.go | 2 +- pkg/network/types.go | 2 +- pkg/operations/remove.go | 61 +++++++++++++++++++----------------- vendor/modules.txt | 2 +- 7 files changed, 42 insertions(+), 49 deletions(-) diff --git a/cmd/ignite/run/stop.go b/cmd/ignite/run/stop.go index af2004d6d..37d0a4605 100644 --- a/cmd/ignite/run/stop.go +++ b/cmd/ignite/run/stop.go @@ -1,8 +1,6 @@ package run import ( - "fmt" - api "github.com/weaveworks/ignite/pkg/apis/ignite" "github.com/weaveworks/ignite/pkg/operations" ) @@ -24,11 +22,6 @@ func (sf *StopFlags) NewStopOptions(vmMatches []string) (so *stopOptions, err er func Stop(so *stopOptions) error { for _, vm := range so.vms { - // Check if the VM is running - if !vm.Running() { - return fmt.Errorf("VM %q is not running", vm.GetUID()) - } - // Stop the VM, and optionally kill it if err := operations.StopVM(vm, so.Kill, false); err != nil { return err diff --git a/go.mod b/go.mod index eb6b942c5..5b1c8ad4f 100644 --- a/go.mod +++ b/go.mod @@ -59,6 +59,6 @@ require ( golang.org/x/sys v0.0.0-20200610111108-226ff32320da gotest.tools v2.2.0+incompatible k8s.io/apimachinery v0.18.3 - k8s.io/kube-openapi v0.0.0-20200727223308-4c7aaf529f79 + k8s.io/kube-openapi v0.0.0-20200811211545-daf3cbb84823 sigs.k8s.io/yaml v1.2.0 ) diff --git a/pkg/network/cni/cni.go b/pkg/network/cni/cni.go index a2a3a5f35..fd890f9a4 100644 --- a/pkg/network/cni/cni.go +++ b/pkg/network/cni/cni.go @@ -189,7 +189,7 @@ func cniToIgniteResult(r *gocni.CNIResult) *network.Result { return result } -func (plugin *cniNetworkPlugin) RemoveContainerNetwork(containerID string, isRunning bool) (err error) { +func (plugin *cniNetworkPlugin) RemoveContainerNetwork(containerID string) (err error) { if err = plugin.initialize(); err != nil { return err } @@ -208,13 +208,10 @@ func (plugin *cniNetworkPlugin) RemoveContainerNetwork(containerID string, isRun return nil } - netnsPath := "" - if isRunning { - netnsPath = fmt.Sprintf(netNSPathFmt, c.PID) - if c.PID == 0 { - log.Info("CNI failed to retrieve network namespace path, PID was 0") - return nil - } + netnsPath := fmt.Sprintf(netNSPathFmt, c.PID) + if c.PID == 0 { + log.Info("CNI failed to retrieve network namespace path, PID was 0") + return nil } return plugin.cni.Remove(context.Background(), containerID, netnsPath) @@ -239,7 +236,7 @@ func (plugin *cniNetworkPlugin) cleanupBridges(containerID string) error { } if hasBridge { - log.Debugf("TeardownIPMasq for container %q on CNI network %q which contains a bridge", containerID, net.Config.Name) + log.Debugf("Teardown IPMasq for container %q on CNI network %q which contains a bridge", containerID, net.Config.Name) comment := utils.FormatComment(net.Config.Name, containerID) for _, t := range result { if err = ip.TeardownIPMasq(t.ip, t.chain, comment); err != nil { diff --git a/pkg/network/docker/docker.go b/pkg/network/docker/docker.go index e3377baa1..4d4192672 100644 --- a/pkg/network/docker/docker.go +++ b/pkg/network/docker/docker.go @@ -44,7 +44,7 @@ func (plugin *dockerNetworkPlugin) SetupContainerNetwork(containerID string, _ . }, nil } -func (*dockerNetworkPlugin) RemoveContainerNetwork(_ string, _ bool) error { +func (*dockerNetworkPlugin) RemoveContainerNetwork(_ string) error { // no-op for docker, this is handled automatically return nil } diff --git a/pkg/network/types.go b/pkg/network/types.go index 89273292c..f91ce053c 100644 --- a/pkg/network/types.go +++ b/pkg/network/types.go @@ -21,7 +21,7 @@ type Plugin interface { SetupContainerNetwork(containerID string, portmappings ...meta.PortMapping) (*Result, error) // RemoveContainerNetwork is the method called before a container using the network plugin can be deleted - RemoveContainerNetwork(containerID string, isRunning bool) error + RemoveContainerNetwork(containerID string) error } type Result struct { diff --git a/pkg/operations/remove.go b/pkg/operations/remove.go index d67f0205f..2786c254e 100644 --- a/pkg/operations/remove.go +++ b/pkg/operations/remove.go @@ -32,16 +32,11 @@ func CleanupVM(vm *api.VM) error { // Inspect the container before trying to stop it and it gets auto-removed inspectResult, _ := providers.Runtime.InspectContainer(util.NewPrefixer().Prefix(vm.GetUID())) - // If the VM is running, try to kill it first so we don't leave dangling containers - if vm.Running() { - if err := StopVM(vm, true, true); err != nil { + // If the VM is running, try to kill it first so we don't leave dangling containers. Otherwise, try to cleanup VM networking. + if err := StopVM(vm, true, true); err != nil { + if vm.Running() { return err } - } else { - // Try to cleanup VM networking - if err := removeNetworking(util.NewPrefixer().Prefix(vm.GetUID()), false); err != nil { - log.Warnf("Failed to cleanup networking for stopped container %s %q: %v", vm.GetKind(), vm.GetUID(), err) - } } // Remove the VM container if it exists @@ -77,43 +72,51 @@ func RemoveVMContainer(result *runtime.ContainerInspectResult) { _ = providers.Runtime.RemoveContainer(result.ID) } -// StopVM stops or kills a VM +// StopVM removes networking of the given VM and stops or kills it func StopVM(vm *api.VM, kill, silent bool) error { var err error container := util.NewPrefixer().Prefix(vm.GetUID()) action := "stop" + if !vm.Running() && !logs.Quiet { + log.Warnf("VM %q is not running but trying to cleanup networking for stopped container\n", vm.GetUID()) + } + // Remove VM networking - if err = removeNetworking(util.NewPrefixer().Prefix(vm.GetUID()), true); err != nil { + if err = removeNetworking(util.NewPrefixer().Prefix(vm.GetUID())); err != nil { + log.Warnf("Failed to cleanup networking for stopped container %s %q: %v", vm.GetKind(), vm.GetUID(), err) + return err } - // Stop or kill the VM container - if kill { - action = "kill" - err = providers.Runtime.KillContainer(container, signalSIGQUIT) // TODO: common constant for SIGQUIT - } else { - err = providers.Runtime.StopContainer(container, nil) - } + if vm.Running() { + // Stop or kill the VM container + if kill { + action = "kill" + err = providers.Runtime.KillContainer(container, signalSIGQUIT) // TODO: common constant for SIGQUIT + } else { + err = providers.Runtime.StopContainer(container, nil) + } - if err != nil { - return fmt.Errorf("failed to %s container for %s %q: %v", action, vm.GetKind(), vm.GetUID(), err) - } + if err != nil { + return fmt.Errorf("failed to %s container for %s %q: %v", action, vm.GetKind(), vm.GetUID(), err) + } - if silent { - return nil - } + if silent { + return nil + } - if logs.Quiet { - fmt.Println(vm.GetUID()) - } else { - log.Infof("Stopped %s with name %q and ID %q", vm.GetKind(), vm.GetName(), vm.GetUID()) + if logs.Quiet { + fmt.Println(vm.GetUID()) + } else { + log.Infof("Stopped %s with name %q and ID %q", vm.GetKind(), vm.GetName(), vm.GetUID()) + } } return nil } -func removeNetworking(containerID string, isRunning bool) error { +func removeNetworking(containerID string) error { log.Infof("Removing the container with ID %q from the %q network", containerID, providers.NetworkPlugin.Name()) - return providers.NetworkPlugin.RemoveContainerNetwork(containerID, isRunning) + return providers.NetworkPlugin.RemoveContainerNetwork(containerID) } diff --git a/vendor/modules.txt b/vendor/modules.txt index 29a977856..adb32c337 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -615,7 +615,7 @@ k8s.io/apimachinery/third_party/forked/golang/json k8s.io/apimachinery/third_party/forked/golang/reflect # k8s.io/klog v1.0.0 k8s.io/klog -# k8s.io/kube-openapi v0.0.0-20200727223308-4c7aaf529f79 => k8s.io/kube-openapi v0.0.0-20200121204235-bf4fb3bd569c +# k8s.io/kube-openapi v0.0.0-20200811211545-daf3cbb84823 => k8s.io/kube-openapi v0.0.0-20200121204235-bf4fb3bd569c ## explicit k8s.io/kube-openapi/pkg/common k8s.io/kube-openapi/pkg/util/proto