Merge pull request #1354 from Random-Liu/cherrypick-#1351-release-1.2

Random-Liu · web-flow · commit 5420c6fb838d · 2019-12-09T13:47:55.000-08:00
[release/1.2] Better handle unknown state.
diff --git a/pkg/server/events.go b/pkg/server/events.go
@@ -337,6 +337,12 @@ func handleContainerExit(ctx context.Context, e *eventtypes.TaskExit, cntr conta
 		status.Pid = 0
 		status.FinishedAt = e.ExitedAt.UnixNano()
 		status.ExitCode = int32(e.ExitStatus)
+		// Unknown state can only transit to EXITED state, so we need
+		// to handle unknown state here.
+		if status.Unknown {
+			logrus.Debugf("Container %q transited from UNKNOWN to EXITED", cntr.ID)
+			status.Unknown = false
+		}
 		return status, nil
 	})
 	if err != nil {
diff --git a/pkg/server/helpers.go b/pkg/server/helpers.go
@@ -576,6 +576,7 @@ func unknownContainerStatus() containerstore.Status {
 		FinishedAt: 0,
 		ExitCode:   unknownExitCode,
 		Reason:     unknownExitReason,
+		Unknown:    true,
 	}
 }
 
diff --git a/pkg/server/restart.go b/pkg/server/restart.go
@@ -307,7 +307,9 @@ func (c *criService) loadContainer(ctx context.Context, cntr containerd.Containe
 	}()
 	if err != nil {
 		logrus.WithError(err).Errorf("Failed to load container status for %q", id)
-		status = unknownContainerStatus()
+		// Only set the unknown field in this case, because other fields may
+		// contain useful information loaded from the checkpoint.
+		status.Unknown = true
 	}
 	opts := []containerstore.Opts{
 		containerstore.WithStatus(status, containerDir),
diff --git a/pkg/store/container/status.go b/pkg/store/container/status.go
@@ -95,10 +95,16 @@ type Status struct {
 	// Removing indicates that the container is in removing state.
 	// This field doesn't need to be checkpointed.
 	Removing bool `json:"-"`
+	// Unknown indicates that the container status is not fully loaded.
+	// This field doesn't need to be checkpointed.
+	Unknown bool `json:"-"`
 }
 
 // State returns current state of the container based on the container status.
 func (s Status) State() runtime.ContainerState {
+	if s.Unknown {
+		return runtime.ContainerState_CONTAINER_UNKNOWN
+	}
 	if s.FinishedAt != 0 {
 		return runtime.ContainerState_CONTAINER_EXITED
 	}
diff --git a/pkg/store/container/status_test.go b/pkg/store/container/status_test.go
@@ -36,6 +36,12 @@ func TestContainerState(t *testing.T) {
 		state  runtime.ContainerState
 	}{
 		"unknown state": {
+			status: Status{
+				Unknown: true,
+			},
+			state: runtime.ContainerState_CONTAINER_UNKNOWN,
+		},
+		"unknown state because there is no timestamp set": {
 			status: Status{},
 			state:  runtime.ContainerState_CONTAINER_UNKNOWN,
 		},
@@ -76,6 +82,7 @@ func TestStatusEncodeDecode(t *testing.T) {
 		Message:    "test-message",
 		Removing:   true,
 		Starting:   true,
+		Unknown:    true,
 	}
 	assert := assertlib.New(t)
 	data, err := s.encode()
@@ -84,6 +91,7 @@ func TestStatusEncodeDecode(t *testing.T) {
 	assert.NoError(newS.decode(data))
 	s.Removing = false // Removing should not be encoded.
 	s.Starting = false // Starting should not be encoded.
+	s.Unknown = false  // Unknown should not be encoded.
 	assert.Equal(s, newS)
 
 	unsupported, err := json.Marshal(&versionedStatus{

Original file line number	Diff line number	Diff line change
`@@ -576,6 +576,7 @@ func unknownContainerStatus() containerstore.Status {`
`576`	`576`	`FinishedAt: 0,`
`577`	`577`	`ExitCode: unknownExitCode,`
`578`	`578`	`Reason: unknownExitReason,`
	`579`	`+ Unknown: true,`
`579`	`580`	`}`
`580`	`581`	`}`
`581`	`582`