diff --git a/agent/app/agent.go b/agent/app/agent.go index 77ae11d8f12..420b31b25a5 100644 --- a/agent/app/agent.go +++ b/agent/app/agent.go @@ -395,7 +395,11 @@ func (agent *ecsAgent) doStart(containerChangeEventStream *eventstream.EventStre } return exitcodes.ExitTerminal } - agent.serviceconnectManager.SetECSClient(client, agent.containerInstanceARN) + scManager := agent.serviceconnectManager + scManager.SetECSClient(client, agent.containerInstanceARN) + if loaded, _ := scManager.IsLoaded(agent.dockerClient); loaded { + imageManager.AddImageToCleanUpExclusionList(agent.serviceconnectManager.GetLoadedImageName()) + } // Add container instance ARN to metadata manager if agent.cfg.ContainerMetadataEnabled.Enabled() { diff --git a/agent/app/agent_test.go b/agent/app/agent_test.go index 3f1ba8eeec7..85764bad29c 100644 --- a/agent/app/agent_test.go +++ b/agent/app/agent_test.go @@ -424,6 +424,8 @@ func testDoStartHappyPathWithConditions(t *testing.T, blackholed bool, warmPools mockServiceConnectManager.EXPECT().GetCapabilitiesForAppnetInterfaceVersion("").AnyTimes() mockServiceConnectManager.EXPECT().SetECSClient(gomock.Any(), gomock.Any()).AnyTimes() mockServiceConnectManager.EXPECT().GetAppnetContainerTarballDir().AnyTimes() + mockServiceConnectManager.EXPECT().GetLoadedImageName().Return("service_connect_agent:v1").AnyTimes() + imageManager.EXPECT().AddImageToCleanUpExclusionList(gomock.Eq("service_connect_agent:v1")).Times(1) imageManager.EXPECT().StartImageCleanupProcess(gomock.Any()).MaxTimes(1) dockerClient.EXPECT().ListContainers(gomock.Any(), gomock.Any(), gomock.Any()).Return( dockerapi.ListContainersResponse{}).AnyTimes() diff --git a/agent/app/agent_unix_test.go b/agent/app/agent_unix_test.go index 444f0c8a59d..32149e574cf 100644 --- a/agent/app/agent_unix_test.go +++ b/agent/app/agent_unix_test.go @@ -113,6 +113,8 @@ func TestDoStartTaskENIHappyPath(t *testing.T) { mockServiceConnectManager.EXPECT().GetCapabilitiesForAppnetInterfaceVersion("").AnyTimes() mockServiceConnectManager.EXPECT().SetECSClient(gomock.Any(), gomock.Any()).AnyTimes() mockServiceConnectManager.EXPECT().GetAppnetContainerTarballDir().AnyTimes() + mockServiceConnectManager.EXPECT().GetLoadedImageName().Return("service_connect_agent:v1").AnyTimes() + imageManager.EXPECT().AddImageToCleanUpExclusionList(gomock.Eq("service_connect_agent:v1")).Times(1) mockUdevMonitor.EXPECT().Monitor(gomock.Any()).Return(monitoShutdownEvents).AnyTimes() gomock.InOrder( @@ -456,6 +458,8 @@ func TestDoStartCgroupInitHappyPath(t *testing.T) { mockServiceConnectManager.EXPECT().GetCapabilitiesForAppnetInterfaceVersion("").AnyTimes() mockServiceConnectManager.EXPECT().SetECSClient(gomock.Any(), gomock.Any()).AnyTimes() mockServiceConnectManager.EXPECT().GetAppnetContainerTarballDir().AnyTimes() + mockServiceConnectManager.EXPECT().GetLoadedImageName().Return("service_connect_agent:v1").AnyTimes() + imageManager.EXPECT().AddImageToCleanUpExclusionList(gomock.Eq("service_connect_agent:v1")).Times(1) gomock.InOrder( mockControl.EXPECT().Init().Return(nil), @@ -618,6 +622,8 @@ func TestDoStartGPUManagerHappyPath(t *testing.T) { mockServiceConnectManager.EXPECT().GetCapabilitiesForAppnetInterfaceVersion("").AnyTimes() mockServiceConnectManager.EXPECT().SetECSClient(gomock.Any(), gomock.Any()).AnyTimes() mockServiceConnectManager.EXPECT().GetAppnetContainerTarballDir().AnyTimes() + mockServiceConnectManager.EXPECT().GetLoadedImageName().Return("service_connect_agent:v1").AnyTimes() + imageManager.EXPECT().AddImageToCleanUpExclusionList(gomock.Eq("service_connect_agent:v1")).Times(1) gomock.InOrder( mockGPUManager.EXPECT().Initialize().Return(nil), diff --git a/agent/engine/docker_image_manager.go b/agent/engine/docker_image_manager.go index 6c2539b6c51..263d0cc46d9 100644 --- a/agent/engine/docker_image_manager.go +++ b/agent/engine/docker_image_manager.go @@ -49,6 +49,7 @@ type ImageManager interface { GetImageStateFromImageName(containerImageName string) (*image.ImageState, bool) StartImageCleanupProcess(ctx context.Context) SetDataClient(dataClient data.Client) + AddImageToCleanUpExclusionList(image string) } // dockerImageManager accounts all the images and their states in the instance. @@ -112,6 +113,13 @@ func buildImageCleanupExclusionList(cfg *config.Config) []string { return excludedImages } +func (imageManager *dockerImageManager) AddImageToCleanUpExclusionList(image string) { + imageManager.imageCleanupExclusionList = append(imageManager.imageCleanupExclusionList, image) + logger.Info("Image excluded from cleanup", logger.Fields{ + field.Image: image, + }) +} + func (imageManager *dockerImageManager) AddAllImageStates(imageStates []*image.ImageState) { imageManager.updateLock.Lock() defer imageManager.updateLock.Unlock() diff --git a/agent/engine/mocks/engine_mocks.go b/agent/engine/mocks/engine_mocks.go index 2de73cc6299..976288e2998 100644 --- a/agent/engine/mocks/engine_mocks.go +++ b/agent/engine/mocks/engine_mocks.go @@ -266,6 +266,18 @@ func (mr *MockImageManagerMockRecorder) AddAllImageStates(arg0 interface{}) *gom return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AddAllImageStates", reflect.TypeOf((*MockImageManager)(nil).AddAllImageStates), arg0) } +// AddImageToCleanUpExclusionList mocks base method +func (m *MockImageManager) AddImageToCleanUpExclusionList(arg0 string) { + m.ctrl.T.Helper() + m.ctrl.Call(m, "AddImageToCleanUpExclusionList", arg0) +} + +// AddImageToCleanUpExclusionList indicates an expected call of AddImageToCleanUpExclusionList +func (mr *MockImageManagerMockRecorder) AddImageToCleanUpExclusionList(arg0 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AddImageToCleanUpExclusionList", reflect.TypeOf((*MockImageManager)(nil).AddImageToCleanUpExclusionList), arg0) +} + // GetImageStateFromImageName mocks base method func (m *MockImageManager) GetImageStateFromImageName(arg0 string) (*image.ImageState, bool) { m.ctrl.T.Helper() diff --git a/agent/engine/serviceconnect/manager.go b/agent/engine/serviceconnect/manager.go index 83ef613fad2..ce79e60f875 100644 --- a/agent/engine/serviceconnect/manager.go +++ b/agent/engine/serviceconnect/manager.go @@ -25,7 +25,7 @@ import ( type Manager interface { loader.Loader - GetLoadedImageName() (string, error) + GetLoadedImageName() string AugmentTaskContainer(task *apitask.Task, container *apicontainer.Container, hostConfig *dockercontainer.HostConfig) error CreateInstanceTask(config *config.Config) (*apitask.Task, error) AugmentInstanceContainer(task *apitask.Task, container *apicontainer.Container, hostConfig *dockercontainer.HostConfig) error diff --git a/agent/engine/serviceconnect/manager_linux.go b/agent/engine/serviceconnect/manager_linux.go index b2c4f8f1508..933e5e4ea4c 100644 --- a/agent/engine/serviceconnect/manager_linux.go +++ b/agent/engine/serviceconnect/manager_linux.go @@ -168,7 +168,7 @@ func (m *manager) augmentAgentContainer(task *apitask.Task, container *apicontai config.DrainRequest = m.adminDrainRequest task.PopulateServiceConnectRuntimeConfig(config) - container.Image, _ = m.GetLoadedImageName() + container.Image = m.GetLoadedImageName() return nil } @@ -297,10 +297,7 @@ func (m *manager) AugmentTaskContainer(task *apitask.Task, container *apicontain } func (m *manager) CreateInstanceTask(cfg *config.Config) (*apitask.Task, error) { - imageName, err := m.GetLoadedImageName() - if err != nil { - return nil, err - } + imageName := m.GetLoadedImageName() containerRunning := apicontainerstatus.ContainerRunning dockerHostConfig := dockercontainer.HostConfig{ NetworkMode: apitask.HostNetworkMode, @@ -404,7 +401,7 @@ func (agent *manager) LoadImage(ctx context.Context, _ *config.Config, dockerCli continue } agent.setLoadedAppnetVerion(supportedAppnetInterfaceVersion) - imageName, _ := agent.GetLoadedImageName() + imageName := agent.GetLoadedImageName() logger.Info(fmt.Sprintf("Successfully loaded Appnet agent container tarball: %s", agentContainerTarballPath), logger.Fields{ field.Image: imageName, @@ -415,13 +412,12 @@ func (agent *manager) LoadImage(ctx context.Context, _ *config.Config, dockerCli } func (agent *manager) IsLoaded(dockerClient dockerapi.DockerClient) (bool, error) { - imageName, _ := agent.GetLoadedImageName() - return loader.IsImageLoaded(imageName, dockerClient) + return loader.IsImageLoaded(agent.GetLoadedImageName(), dockerClient) } -func (agent *manager) GetLoadedImageName() (string, error) { +func (agent *manager) GetLoadedImageName() string { agent.agentContainerTag = fmt.Sprintf(defaultAgentContainerTagFormat, agent.appnetInterfaceVersion) - return fmt.Sprintf("%s:%s", agent.agentContainerImageName, agent.agentContainerTag), nil + return fmt.Sprintf("%s:%s", agent.agentContainerImageName, agent.agentContainerTag) } func (agent *manager) GetLoadedAppnetVersion() (string, error) { diff --git a/agent/engine/serviceconnect/manager_other.go b/agent/engine/serviceconnect/manager_other.go index 23b25905a4f..be0b7f118a2 100644 --- a/agent/engine/serviceconnect/manager_other.go +++ b/agent/engine/serviceconnect/manager_other.go @@ -64,10 +64,8 @@ func (*manager) IsLoaded(dockerClient dockerapi.DockerClient) (bool, error) { func (m *manager) SetECSClient(api.ECSClient, string) { } -func (*manager) GetLoadedImageName() (string, error) { - return "", loader.NewUnsupportedPlatformError(fmt.Errorf( - "appnetAgent container get image name: unsupported platform: %s/%s", - runtime.GOOS, runtime.GOARCH)) +func (*manager) GetLoadedImageName() string { + return "" } func (*manager) GetLoadedAppnetVersion() (string, error) { diff --git a/agent/engine/serviceconnect/mock/manager.go b/agent/engine/serviceconnect/mock/manager.go index e79b4a5e652..fa840fa3e9f 100644 --- a/agent/engine/serviceconnect/mock/manager.go +++ b/agent/engine/serviceconnect/mock/manager.go @@ -143,12 +143,11 @@ func (mr *MockManagerMockRecorder) GetLoadedAppnetVersion() *gomock.Call { } // GetLoadedImageName mocks base method -func (m *MockManager) GetLoadedImageName() (string, error) { +func (m *MockManager) GetLoadedImageName() string { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "GetLoadedImageName") ret0, _ := ret[0].(string) - ret1, _ := ret[1].(error) - return ret0, ret1 + return ret0 } // GetLoadedImageName indicates an expected call of GetLoadedImageName diff --git a/agent/stats/service_connect_linux_test.go b/agent/stats/service_connect_linux_test.go index 17e5ca519de..5de58362794 100644 --- a/agent/stats/service_connect_linux_test.go +++ b/agent/stats/service_connect_linux_test.go @@ -121,31 +121,33 @@ func TestRetrieveServiceConnectMetrics(t *testing.T) { } for _, test := range tests { - // Set up a mock http sever on the statsUrlpath - mockUDSPath := "/tmp/appnet_admin.sock" - r := mux.NewRouter() - r.HandleFunc("/get/them/stats", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - fmt.Fprintf(w, "%v", test.stats) - })) + func() { + // Set up a mock http sever on the statsUrlpath + mockUDSPath := "/tmp/appnet_admin.sock" + r := mux.NewRouter() + r.HandleFunc("/get/them/stats", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + fmt.Fprintf(w, "%v", test.stats) + })) - ts := httptest.NewUnstartedServer(r) + ts := httptest.NewUnstartedServer(r) + defer ts.Close() - l, err := net.Listen("unix", mockUDSPath) - assert.NoError(t, err) + l, err := net.Listen("unix", mockUDSPath) + assert.NoError(t, err) - ts.Listener.Close() - ts.Listener = l - ts.Start() + ts.Listener.Close() + ts.Listener = l + ts.Start() - serviceConnectStats := &ServiceConnectStats{ - appnetClient: appnet.Client(), - } - serviceConnectStats.retrieveServiceConnectStats(t1) + serviceConnectStats := &ServiceConnectStats{ + appnetClient: appnet.Client(), + } + serviceConnectStats.retrieveServiceConnectStats(t1) - sortMetrics(serviceConnectStats.GetStats()) - sortMetrics(test.expectedStats) - assert.Equal(t, test.expectedStats, serviceConnectStats.GetStats()) - ts.Close() + sortMetrics(serviceConnectStats.GetStats()) + sortMetrics(test.expectedStats) + assert.Equal(t, test.expectedStats, serviceConnectStats.GetStats()) + }() } }