Improve the error message when reserved memory is larger than the available memory

agent/api/ecsclient/client.go

@@ -15,6 +15,7 @@ package ecsclient
 
 import (
 	"errors"
+	"fmt"
 	"runtime"
 	"strings"
 	"time"
@@ -172,7 +173,12 @@ func (client *APIECSClient) registerContainerInstance(clusterRef string, contain
 	integerStr := "INTEGER"
 
 	cpu, mem := getCpuAndMemory()
-	mem = mem - int64(client.config.ReservedMemory)
+	remainingMem := mem - int64(client.config.ReservedMemory)
+	if remainingMem < 0 {
+		return "", fmt.Errorf(
+			"api register-container-instance: reserved memory is higher than available memory on the host, total memory: %d, reserved: %d",
+			mem, client.config.ReservedMemory)
+	}
 
 	cpuResource := ecs.Resource{
 		Name:         utils.Strptr("CPU"),
@@ -182,7 +188,7 @@ func (client *APIECSClient) registerContainerInstance(clusterRef string, contain
 	memResource := ecs.Resource{
 		Name:         utils.Strptr("MEMORY"),
 		Type:         &integerStr,
-		IntegerValue: &mem,
+		IntegerValue: &remainingMem,
 	}
 	portResource := ecs.Resource{
 		Name:           utils.Strptr("PORTS"),

agent/api/ecsclient/client_test.go

@@ -349,6 +349,30 @@ func TestRegisterContainerInstance(t *testing.T) {
 	assert.Equal(t, "registerArn", arn)
 }
 
+// TestRegisterContainerInstanceWithNegativeResource tests the registeration should fail with negative resource
+func TestRegisterContainerInstanceWithNegativeResource(t *testing.T) {
+	mockCtrl := gomock.NewController(t)
+	defer mockCtrl.Finish()
+
+	_, mem := getCpuAndMemory()
+	mockEC2Metadata := mock_ec2.NewMockEC2MetadataClient(mockCtrl)
+	client := NewECSClient(credentials.AnonymousCredentials,
+		&config.Config{Cluster: configuredCluster,
+			AWSRegion:      "us-east-1",
+			ReservedMemory: uint16(mem) + 1,
+		}, mockEC2Metadata)
+	mockSDK := mock_api.NewMockECSSDK(mockCtrl)
+	mockSubmitStateSDK := mock_api.NewMockECSSubmitStateSDK(mockCtrl)
+	client.(*APIECSClient).SetSDK(mockSDK)
+	client.(*APIECSClient).SetSubmitStateChangeSDK(mockSubmitStateSDK)
+
+	mockEC2Metadata.EXPECT().GetDynamicData(ec2.InstanceIdentityDocumentResource).Return("instanceIdentityDocument", nil)
+	mockEC2Metadata.EXPECT().GetDynamicData(ec2.InstanceIdentityDocumentSignatureResource).Return("signature", nil)
+
+	_, err := client.RegisterContainerInstance("", nil)
+	assert.Error(t, err, "Register resource with negative value should cause registration fail")
+}
+
 func TestValidateRegisteredAttributes(t *testing.T) {
 	origAttributes := []*ecs.Attribute{
 		{Name: aws.String("foo"), Value: aws.String("bar")},

agent/app/agent.go

@@ -341,7 +341,6 @@ func (agent *ecsAgent) registerContainerInstance(
 	// Save our shiny new containerInstanceArn
 	stateManager.Save()
 	return nil
-
 }
 
 // registerContainerInstance registers a container instance that has already been

agent/engine/docker_task_engine_test.go

@@ -605,16 +605,17 @@ func TestStopWithPendingStops(t *testing.T) {
 
 	taskEngine.AddTask(sleepTask2)
 	<-pullInvoked
-	stopSleep2 := *sleepTask2
+	stopSleep2 := testdata.LoadTask("sleep5")
+	stopSleep2.Arn = "arn2"
 	stopSleep2.SetDesiredStatus(api.TaskStopped)
 	stopSleep2.StopSequenceNumber = 4
-	taskEngine.AddTask(&stopSleep2)
+	taskEngine.AddTask(stopSleep2)
 
 	taskEngine.AddTask(sleepTask1)
-	stopSleep1 := *sleepTask1
+	stopSleep1 := testdata.LoadTask("sleep5")
 	stopSleep1.SetDesiredStatus(api.TaskStopped)
 	stopSleep1.StopSequenceNumber = 5
-	taskEngine.AddTask(&stopSleep1)
+	taskEngine.AddTask(stopSleep1)
 	pullDone <- true
 	// this means the PullImage is only called once due to the task is stopped before it
 	// gets the pull image lock
@@ -762,9 +763,9 @@ func TestTaskTransitionWhenStopContainerTimesout(t *testing.T) {
 	assert.Equal(t, event.(api.TaskStateChange).Status, api.TaskRunning, "Expected task to be RUNNING")
 
 	// Set the task desired status to be stopped and StopContainer will be called
-	updateSleepTask := *sleepTask
+	updateSleepTask := testdata.LoadTask("sleep5")
 	updateSleepTask.SetDesiredStatus(api.TaskStopped)
-	go taskEngine.AddTask(&updateSleepTask)
+	go taskEngine.AddTask(updateSleepTask)
 
 	// StopContainer timeout error shouldn't cause cantainer/task status change
 	// until receive stop event from docker event stream
@@ -867,9 +868,9 @@ func TestTaskTransitionWhenStopContainerReturnsUnretriableError(t *testing.T) {
 	eventsReported.Wait()
 
 	// Set the task desired status to be stopped and StopContainer will be called
-	updateSleepTask := *sleepTask
+	updateSleepTask := testdata.LoadTask("sleep5")
 	updateSleepTask.SetDesiredStatus(api.TaskStopped)
-	go taskEngine.AddTask(&updateSleepTask)
+	go taskEngine.AddTask(updateSleepTask)
 
 	// StopContainer was called again and received stop event from docker event stream
 	// Expect it to go to stopped
@@ -944,9 +945,9 @@ func TestTaskTransitionWhenStopContainerReturnsTransientErrorBeforeSucceeding(t
 	}
 
 	// Set the task desired status to be stopped and StopContainer will be called
-	updateSleepTask := *sleepTask
+	updateSleepTask := testdata.LoadTask("sleep5")
 	updateSleepTask.SetDesiredStatus(api.TaskStopped)
-	go taskEngine.AddTask(&updateSleepTask)
+	go taskEngine.AddTask(updateSleepTask)
 
 	// StopContainer invocation should have caused it to stop eventually.
 	event = <-stateChangeEvents