From 9488a1a81259693e3b43631a77d4aabcae688792 Mon Sep 17 00:00:00 2001 From: David Vossel Date: Mon, 9 Apr 2018 16:43:22 -0400 Subject: [PATCH 1/4] Add functional test to verify pod termination Signed-off-by: David Vossel --- tests/vmlifecycle_test.go | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/tests/vmlifecycle_test.go b/tests/vmlifecycle_test.go index a336d9967a5e..374942aa63d4 100644 --- a/tests/vmlifecycle_test.go +++ b/tests/vmlifecycle_test.go @@ -37,6 +37,7 @@ import ( "kubevirt.io/kubevirt/pkg/api/v1" "kubevirt.io/kubevirt/pkg/kubecli" + "kubevirt.io/kubevirt/pkg/virt-controller/services" "kubevirt.io/kubevirt/tests" ) @@ -351,6 +352,32 @@ var _ = Describe("Vmlifecycle", func() { }) Describe("Delete a VM", func() { + Context("with an active pod.", func() { + It("should result in pod being terminated", func(done Done) { + + By("Creating the VM") + obj, err := virtClient.VM(tests.NamespaceTestDefault).Create(vm) + Expect(err).ToNot(HaveOccurred()) + tests.WaitForSuccessfulVMStart(obj) + + By("Verifying VM's pod is active") + pods, err := virtClient.CoreV1().Pods(tests.NamespaceTestDefault).List(services.UnfinishedVMPodSelector(vm)) + Expect(err).ToNot(HaveOccurred()) + Expect(len(pods.Items)).To(Equal(1)) + + By("Deleting the VM") + Expect(virtClient.VM(vm.Namespace).Delete(obj.Name, &metav1.DeleteOptions{})).To(Succeed()) + + By("Verifying VM's pod terminates") + Eventually(func() int { + pods, err := virtClient.CoreV1().Pods(tests.NamespaceTestDefault).List(services.UnfinishedVMPodSelector(vm)) + Expect(err).ToNot(HaveOccurred()) + return len(pods.Items) + }, 75, 0.5).Should(Equal(0)) + + close(done) + }, 90) + }) Context("with grace period greater than 0", func() { It("should run graceful shutdown", func(done Done) { nodes, err := virtClient.CoreV1().Nodes().List(metav1.ListOptions{}) From b3fe6cb4f862410066a04b7e46ad04bbdabc5663 Mon Sep 17 00:00:00 2001 From: David Vossel Date: Tue, 10 Apr 2018 12:53:16 -0400 Subject: [PATCH 2/4] Add a log message to registry disk on exit Signed-off-by: David Vossel --- cmd/registry-disk-v1alpha/entry-point.sh | 2 ++ 1 file changed, 2 insertions(+) diff --git a/cmd/registry-disk-v1alpha/entry-point.sh b/cmd/registry-disk-v1alpha/entry-point.sh index 9f6b35049ac8..c673d4b5d09d 100755 --- a/cmd/registry-disk-v1alpha/entry-point.sh +++ b/cmd/registry-disk-v1alpha/entry-point.sh @@ -19,6 +19,8 @@ # https://fedoraproject.org/wiki/Scsi-target-utils_Quickstart_Guide +trap 'echo "Graceful exit"; exit 0' SIGINT SIGQUIT SIGTERM + if [ -z "$COPY_PATH" ]; then echo "COPY_PATH variable not set" exit 1 From 2ca42cd6322d3677b2c8db063f54fcdeb610bffb Mon Sep 17 00:00:00 2001 From: David Vossel Date: Tue, 10 Apr 2018 12:54:19 -0400 Subject: [PATCH 3/4] Ensure signal is forwarded to virt-launcher process Signed-off-by: David Vossel --- cmd/virt-launcher/entrypoint.sh | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/cmd/virt-launcher/entrypoint.sh b/cmd/virt-launcher/entrypoint.sh index ac1df31ed0f6..fe59d89eeaa2 100755 --- a/cmd/virt-launcher/entrypoint.sh +++ b/cmd/virt-launcher/entrypoint.sh @@ -1,6 +1,13 @@ #!/bin/bash set +e +_term() { + echo "caught signal" + kill -TERM "$virt_launcher_pid" 2>/dev/null +} + +trap _term SIGTERM SIGINT SIGQUIT + # HACK # Try to create /dev/kvm if not present if [ ! -e /dev/kvm ]; then @@ -10,13 +17,24 @@ fi chown :qemu /dev/kvm chmod 660 /dev/kvm - # Cockpit/OCP hack to all shoing the vm terminal mv /usr/bin/sh /usr/bin/sh.orig mv /sh.sh /usr/bin/sh chmod +x /usr/bin/sh -./virt-launcher $@ +./virt-launcher $@ & +virt_launcher_pid=$! +while true; do + if ! [ -d /proc/$virt_launcher_pid ]; then + break; + fi + sleep 1 +done +# call wait after we know the pid has exited in order +# to get the return code. If we call wait before the pid +# exits, wait will actually return early when we forward +# the trapped signal in _trap(). We don't want that. +wait -n $virt_launcher_pid rc=$? echo "virt-launcher exited with code $rc" From d45a4bb0a3c39b20a4c7feabd9bd0d1b8cb6a9a7 Mon Sep 17 00:00:00 2001 From: David Vossel Date: Tue, 10 Apr 2018 13:27:53 -0400 Subject: [PATCH 4/4] Ensure monitor loop exits early if pod shutdown is invoked and no pid is found This helps some of the inconsistencies we're seeing in func tests. The qemu pid would get created and destroyed so fast that the monitor polling loop would never even see the pid at all. In this case, SIGTERM signal was being ignored. Signed-off-by: David Vossel --- pkg/virt-launcher/monitor.go | 27 ++++++++++++++++++--------- pkg/virt-launcher/monitor_test.go | 24 ++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 9 deletions(-) diff --git a/pkg/virt-launcher/monitor.go b/pkg/virt-launcher/monitor.go index 395fcc6736b5..840450a4a11e 100644 --- a/pkg/virt-launcher/monitor.go +++ b/pkg/virt-launcher/monitor.go @@ -179,6 +179,16 @@ func NewProcessMonitor(commandPrefix string, } } +func (mon *monitor) isGracePeriodExpired() bool { + if mon.gracePeriodStartTime != 0 { + now := time.Now().UTC().Unix() + if (now - mon.gracePeriodStartTime) > int64(mon.gracePeriod) { + return true + } + } + return false +} + func (mon *monitor) refresh() { if mon.isDone { log.Log.Error("Called refresh after done!") @@ -187,6 +197,8 @@ func (mon *monitor) refresh() { log.Log.Debugf("Refreshing. CommandPrefix %s pid %d", mon.commandPrefix, mon.pid) + expired := mon.isGracePeriodExpired() + // is the process there? if mon.pid == 0 { var err error @@ -200,6 +212,9 @@ func (mon *monitor) refresh() { if mon.timeout > 0 && elapsed >= mon.timeout { log.Log.Infof("%s not found after timeout", mon.commandPrefix) mon.isDone = true + } else if expired { + log.Log.Infof("%s not found after grace period expired", mon.commandPrefix) + mon.isDone = true } return } @@ -219,12 +234,9 @@ func (mon *monitor) refresh() { return } - if mon.gracePeriodStartTime != 0 { - now := time.Now().UTC().Unix() - if (now - mon.gracePeriodStartTime) > int64(mon.gracePeriod) { - log.Log.Infof("Grace Period expired, shutting down.") - mon.shutdownCallback(mon.pid) - } + if expired { + log.Log.Infof("Grace Period expired, shutting down.") + mon.shutdownCallback(mon.pid) } return @@ -252,9 +264,6 @@ func (mon *monitor) monitorLoop(startTimeout time.Duration, signalChan chan os.S mon.refresh() case s := <-signalChan: log.Log.Infof("Received signal %d.", s) - if mon.pid == 0 { - continue - } if mon.gracePeriodStartTime != 0 { continue diff --git a/pkg/virt-launcher/monitor_test.go b/pkg/virt-launcher/monitor_test.go index 78fb0980b91b..bf76abd7a5a8 100644 --- a/pkg/virt-launcher/monitor_test.go +++ b/pkg/virt-launcher/monitor_test.go @@ -145,6 +145,30 @@ var _ = Describe("VirtLauncher", func() { Expect(exited).To(Equal(true)) }) + It("verify monitor loop exits when signal arrives and no pid is present", func() { + signalChannel := make(chan os.Signal, 1) + done := make(chan string) + + go func() { + mon.monitorLoop(1*time.Second, signalChannel) + done <- "exit" + }() + + time.Sleep(time.Second) + + signalChannel <- syscall.SIGQUIT + noExitCheck := time.After(5 * time.Second) + exited := false + + select { + case <-noExitCheck: + case <-done: + exited = true + } + + Expect(exited).To(Equal(true)) + }) + It("verify graceful shutdown trigger works", func() { signalChannel := make(chan os.Signal, 1) done := make(chan string)