Merge pull request #179 from sboeuf/fix_rollback

virtcontainers: Fix container creation rollback
kata-containers · Apr 4, 2018 · dda4a44 · dda4a44
2 parents e75713f + 1404928
commit dda4a44
Show file tree

Hide file tree

Showing 2 changed files with 26 additions and 22 deletions.
diff --git a/virtcontainers/container.go b/virtcontainers/container.go
@@ -420,17 +420,15 @@ func newContainer(pod *Pod, contConfig ContainerConfig) (*Container, error) {
 // been performed before the container creation failed.
 // - Unplug CPU and memory resources from the VM.
 // - Unplug devices from the VM.
-func rollbackFailingContainerCreation(c *Container, err error) {
-	if err != nil && c != nil {
-		if err2 := c.removeResources(); err2 != nil {
-			c.Logger().WithError(err2).Error("rollback failed removeResources()")
-		}
-		if err2 := c.detachDevices(); err2 != nil {
-			c.Logger().WithError(err2).Error("rollback failed detachDevices()")
-		}
-		if err2 := c.removeDrive(); err2 != nil {
-			c.Logger().WithError(err2).Error("rollback failed removeDrive()")
-		}
+func (c *Container) rollbackFailingContainerCreation() {
+	if err := c.removeResources(); err != nil {
+		c.Logger().WithError(err).Error("rollback failed removeResources()")
+	}
+	if err := c.detachDevices(); err != nil {
+		c.Logger().WithError(err).Error("rollback failed detachDevices()")
+	}
+	if err := c.removeDrive(); err != nil {
+		c.Logger().WithError(err).Error("rollback failed removeDrive()")
 	}
 }
 
@@ -452,7 +450,11 @@ func createContainer(pod *Pod, contConfig ContainerConfig) (c *Container, err er
 
 	// In case the container creation fails, the following takes care
 	// of rolling back all the actions previously performed.
-	defer rollbackFailingContainerCreation(c, err)
+	defer func() {
+		if err != nil {
+			c.rollbackFailingContainerCreation()
+		}
+	}()
 
 	if err = c.hotplugDrive(); err != nil {
 		return

diff --git a/virtcontainers/kata_agent.go b/virtcontainers/kata_agent.go
@@ -603,16 +603,14 @@ func (k *kataAgent) appendDevices(deviceList []*grpc.Device, devices []Device) [
 // been performed before the container creation failed.
 // - Unmount container volumes.
 // - Unmount container rootfs.
-func (k *kataAgent) rollbackFailingContainerCreation(c *Container, err error) {
-	if err != nil {
-		if c != nil {
-			if err2 := c.unmountHostMounts(); err2 != nil {
-				k.Logger().WithError(err2).Error("rollback failed unmountHostMounts()")
-			}
+func (k *kataAgent) rollbackFailingContainerCreation(c *Container) {
+	if c != nil {
+		if err2 := c.unmountHostMounts(); err2 != nil {
+			k.Logger().WithError(err2).Error("rollback failed unmountHostMounts()")
+		}
 
-			if err2 := bindUnmountContainerRootfs(kataHostSharedDir, c.pod.id, c.id); err2 != nil {
-				k.Logger().WithError(err2).Error("rollback failed bindUnmountContainerRootfs()")
-			}
+		if err2 := bindUnmountContainerRootfs(kataHostSharedDir, c.pod.id, c.id); err2 != nil {
+			k.Logger().WithError(err2).Error("rollback failed bindUnmountContainerRootfs()")
 		}
 	}
 }
@@ -639,7 +637,11 @@ func (k *kataAgent) createContainer(pod *Pod, c *Container) (p *Process, err err
 
 	// In case the container creation fails, the following defer statement
 	// takes care of rolling back actions previously performed.
-	defer k.rollbackFailingContainerCreation(c, err)
+	defer func() {
+		if err != nil {
+			k.rollbackFailingContainerCreation(c)
+		}
+	}()
 
 	if c.state.Fstype != "" {
 		// This is a block based device rootfs.