From 8a9395964e4a5464fddabf705c0ce849a9bea51d Mon Sep 17 00:00:00 2001
From: Antonio Murdaca <runcom@redhat.com>
Date: Thu, 13 Oct 2016 10:50:51 +0200
Subject: [PATCH] server/sandbox: fix add/release pod/ctr ids and names

Signed-off-by: Antonio Murdaca <runcom@redhat.com>
---
 server/sandbox.go | 48 +++++++++++++++++++++++++++++++++++++++++------
 test/ctr.bats     | 34 +++++++++++++++++++++++++++++++++
 test/restore.bats |  4 ++--
 3 files changed, 78 insertions(+), 8 deletions(-)

diff --git a/server/sandbox.go b/server/sandbox.go
index da51d392..5144f13d 100644
--- a/server/sandbox.go
+++ b/server/sandbox.go
@@ -99,6 +99,25 @@ func (s *Server) RunPodSandbox(ctx context.Context, req *pb.RunPodSandboxRequest
 	if err != nil {
 		return nil, err
 	}
+
+	defer func() {
+		if err != nil {
+			s.releasePodName(name)
+		}
+	}()
+
+	if err = s.podIDIndex.Add(id); err != nil {
+		return nil, err
+	}
+
+	defer func() {
+		if err != nil {
+			if err = s.podIDIndex.Delete(id); err != nil {
+				logrus.Warnf("couldn't delete pod id %s from idIndex", id)
+			}
+		}
+	}()
+
 	podSandboxDir := filepath.Join(s.config.SandboxDir, id)
 	if _, err = os.Stat(podSandboxDir); err == nil {
 		return nil, fmt.Errorf("pod sandbox (%s) already exists", podSandboxDir)
@@ -106,7 +125,6 @@ func (s *Server) RunPodSandbox(ctx context.Context, req *pb.RunPodSandboxRequest
 
 	defer func() {
 		if err != nil {
-			s.releasePodName(name)
 			if err2 := os.RemoveAll(podSandboxDir); err2 != nil {
 				logrus.Warnf("couldn't cleanup podSandboxDir %s: %v", podSandboxDir, err2)
 			}
@@ -178,6 +196,28 @@ func (s *Server) RunPodSandbox(ctx context.Context, req *pb.RunPodSandboxRequest
 	}
 
 	containerID, containerName, err := s.generateContainerIDandName(name, "infra", 0)
+	if err != nil {
+		return nil, err
+	}
+
+	defer func() {
+		if err != nil {
+			s.releaseContainerName(containerName)
+		}
+	}()
+
+	if err = s.ctrIDIndex.Add(containerID); err != nil {
+		return nil, err
+	}
+
+	defer func() {
+		if err != nil {
+			if err = s.ctrIDIndex.Delete(containerID); err != nil {
+				logrus.Warnf("couldn't delete ctr id %s from idIndex", containerID)
+			}
+		}
+	}()
+
 	g.AddAnnotation("ocid/labels", string(labelsJSON))
 	g.AddAnnotation("ocid/annotations", string(annotationsJSON))
 	g.AddAnnotation("ocid/log_path", logDir)
@@ -273,10 +313,6 @@ func (s *Server) RunPodSandbox(ctx context.Context, req *pb.RunPodSandboxRequest
 
 	s.addContainer(container)
 
-	if err = s.podIDIndex.Add(id); err != nil {
-		return nil, err
-	}
-
 	if err = s.runtime.UpdateStatus(container); err != nil {
 		return nil, err
 	}
@@ -348,7 +384,7 @@ func (s *Server) RemovePodSandbox(ctx context.Context, req *pb.RemovePodSandboxR
 			continue
 		}
 
-		containerDir := filepath.Join(s.runtime.ContainerDir(), c.Name())
+		containerDir := filepath.Join(s.runtime.ContainerDir(), c.ID())
 		if err := os.RemoveAll(containerDir); err != nil {
 			return nil, fmt.Errorf("failed to remove container %s directory: %v", c.Name(), err)
 		}
diff --git a/test/ctr.bats b/test/ctr.bats
index 2244eda5..de300682 100644
--- a/test/ctr.bats
+++ b/test/ctr.bats
@@ -108,3 +108,37 @@ function teardown() {
 	cleanup_pods
 	stop_ocid
 }
+
+# regression test for #127
+@test "ctrs status for a pod" {
+	# this test requires docker, thus it can't yet be run in a container
+	if [ "$TRAVIS" = "true" ]; then # instead of $TRAVIS, add a function is_containerized to skip here
+		skip "cannot yet run this test in a container, use sudo make localintegration"
+	fi
+
+	start_ocid
+	run ocic pod create --config "$TESTDATA"/sandbox_config.json
+	echo "$output"
+	[ "$status" -eq 0 ]
+	pod_id="$output"
+	run ocic ctr create --config "$TESTDATA"/container_redis.json --pod "$pod_id"
+	echo "$output"
+	[ "$status" -eq 0 ]
+
+	run ocic ctr list --quiet
+	echo "$output"
+	[ "$status" -eq 0 ]
+	[[ "${output}" != "" ]]
+
+	printf '%s\n' "$output" | while IFS= read -r id
+	do
+		run ocic ctr status --id "$id"
+		echo "$output"
+		[ "$status" -eq 0 ]
+	done
+
+	cleanup_ctrs
+	cleanup_pods
+	stop_ocid
+}
+
diff --git a/test/restore.bats b/test/restore.bats
index b3b206a9..b3950a9a 100644
--- a/test/restore.bats
+++ b/test/restore.bats
@@ -30,13 +30,13 @@ function teardown() {
 	echo "$output"
 	[ "$status" -eq 0 ]
 	[[ "${output}" != "" ]]
-	[[ "${output}" =~ "${pod_id}"  ]]
+	[[ "${output}" =~ "${pod_id}" ]]
 
 	run ocic ctr list
 	echo "$output"
 	[ "$status" -eq 0 ]
 	[[ "${output}" != "" ]]
-	[[ "${output}" =~ "${pod_id}"  ]]
+	[[ "${output}" =~ "${pod_id}" ]]
 
 	cleanup_ctrs
 	cleanup_pods