Add file watcher for Appnet agent image update #3435

mythri-garaga · 2022-10-17T21:15:32Z

Summary

This PR adds a file watcher for filepath (/var/lib/ecs/deps/serviceconnenct) where ECS Agent loads Appnet agent image from. With these changes, if there any update current Appnet agent image or a new Appnet agent image version becomes available, the file watcher invokes to reload this image and restart the internal instance relay with the updated image.

Update go.mod and vendor files to include fsnotify package.

Implementation details

Testing

make test

Also for manual testing purpose, made changes to Agent to include v2 as supported Appnet version and tested the following scenarios

ECS Agent is running and has ecs-service-connect-agent:interface-v1 appnet image loaded. There are no Service Connect task running on the instance. Placed the Appnet v2 - ecs-service-connect-agent.interface-v2.tar tarball in the service connect deps dir. The ECS Agent agent reloaded the image ecs-service-connect-agent:v2.
Tested scenario 1 with Service Connect task running in bridge mode. Observed that ECS Agent reloaded the new Appnet image and restart the instance relay task with ecs-service-connect-agent:v2
With Scenario 2, that is ECS Agent has reloaded the new Appnet image and restarted the instance relay task with ecs-service-connect-agent:v2 but SC task's Appnet agent is still running with ecs-service-connect-agent:v1 image.
Stopped this SC task and started a new one with same task def. The new SC task started successfully and it's Appnet agent is still running with ecs-service-connect-agent:v2 image.

New tests cover the changes: yes

Description for the changelog

Licensing

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

yinyic

Have we verified (manually) that this works? If so could you include that in the description?

Update - thanks for adding the manual test scenarios!

agent/engine/docker_task_engine_linux.go

agent/engine/docker_task_engine_linux_test.go

yinyic · 2022-10-18T21:41:44Z

agent/engine/docker_task_engine_linux.go

+		seelog.Errorf("error adding %s to fsnotify watcher, error: %v", filepath, err)
+	}
+	err = watcher.Add(filepath)
+	if err != nil {
+		seelog.Errorf("failed to initialize fsnotify NewWatcher, error: %v", err)


Just noticed that the two error messages should probably be swapped :)

Thanks for catching that! just updated:)

ubhattacharjya · 2022-10-18T21:52:01Z

agent/engine/docker_task_engine_linux.go

 	"time"

+	"github.com/cihub/seelog"


Do you want to use "github.com/aws/amazon-ecs-agent/agent/logger" instead of seelog?

yinyic · 2022-10-18T21:57:38Z

agent/engine/docker_task_engine.go

@@ -109,6 +109,8 @@ const (
 	stopContainerBackoffJitter     = 0.2
 	stopContainerBackoffMultiplier = 1.3
 	stopContainerMaxRetryCount     = 5
+
+	serviceConnectAppnetAgenTarballDir = "/var/lib/ecs/deps/serviceconnect/"


It just occurred to me that the /var/lib/ecs/deps/ is the host directory, and is mounted into ECS Agent container as /managed/agents/ (see the bind mount configuration here)

I thought the watcher should be created on the container path (because the exact host path does not exist in Agent container technically), but are we observing expected behaviors even when the watcher is created with the host path?

You are right, watching container path is appropriate here, made changes in the latest rev accordingly.

ubhattacharjya · 2022-10-18T21:58:44Z

agent/engine/docker_task_engine_linux.go

+				return
+			}
+			const writeOrCreateMask = fsnotify.Write | fsnotify.Create
+			if event.Op&writeOrCreateMask != 0 {


should there be a space between event.Op and writeOrCreateMask ? Also, can you explain what this means?

added space but I think gofmt removes it.

Also add a comment to clarify the condition we check:)

Thanks Mythri :)

chienhanlin · 2022-10-19T17:52:26Z

agent/engine/docker_task_engine_linux.go

+func (engine *DockerTaskEngine) watchAppNetImage(ctx context.Context) {
+	watcher, err := fsnotify.NewWatcher()
+	if err != nil {
+		logger.Error(fmt.Sprintf("failed to initialize fsnotify NewWatcher, error: %v", err))


not blocking. we can use logger fields : )

logger.Error("Failed to initialize fsnotify NewWatcher", logger.Fields{ field.Error: err, })

oops...will fix it in the next PR!! Thank you:)

chienhanlin · 2022-10-19T17:56:45Z

agent/engine/docker_task_engine_linux.go

+				engine.stopContainer(engine.serviceconnectRelay, container)
+			}
+		}
+		engine.serviceconnectRelay.SetDesiredStatus(apitaskstatus.TaskStopped)


Q. Would we show the task is stopped/started due to AppNet Agent image reload somewhere in ECS Agent log? or Control plane will receive a message from ECS Agent, saying that task state is changed due to image is reloading?

Since this an internal task, we don't publish this task status to backend but just log the restart on ECS Agent.

chienhanlin · 2022-10-19T17:57:38Z

agent/engine/serviceconnect/manager_linux.go

 		if loadErr = loader.LoadFromFile(ctx, agentContainerTarballPath, dockerClient); loadErr != nil {
-			logger.Warn(fmt.Sprintf("Unable to load appnet agent container tarball: %s", agentContainerTarballPath),
+			logger.Warn(fmt.Sprintf("Unable to load Appnet agent container tarball: %s", agentContainerTarballPath),


mythri-garaga force-pushed the restart-relay branch 6 times, most recently from 4e49369 to 768ef2d Compare October 17, 2022 22:09

mythri-garaga marked this pull request as ready for review October 17, 2022 22:09

mythri-garaga requested a review from a team as a code owner October 17, 2022 22:09

mythri-garaga added the bot/test label Oct 17, 2022

amazon-ecs-bot removed the bot/test label Oct 17, 2022

yinyic reviewed Oct 18, 2022

View reviewed changes

mythri-garaga force-pushed the restart-relay branch 5 times, most recently from 3624222 to 2287f64 Compare October 18, 2022 04:54

mythri-garaga added the bot/test label Oct 18, 2022

amazon-ecs-bot removed the bot/test label Oct 18, 2022

mythri-garaga force-pushed the restart-relay branch from 2287f64 to 2729417 Compare October 18, 2022 17:24

mythri-garaga changed the base branch from feature/service-connect to dev October 18, 2022 17:29

yinyic reviewed Oct 18, 2022

View reviewed changes

mythri-garaga changed the base branch from dev to feature/service-connect October 18, 2022 17:29

mythri-garaga force-pushed the restart-relay branch 2 times, most recently from bfcfd50 to adfbc11 Compare October 18, 2022 19:21

mythri-garaga added the bot/test label Oct 18, 2022

amazon-ecs-bot removed the bot/test label Oct 18, 2022

yinyic force-pushed the feature/service-connect branch from e11391b to 858b2c9 Compare October 18, 2022 20:18

mythri-garaga force-pushed the restart-relay branch from adfbc11 to 0a1fd90 Compare October 18, 2022 20:47

mythri-garaga changed the base branch from feature/service-connect to dev October 18, 2022 20:48

mythri-garaga changed the base branch from dev to feature/service-connect October 18, 2022 20:51

mythri-garaga added the bot/test label Oct 18, 2022

amazon-ecs-bot removed the bot/test label Oct 18, 2022

mythri-garaga changed the base branch from feature/service-connect to dev October 18, 2022 21:08

mythri-garaga force-pushed the restart-relay branch from 0a1fd90 to 739c6f5 Compare October 18, 2022 21:10

mythri-garaga added the bot/test label Oct 18, 2022

amazon-ecs-bot removed the bot/test label Oct 18, 2022

yinyic reviewed Oct 18, 2022

View reviewed changes

mythri-garaga force-pushed the restart-relay branch from 739c6f5 to 178fcdc Compare October 18, 2022 21:49

ubhattacharjya reviewed Oct 18, 2022

View reviewed changes

yinyic reviewed Oct 18, 2022

View reviewed changes

ubhattacharjya reviewed Oct 18, 2022

View reviewed changes

mythri-garaga force-pushed the restart-relay branch 4 times, most recently from d0ca83b to 6996eb9 Compare October 19, 2022 16:38

mythri-garaga requested review from yinyic and ubhattacharjya October 19, 2022 16:44

Add file watcher for Appnet agent image update

df1b7a0

mythri-garaga force-pushed the restart-relay branch from 6996eb9 to df1b7a0 Compare October 19, 2022 17:28

yinyic approved these changes Oct 19, 2022

View reviewed changes

mythri-garaga added the bot/test label Oct 19, 2022

amazon-ecs-bot removed the bot/test label Oct 19, 2022

ubhattacharjya approved these changes Oct 19, 2022

View reviewed changes

chienhanlin approved these changes Oct 19, 2022

View reviewed changes

mythri-garaga merged commit c0c801b into aws:dev Oct 19, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add file watcher for Appnet agent image update #3435

Add file watcher for Appnet agent image update #3435

mythri-garaga commented Oct 17, 2022 •

edited

Loading

yinyic left a comment •

edited

Loading

yinyic Oct 18, 2022

mythri-garaga Oct 18, 2022

ubhattacharjya Oct 18, 2022

yinyic Oct 18, 2022 •

edited

Loading

mythri-garaga Oct 19, 2022

ubhattacharjya Oct 18, 2022

mythri-garaga Oct 19, 2022 •

edited

Loading

ubhattacharjya Oct 19, 2022

chienhanlin Oct 19, 2022

mythri-garaga Oct 19, 2022

chienhanlin Oct 19, 2022

mythri-garaga Oct 19, 2022

chienhanlin Oct 19, 2022

Add file watcher for Appnet agent image update #3435

Add file watcher for Appnet agent image update #3435

Conversation

mythri-garaga commented Oct 17, 2022 • edited Loading

Summary

Implementation details

Testing

Description for the changelog

Licensing

yinyic left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yinyic Oct 18, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mythri-garaga Oct 19, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mythri-garaga commented Oct 17, 2022 •

edited

Loading

yinyic left a comment •

edited

Loading

yinyic Oct 18, 2022 •

edited

Loading

mythri-garaga Oct 19, 2022 •

edited

Loading