From 0b2f6b53546463d4b30b55d0967ccb0544ce4098 Mon Sep 17 00:00:00 2001
From: Antonio Murdaca <runcom@redhat.com>
Date: Sat, 10 Jun 2017 18:18:59 +0200
Subject: [PATCH] adjust status on container start failure

Signed-off-by: Antonio Murdaca <runcom@redhat.com>
---
 oci/container.go           |  1 +
 oci/oci.go                 |  9 +++++++++
 server/container_start.go  | 23 +++++++++++++++++++----
 server/container_status.go | 13 ++++++++++---
 4 files changed, 39 insertions(+), 7 deletions(-)

diff --git a/oci/container.go b/oci/container.go
index 2e760762..42885db4 100644
--- a/oci/container.go
+++ b/oci/container.go
@@ -50,6 +50,7 @@ type ContainerState struct {
 	Finished  time.Time `json:"finished,omitempty"`
 	ExitCode  int32     `json:"exitCode,omitempty"`
 	OOMKilled bool      `json:"oomKilled,omitempty"`
+	Error     string    `json:"error,omitempty"`
 }
 
 // NewContainer creates a container object.
diff --git a/oci/oci.go b/oci/oci.go
index bc64b34c..b1091601 100644
--- a/oci/oci.go
+++ b/oci/oci.go
@@ -545,6 +545,15 @@ func (r *Runtime) DeleteContainer(c *Container) error {
 	return err
 }
 
+// SetStartFailed sets the container state appropriately after a start failure
+func (r *Runtime) SetStartFailed(c *Container, err error) {
+	c.opLock.Lock()
+	defer c.opLock.Unlock()
+	// adjust finished and started times
+	c.state.Finished, c.state.Started = c.state.Created, c.state.Created
+	c.state.Error = err.Error()
+}
+
 // UpdateStatus refreshes the status of the container.
 func (r *Runtime) UpdateStatus(c *Container) error {
 	c.opLock.Lock()
diff --git a/server/container_start.go b/server/container_start.go
index a426def9..6ef7d415 100644
--- a/server/container_start.go
+++ b/server/container_start.go
@@ -4,6 +4,7 @@ import (
 	"fmt"
 
 	"github.com/Sirupsen/logrus"
+	"github.com/kubernetes-incubator/cri-o/oci"
 	"golang.org/x/net/context"
 	pb "k8s.io/kubernetes/pkg/kubelet/api/v1alpha1/runtime"
 )
@@ -15,12 +16,26 @@ func (s *Server) StartContainer(ctx context.Context, req *pb.StartContainerReque
 	if err != nil {
 		return nil, err
 	}
-
-	if err = s.runtime.StartContainer(c); err != nil {
-		return nil, fmt.Errorf("failed to start container %s: %v", c.ID(), err)
+	state := s.runtime.ContainerStatus(c)
+	if state.Status != oci.ContainerStateCreated {
+		return nil, fmt.Errorf("container %s is not in created state: %s", c.ID(), state.Status)
 	}
 
-	s.containerStateToDisk(c)
+	defer func() {
+		// if the call to StartContainer fails below we still want to fill
+		// some fields of a container status. In particular, we're going to
+		// adjust container started/finished time and set an error to be
+		// returned in the Reason field for container status call.
+		if err != nil {
+			s.runtime.SetStartFailed(c, err)
+		}
+		s.containerStateToDisk(c)
+	}()
+
+	err = s.runtime.StartContainer(c)
+	if err != nil {
+		return nil, fmt.Errorf("failed to start container %s: %v", c.ID(), err)
+	}
 
 	resp := &pb.StartContainerResponse{}
 	logrus.Debugf("StartContainerResponse %+v", resp)
diff --git a/server/container_status.go b/server/container_status.go
index 3320365f..64c8c024 100644
--- a/server/container_status.go
+++ b/server/container_status.go
@@ -12,6 +12,12 @@ import (
 	pb "k8s.io/kubernetes/pkg/kubelet/api/v1alpha1/runtime"
 )
 
+const (
+	oomKilledReason = "OOMKilled"
+	completedReason = "Completed"
+	errorReason     = "Error"
+)
+
 // ContainerStatus returns status of the container.
 func (s *Server) ContainerStatus(ctx context.Context, req *pb.ContainerStatusRequest) (*pb.ContainerStatusResponse, error) {
 	logrus.Debugf("ContainerStatusRequest %+v", req)
@@ -100,11 +106,12 @@ func (s *Server) ContainerStatus(ctx context.Context, req *pb.ContainerStatusReq
 		resp.Status.ExitCode = cState.ExitCode
 		switch {
 		case cState.OOMKilled:
-			resp.Status.Reason = "OOMKilled"
+			resp.Status.Reason = oomKilledReason
 		case cState.ExitCode == 0:
-			resp.Status.Reason = "Completed"
+			resp.Status.Reason = completedReason
 		default:
-			resp.Status.Reason = "Error"
+			resp.Status.Reason = errorReason
+			resp.Status.Message = cState.Error
 		}
 	}