[WIP] Decouple Container Instance Health from the doctor.

agent/doctor/docker_runtime_healthcheck.go

@@ -19,7 +19,7 @@ import (
 	"time"
 
 	"github.com/aws/amazon-ecs-agent/agent/dockerclient/dockerapi"
-	"github.com/aws/amazon-ecs-agent/ecs-agent/doctor"
+	"github.com/aws/amazon-ecs-agent/ecs-agent/tcs/model/ecstcs"
 	"github.com/cihub/seelog"
 )
 
@@ -28,95 +28,104 @@ const systemPingTimeout = time.Second * 2
 var timeNow = time.Now
 
 type dockerRuntimeHealthcheck struct {
-	// HealthcheckType is the reported healthcheck type
+	// HealthcheckType is the reported healthcheck type.
 	HealthcheckType string `json:"HealthcheckType,omitempty"`
-	// Status is the container health status
-	Status doctor.HealthcheckStatus `json:"HealthcheckStatus,omitempty"`
-	// Timestamp is the timestamp when container health status changed
+	// Status is the container health status.
+	Status ecstcs.InstanceHealthCheckStatus `json:"HealthcheckStatus,omitempty"`
+	// TimeStamp is the timestamp when container health status changed.
 	TimeStamp time.Time `json:"TimeStamp,omitempty"`
-	// StatusChangeTime is the latest time the health status changed
+	// StatusChangeTime is the latest time the health status changed.
 	StatusChangeTime time.Time `json:"StatusChangeTime,omitempty"`
 
-	// LastStatus is the last container health status
-	LastStatus doctor.HealthcheckStatus `json:"LastStatus,omitempty"`
-	// LastTimeStamp is the timestamp of last container health status
+	// LastStatus is the last container health status.
+	LastStatus ecstcs.InstanceHealthCheckStatus `json:"LastStatus,omitempty"`
+	// LastTimeStamp is the timestamp of last container health status.
 	LastTimeStamp time.Time `json:"LastTimeStamp,omitempty"`
 
 	client dockerapi.DockerClient
 	lock   sync.RWMutex
 }
 
+// NewDockerRuntimeHealthcheck creates a new Docker runtime health check.
 func NewDockerRuntimeHealthcheck(client dockerapi.DockerClient) *dockerRuntimeHealthcheck {
 	nowTime := timeNow()
 	return &dockerRuntimeHealthcheck{
-		HealthcheckType:  doctor.HealthcheckTypeContainerRuntime,
-		Status:           doctor.HealthcheckStatusInitializing,
+		HealthcheckType:  ecstcs.InstanceHealthCheckTypeContainerRuntime,
+		Status:           ecstcs.InstanceHealthCheckStatusInitializing,
 		TimeStamp:        nowTime,
 		StatusChangeTime: nowTime,
 		LastTimeStamp:    nowTime,
 		client:           client,
 	}
 }
 
-func (dhc *dockerRuntimeHealthcheck) RunCheck() doctor.HealthcheckStatus {
-	// TODO pass in context as an argument
+// RunCheck performs a health check by pinging the Docker daemon.
+func (dhc *dockerRuntimeHealthcheck) RunCheck() ecstcs.InstanceHealthCheckStatus {
+	// TODO: Pass in context as an argument.
 	res := dhc.client.SystemPing(context.TODO(), systemPingTimeout)
-	resultStatus := doctor.HealthcheckStatusOk
+	resultStatus := ecstcs.InstanceHealthCheckStatusOk
 	if res.Error != nil {
 		seelog.Infof("[DockerRuntimeHealthcheck] Docker Ping failed with error: %v", res.Error)
-		resultStatus = doctor.HealthcheckStatusImpaired
+		resultStatus = ecstcs.InstanceHealthCheckStatusImpaired
 	}
 	dhc.SetHealthcheckStatus(resultStatus)
 	return resultStatus
 }
 
-func (dhc *dockerRuntimeHealthcheck) SetHealthcheckStatus(healthStatus doctor.HealthcheckStatus) {
+// SetHealthcheckStatus updates the health check status and timestamps.
+func (dhc *dockerRuntimeHealthcheck) SetHealthcheckStatus(healthStatus ecstcs.InstanceHealthCheckStatus) {
 	dhc.lock.Lock()
 	defer dhc.lock.Unlock()
 	nowTime := time.Now()
-	// if the status has changed, update status change timestamp
+	// If the status has changed, update status change timestamp.
 	if dhc.Status != healthStatus {
 		dhc.StatusChangeTime = nowTime
 	}
-	// track previous status
+	// Track previous status.
 	dhc.LastStatus = dhc.Status
 	dhc.LastTimeStamp = dhc.TimeStamp
 
-	// update latest status
+	// Update latest status.
 	dhc.Status = healthStatus
 	dhc.TimeStamp = nowTime
 }
 
+// GetHealthcheckType returns the type of this health check.
 func (dhc *dockerRuntimeHealthcheck) GetHealthcheckType() string {
 	dhc.lock.RLock()
 	defer dhc.lock.RUnlock()
 	return dhc.HealthcheckType
 }
 
-func (dhc *dockerRuntimeHealthcheck) GetHealthcheckStatus() doctor.HealthcheckStatus {
+// GetHealthcheckStatus returns the current health check status.
+func (dhc *dockerRuntimeHealthcheck) GetHealthcheckStatus() ecstcs.InstanceHealthCheckStatus {
 	dhc.lock.RLock()
 	defer dhc.lock.RUnlock()
 	return dhc.Status
 }
 
+// GetHealthcheckTime returns the timestamp of the current health check status.
 func (dhc *dockerRuntimeHealthcheck) GetHealthcheckTime() time.Time {
 	dhc.lock.RLock()
 	defer dhc.lock.RUnlock()
 	return dhc.TimeStamp
 }
 
+// GetStatusChangeTime returns the timestamp when the status last changed.
 func (dhc *dockerRuntimeHealthcheck) GetStatusChangeTime() time.Time {
 	dhc.lock.RLock()
 	defer dhc.lock.RUnlock()
 	return dhc.StatusChangeTime
 }
 
-func (dhc *dockerRuntimeHealthcheck) GetLastHealthcheckStatus() doctor.HealthcheckStatus {
+// GetLastHealthcheckStatus returns the previous health check status.
+func (dhc *dockerRuntimeHealthcheck) GetLastHealthcheckStatus() ecstcs.InstanceHealthCheckStatus {
 	dhc.lock.RLock()
 	defer dhc.lock.RUnlock()
 	return dhc.LastStatus
 }
 
+// GetLastHealthcheckTime returns the timestamp of the previous health check status.
 func (dhc *dockerRuntimeHealthcheck) GetLastHealthcheckTime() time.Time {
 	dhc.lock.RLock()
 	defer dhc.lock.RUnlock()

agent/doctor/docker_runtime_healthcheck_test.go

@@ -9,7 +9,7 @@ import (
 
 	"github.com/aws/amazon-ecs-agent/agent/dockerclient/dockerapi"
 	mock_dockerapi "github.com/aws/amazon-ecs-agent/agent/dockerclient/dockerapi/mocks"
-	"github.com/aws/amazon-ecs-agent/ecs-agent/doctor"
+	"github.com/aws/amazon-ecs-agent/ecs-agent/tcs/model/ecstcs"
 	"github.com/docker/docker/api/types"
 	"github.com/golang/mock/gomock"
 	"github.com/stretchr/testify/assert"
@@ -27,8 +27,8 @@ func TestNewDockerRuntimeHealthCheck(t *testing.T) {
 	defer func() { timeNow = originalTimeNow }()
 
 	expectedDockerRuntimeHealthcheck := &dockerRuntimeHealthcheck{
-		HealthcheckType:  doctor.HealthcheckTypeContainerRuntime,
-		Status:           doctor.HealthcheckStatusInitializing,
+		HealthcheckType:  ecstcs.InstanceHealthCheckTypeContainerRuntime,
+		Status:           ecstcs.InstanceHealthCheckStatusInitializing,
 		TimeStamp:        mockTime,
 		StatusChangeTime: mockTime,
 		LastTimeStamp:    mockTime,
@@ -42,26 +42,26 @@ func TestRunCheck(t *testing.T) {
 	testcases := []struct {
 		name               string
 		dockerPingResponse *dockerapi.PingResponse
-		expectedStatus     doctor.HealthcheckStatus
-		expectedLastStatus doctor.HealthcheckStatus
+		expectedStatus     ecstcs.InstanceHealthCheckStatus
+		expectedLastStatus ecstcs.InstanceHealthCheckStatus
 	}{
 		{
 			name: "empty checks",
 			dockerPingResponse: &dockerapi.PingResponse{
 				Response: &types.Ping{APIVersion: "test_api_version"},
 				Error:    nil,
 			},
-			expectedStatus:     doctor.HealthcheckStatusOk,
-			expectedLastStatus: doctor.HealthcheckStatusInitializing,
+			expectedStatus:     ecstcs.InstanceHealthCheckStatusOk,
+			expectedLastStatus: ecstcs.InstanceHealthCheckStatusInitializing,
 		},
 		{
 			name: "all true checks",
 			dockerPingResponse: &dockerapi.PingResponse{
 				Response: nil,
 				Error:    &dockerapi.DockerTimeoutError{},
 			},
-			expectedStatus:     doctor.HealthcheckStatusImpaired,
-			expectedLastStatus: doctor.HealthcheckStatusInitializing,
+			expectedStatus:     ecstcs.InstanceHealthCheckStatusImpaired,
+			expectedLastStatus: ecstcs.InstanceHealthCheckStatusInitializing,
 		},
 	}
 	ctrl := gomock.NewController(t)
@@ -85,9 +85,9 @@ func TestSetHealthCheckStatus(t *testing.T) {
 	defer ctrl.Finish()
 	dockerClient := mock_dockerapi.NewMockDockerClient(ctrl)
 	dockerRuntimeHealthCheck := NewDockerRuntimeHealthcheck(dockerClient)
-	healthCheckStatus := doctor.HealthcheckStatusOk
+	healthCheckStatus := ecstcs.InstanceHealthCheckStatusOk
 	dockerRuntimeHealthCheck.SetHealthcheckStatus(healthCheckStatus)
-	assert.Equal(t, doctor.HealthcheckStatusOk, dockerRuntimeHealthCheck.Status)
+	assert.Equal(t, ecstcs.InstanceHealthCheckStatusOk, dockerRuntimeHealthCheck.Status)
 }
 
 func TestSetHealthcheckStatusChange(t *testing.T) {
@@ -96,23 +96,23 @@ func TestSetHealthcheckStatusChange(t *testing.T) {
 	dockerClient := mock_dockerapi.NewMockDockerClient(ctrl)
 	dockerRuntimeHealthcheck := NewDockerRuntimeHealthcheck(dockerClient)
 
-	// we should start in initializing status
-	assert.Equal(t, doctor.HealthcheckStatusInitializing, dockerRuntimeHealthcheck.Status)
+	// We should start in initializing status.
+	assert.Equal(t, ecstcs.InstanceHealthCheckStatusInitializing, dockerRuntimeHealthcheck.Status)
 	initializationChangeTime := dockerRuntimeHealthcheck.GetStatusChangeTime()
 
-	// we update to initializing again; our StatusChangeTime remains the same
-	dockerRuntimeHealthcheck.SetHealthcheckStatus(doctor.HealthcheckStatusInitializing)
+	// We update to initializing again; our StatusChangeTime remains the same.
+	dockerRuntimeHealthcheck.SetHealthcheckStatus(ecstcs.InstanceHealthCheckStatusInitializing)
 	updateChangeTime := dockerRuntimeHealthcheck.GetStatusChangeTime()
-	assert.Equal(t, doctor.HealthcheckStatusInitializing, dockerRuntimeHealthcheck.Status)
+	assert.Equal(t, ecstcs.InstanceHealthCheckStatusInitializing, dockerRuntimeHealthcheck.Status)
 	assert.Equal(t, initializationChangeTime, updateChangeTime)
 
-	// add a sleep so we know time has elapsed between the initial status and status change time
+	// Add a sleep so we know time has elapsed between the initial status and status change time.
 	time.Sleep(1 * time.Millisecond)
 
-	// change status.  This should change the update time too
-	dockerRuntimeHealthcheck.SetHealthcheckStatus(doctor.HealthcheckStatusOk)
-	assert.Equal(t, doctor.HealthcheckStatusOk, dockerRuntimeHealthcheck.Status)
+	// Change status. This should change the update time too.
+	dockerRuntimeHealthcheck.SetHealthcheckStatus(ecstcs.InstanceHealthCheckStatusOk)
+	assert.Equal(t, ecstcs.InstanceHealthCheckStatusOk, dockerRuntimeHealthcheck.Status)
 	okChangeTime := dockerRuntimeHealthcheck.GetStatusChangeTime()
-	// have we updated our change time?
+	// Have we updated our change time?
 	assert.True(t, okChangeTime.After(initializationChangeTime))
 }

agent/doctor/ebs_csi_runtime_healthcheck.go

@@ -18,53 +18,55 @@ import (
 
 	"github.com/aws/amazon-ecs-agent/agent/doctor/statustracker"
 	"github.com/aws/amazon-ecs-agent/ecs-agent/csiclient"
-	"github.com/aws/amazon-ecs-agent/ecs-agent/doctor"
+	ecsdoctor "github.com/aws/amazon-ecs-agent/ecs-agent/doctor"
 	"github.com/aws/amazon-ecs-agent/ecs-agent/logger"
 	"github.com/aws/amazon-ecs-agent/ecs-agent/logger/field"
+	"github.com/aws/amazon-ecs-agent/ecs-agent/tcs/model/ecstcs"
 )
 
 const (
-	// Default request timeout for EBS CSI Daemon health check requests
+	// DefaultEBSHealthRequestTimeout is the default request timeout for EBS CSI Daemon health check requests.
 	DefaultEBSHealthRequestTimeout = 2 * time.Second
 )
 
-// Health check for EBS CSI Daemon.
+// ebsCSIDaemonHealthcheck is a health check for EBS CSI Daemon.
 type ebsCSIDaemonHealthcheck struct {
 	csiClient      csiclient.CSIClient
 	requestTimeout time.Duration
 	*statustracker.HealthCheckStatusTracker
 }
 
-// Constructor for EBS CSI Daemon Health Check
+// NewEBSCSIDaemonHealthCheck is the constructor for EBS CSI Daemon Health Check.
 func NewEBSCSIDaemonHealthCheck(
 	csiClient csiclient.CSIClient,
-	requestTimeout time.Duration, // timeout for health check requests
-) doctor.Healthcheck {
+	requestTimeout time.Duration, // Timeout for health check requests.
+) ecsdoctor.Healthcheck {
 	return &ebsCSIDaemonHealthcheck{
 		csiClient:                csiClient,
 		requestTimeout:           requestTimeout,
 		HealthCheckStatusTracker: statustracker.NewHealthCheckStatusTracker(),
 	}
 }
 
-// Performs a health check for EBS CSI Daemon by sending a request to it to get
-// node capabilities. If EBS CSI Daemon is not started yet then returns OK trivially.
-func (e *ebsCSIDaemonHealthcheck) RunCheck() doctor.HealthcheckStatus {
+// RunCheck performs a health check for EBS CSI Daemon by sending a request to it to get node capabilities.
+// If EBS CSI Daemon is not started yet then returns OK trivially.
+func (e *ebsCSIDaemonHealthcheck) RunCheck() ecstcs.InstanceHealthCheckStatus {
 	ctx, cancel := context.WithTimeout(context.Background(), e.requestTimeout)
 	defer cancel()
 
 	resp, err := e.csiClient.NodeGetCapabilities(ctx)
 	if err != nil {
 		logger.Error("EBS CSI Daemon health check failed", logger.Fields{field.Error: err})
-		e.SetHealthcheckStatus(doctor.HealthcheckStatusImpaired)
+		e.SetHealthcheckStatus(ecstcs.InstanceHealthCheckStatusImpaired)
 		return e.GetHealthcheckStatus()
 	}
 
 	logger.Info("EBS CSI Driver is healthy", logger.Fields{"nodeCapabilities": resp})
-	e.SetHealthcheckStatus(doctor.HealthcheckStatusOk)
+	e.SetHealthcheckStatus(ecstcs.InstanceHealthCheckStatusOk)
 	return e.GetHealthcheckStatus()
 }
 
+// GetHealthcheckType returns the type of this health check.
 func (e *ebsCSIDaemonHealthcheck) GetHealthcheckType() string {
-	return doctor.HealthcheckTypeEBSDaemon
+	return ecstcs.InstanceHealthCheckTypeEBSDaemon
 }

agent/doctor/ebs_csi_runtime_healthcheck_test.go

@@ -20,55 +20,55 @@ import (
 	"testing"
 
 	mock_csiclient "github.com/aws/amazon-ecs-agent/ecs-agent/csiclient/mocks"
-	"github.com/aws/amazon-ecs-agent/ecs-agent/doctor"
+	"github.com/aws/amazon-ecs-agent/ecs-agent/tcs/model/ecstcs"
 	"github.com/container-storage-interface/spec/lib/go/csi"
 	"github.com/golang/mock/gomock"
 	"github.com/stretchr/testify/assert"
 )
 
-// Tests that EBS Daemon Health Check is of the right health check type
+// Tests that EBS Daemon Health Check is of the right health check type.
 func TestEBSGetHealthcheckType(t *testing.T) {
 	ctrl := gomock.NewController(t)
 	defer ctrl.Finish()
 
 	csiClient := mock_csiclient.NewMockCSIClient(ctrl)
 	hc := NewEBSCSIDaemonHealthCheck(csiClient, 0)
 
-	assert.Equal(t, doctor.HealthcheckTypeEBSDaemon, hc.GetHealthcheckType())
+	assert.Equal(t, ecstcs.InstanceHealthCheckTypeEBSDaemon, hc.GetHealthcheckType())
 }
 
-// Tests initial health status of EBS Daemon
+// Tests initial health status of EBS Daemon.
 func TestEBSInitialHealth(t *testing.T) {
 	ctrl := gomock.NewController(t)
 	defer ctrl.Finish()
 
 	csiClient := mock_csiclient.NewMockCSIClient(ctrl)
 	hc := NewEBSCSIDaemonHealthCheck(csiClient, 0)
 
-	assert.Equal(t, doctor.HealthcheckStatusInitializing, hc.GetHealthcheckStatus())
+	assert.Equal(t, ecstcs.InstanceHealthCheckStatusInitializing, hc.GetHealthcheckStatus())
 }
 
-// Tests RunCheck method of EBS Daemon Health Check
+// Tests RunCheck method of EBS Daemon Health Check.
 func TestEBSRunHealthCheck(t *testing.T) {
 	tcs := []struct {
 		name                     string
 		setCSIClientExpectations func(csiClient *mock_csiclient.MockCSIClient)
-		expectedStatus           doctor.HealthcheckStatus
+		expectedStatus           ecstcs.InstanceHealthCheckStatus
 	}{
 		{
 			name: "OK when healthcheck succeeds",
 			setCSIClientExpectations: func(csiClient *mock_csiclient.MockCSIClient) {
 				csiClient.EXPECT().NodeGetCapabilities(gomock.Any()).
 					Return(&csi.NodeGetCapabilitiesResponse{}, nil)
 			},
-			expectedStatus: doctor.HealthcheckStatusOk,
+			expectedStatus: ecstcs.InstanceHealthCheckStatusOk,
 		},
 		{
 			name: "IMPAIRED when healthcheck fails",
 			setCSIClientExpectations: func(csiClient *mock_csiclient.MockCSIClient) {
 				csiClient.EXPECT().NodeGetCapabilities(gomock.Any()).Return(nil, errors.New("err"))
 			},
-			expectedStatus: doctor.HealthcheckStatusImpaired,
+			expectedStatus: ecstcs.InstanceHealthCheckStatusImpaired,
 		},
 	}