Fix test after enabled metrics on windows

Author: Peng Yin

agent/app/agent_windows_test.go

@@ -21,74 +21,14 @@ import (
 	"testing"
 	"time"
 
-	app_mocks "github.com/aws/amazon-ecs-agent/agent/app/mocks"
 	"github.com/aws/amazon-ecs-agent/agent/engine"
-	"github.com/aws/amazon-ecs-agent/agent/eventstream"
 	"github.com/aws/amazon-ecs-agent/agent/sighandlers"
-	"github.com/aws/amazon-ecs-agent/agent/statemanager"
 	statemanager_mocks "github.com/aws/amazon-ecs-agent/agent/statemanager/mocks"
-	"github.com/aws/aws-sdk-go/aws/credentials"
 	"github.com/golang/mock/gomock"
 	"github.com/stretchr/testify/assert"
 	"golang.org/x/sys/windows/svc"
 )
 
-// TestDoStartHappyPath tests the doStart method for windows. This method should
-// go away when we support metrics for windows containers
-func TestDoStartHappyPath(t *testing.T) {
-	ctrl, credentialsManager, state, imageManager, client,
-		dockerClient, _, _ := setup(t)
-	defer ctrl.Finish()
-
-	mockCredentialsProvider := app_mocks.NewMockProvider(ctrl)
-
-	var discoverEndpointsInvoked sync.WaitGroup
-	discoverEndpointsInvoked.Add(1)
-	containerChangeEvents := make(chan engine.DockerContainerChangeEvent)
-
-	// These calls are expected to happen, but cannot be ordered as they are
-	// invoked via go routines, which will lead to occasional test failues
-	dockerClient.EXPECT().Version().AnyTimes()
-	imageManager.EXPECT().StartImageCleanupProcess(gomock.Any()).MaxTimes(1)
-	mockCredentialsProvider.EXPECT().IsExpired().Return(false).AnyTimes()
-	client.EXPECT().DiscoverPollEndpoint(gomock.Any()).Do(func(x interface{}) {
-		// Ensures that the test waits until acs session has bee started
-		discoverEndpointsInvoked.Done()
-	}).Return("poll-endpoint", nil)
-	client.EXPECT().DiscoverPollEndpoint(gomock.Any()).Return("acs-endpoint", nil).AnyTimes()
-
-	gomock.InOrder(
-		mockCredentialsProvider.EXPECT().Retrieve().Return(credentials.Value{}, nil),
-		dockerClient.EXPECT().SupportedVersions().Return(nil),
-		dockerClient.EXPECT().KnownVersions().Return(nil),
-		client.EXPECT().RegisterContainerInstance(gomock.Any(), gomock.Any()).Return("arn", nil),
-		imageManager.EXPECT().SetSaver(gomock.Any()),
-		dockerClient.EXPECT().ContainerEvents(gomock.Any()).Return(containerChangeEvents, nil),
-		state.EXPECT().AllImageStates().Return(nil),
-		state.EXPECT().AllTasks().Return(nil),
-	)
-
-	cfg := getTestConfig()
-	ctx, cancel := context.WithCancel(context.TODO())
-	// Cancel the context to cancel async routines
-	defer cancel()
-	agent := &ecsAgent{
-		ctx:                ctx,
-		cfg:                &cfg,
-		credentialProvider: credentials.NewCredentials(mockCredentialsProvider),
-		dockerClient:       dockerClient,
-		terminationHandler: func(saver statemanager.Saver, taskEngine engine.TaskEngine) {},
-	}
-
-	go agent.doStart(eventstream.NewEventStream("events", ctx),
-		credentialsManager, state, imageManager, client)
-
-	// Wait for both DiscoverPollEndpointInput and DiscoverTelemetryEndpoint to be
-	// invoked. These are used as proxies to indicate that acs and tcs handlers'
-	// NewSession call has been invoked
-	discoverEndpointsInvoked.Wait()
-}
-
 type mockAgent struct {
 	startFunc          func() int
 	terminationHandler sighandlers.TerminationHandler

agent/config/config_windows_test.go

@@ -34,7 +34,7 @@ func TestConfigDefault(t *testing.T) {
 
 	assert.Equal(t, "npipe:////./pipe/docker_engine", cfg.DockerEndpoint, "Default docker endpoint set incorrectly")
 	assert.Equal(t, `C:\ProgramData\Amazon\ECS\data`, cfg.DataDir, "Default datadir set incorrectly")
-	assert.True(t, cfg.DisableMetrics, "Default disablemetrics set incorrectly")
+	assert.False(t, cfg.DisableMetrics, "Default disablemetrics set incorrectly")
 	assert.Equal(t, 10, len(cfg.ReservedPorts), "Default reserved ports set incorrectly")
 	assert.Equal(t, uint16(0), cfg.ReservedMemory, "Default reserved memory set incorrectly")
 	assert.Equal(t, 30*time.Second, cfg.DockerStopTimeout, "Default docker stop container timeout set incorrectly")

agent/stats/container_test.go

@@ -61,14 +61,14 @@ func TestContainerStatsCollection(t *testing.T) {
 			// deal with the docker.Stats.MemoryStats inner struct
 			jsonStat := fmt.Sprintf(`
 				{
-					"memory_stats": {"usage":%d},
+					"memory_stats": {"usage":%d, "privateworkingset":%d},
 					"cpu_stats":{
 						"cpu_usage":{
 							"percpu_usage":[%d],
 							"total_usage":%d
 						}
 					}
-				}`, stat.memBytes, stat.cpuTime, stat.cpuTime)
+				}`, stat.memBytes, stat.memBytes, stat.cpuTime, stat.cpuTime)
 			dockerStat := &docker.Stats{}
 			json.Unmarshal([]byte(jsonStat), dockerStat)
 			dockerStat.Read = stat.timestamp

agent/stats/utils_test.go

@@ -66,25 +66,6 @@ func TestDockerStatsToContainerStatsCpuUsage(t *testing.T) {
 	}
 }
 
-func TestDockerStatsToContainerStatsZeroCoresGeneratesError(t *testing.T) {
-	// doing this with json makes me sad, but is the easiest way to deal with
-	// the inner structs
-	jsonStat := fmt.Sprintf(`
-		{
-			"cpu_stats":{
-				"cpu_usage":{
-					"total_usage":%d
-				}
-			}
-		}`, 100)
-	dockerStat := &docker.Stats{}
-	json.Unmarshal([]byte(jsonStat), dockerStat)
-	_, err := dockerStatsToContainerStats(dockerStat)
-	if err == nil {
-		t.Error("Expected error converting container stats with empty PercpuUsage")
-	}
-}
-
 func TestDockerStatsToContainerStatsMemUsage(t *testing.T) {
 	jsonStat := fmt.Sprintf(`
 		{
@@ -100,9 +81,10 @@ func TestDockerStatsToContainerStatsMemUsage(t *testing.T) {
 				"stats": {
 					"cache": %d,
 					"rss": %d
-				}
+				},
+				"privateworkingset": %d
 			}
-		}`, 1, 2, 3, 4, 100, 30, 100, 20, 10)
+		}`, 1, 2, 3, 4, 100, 30, 100, 20, 10, 10)
 	dockerStat := &docker.Stats{}
 	json.Unmarshal([]byte(jsonStat), dockerStat)
 	containerStats, err := dockerStatsToContainerStats(dockerStat)

agent/stats/utils_unix.go

@@ -24,9 +24,9 @@ import (
 // dockerStatsToContainerStats returns a new object of the ContainerStats object from docker stats.
 func dockerStatsToContainerStats(dockerStats *docker.Stats) (*ContainerStats, error) {
 	// The length of PercpuUsage represents the number of cores in an instance.
-	if len(dockerStats.CPUStats.CPUUsage.PercpuUsage) == 0 {
-		seelog.Debug("Invalid container statistics reported, invalid stats payload from docker")
-		return nil, fmt.Errorf("Invalid container statistics reported")
+	if len(dockerStats.CPUStats.CPUUsage.PercpuUsage) == 0 || numCores == uint64(0) {
+		seelog.Debug("Invalid container statistics reported, no cpu core usage reported")
+		return nil, fmt.Errorf("Invalid container statistics reported, no cpu core usage reported")
 	}
 
 	cpuUsage := dockerStats.CPUStats.CPUUsage.TotalUsage / numCores

agent/stats/utils_unix_test.go

@@ -0,0 +1,42 @@
+// +build !windows,!integration
+
+// Copyright 2014-2016 Amazon.com, Inc. or its affiliates. All Rights Reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License"). You may
+// not use this file except in compliance with the License. A copy of the
+// License is located at
+//
+//	http://aws.amazon.com/apache2.0/
+//
+// or in the "license" file accompanying this file. This file is distributed
+// on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
+// express or implied. See the License for the specific language governing
+// permissions and limitations under the License.
+package stats
+
+import (
+	"encoding/json"
+	"fmt"
+	"testing"
+
+	docker "github.com/fsouza/go-dockerclient"
+)
+
+func TestDockerStatsToContainerStatsZeroCoresGeneratesError(t *testing.T) {
+	// doing this with json makes me sad, but is the easiest way to deal with
+	// the inner structs
+	jsonStat := fmt.Sprintf(`
+		{
+			"cpu_stats":{
+				"cpu_usage":{
+					"total_usage":%d
+				}
+			}
+		}`, 100)
+	dockerStat := &docker.Stats{}
+	json.Unmarshal([]byte(jsonStat), dockerStat)
+	_, err := dockerStatsToContainerStats(dockerStat)
+	if err == nil {
+		t.Error("Expected error converting container stats with empty PercpuUsage")
+	}
+}

agent/stats/utils_windows.go

@@ -24,8 +24,8 @@ import (
 // dockerStatsToContainerStats returns a new object of the ContainerStats object from docker stats.
 func dockerStatsToContainerStats(dockerStats *docker.Stats) (*ContainerStats, error) {
 	if numCores == uint64(0) {
-		seelog.Error("Invalid number of cpu cores")
-		return nil, fmt.Errorf("Invalid number of cpu cores")
+		seelog.Error("Invalid number of cpu cores acquired from the system")
+		return nil, fmt.Errorf("invalid number of cpu cores acquired from the system")
 	}
 
 	cpuUsage := dockerStats.CPUStats.CPUUsage.TotalUsage / numCores

agent/stats/utils_windows_test.go

@@ -0,0 +1,41 @@
+// +build windows,!integration
+
+// Copyright 2017 Amazon.com, Inc. or its affiliates. All Rights Reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License"). You may
+// not use this file except in compliance with the License. A copy of the
+// License is located at
+//
+//	http://aws.amazon.com/apache2.0/
+//
+// or in the "license" file accompanying this file. This file is distributed
+// on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
+// express or implied. See the License for the specific language governing
+// permissions and limitations under the License.
+package stats
+
+import (
+	"encoding/json"
+	"fmt"
+	"testing"
+
+	docker "github.com/fsouza/go-dockerclient"
+)
+
+func TestDockerStatsToContainerStatsZeroCoresGeneratesError(t *testing.T) {
+	numCores = uint64(0)
+	jsonStat := fmt.Sprintf(`
+		{
+			"cpu_stats":{
+				"cpu_usage":{
+					"total_usage":%d
+				}
+			}
+		}`, 100)
+	dockerStat := &docker.Stats{}
+	json.Unmarshal([]byte(jsonStat), dockerStat)
+	_, err := dockerStatsToContainerStats(dockerStat)
+	if err == nil {
+		t.Error("Expected error converting container stats with zero cpu cores")
+	}
+}