Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for AppMesh #3267

Merged
merged 1 commit into from
Jul 15, 2023
Merged

Conversation

arnaldo2792
Copy link
Contributor

@arnaldo2792 arnaldo2792 commented Jul 14, 2023

Issue number:

Closes #1078

Description of changes:

This PR adds support for AppMesh in the ECS variant. The first-party patches to the ECS agent were regenerated, since a patch to remove the AppMesh CNI plugin from the list of supported CNI plugins was dropped. Thus, reviewers can ignore changes in patches 0001, 0003-0005.

In the packaging for the ECS agent, there is a patch to change the location of the log files created by the VPC CNI plugins (among other files). The VPC CNI plugins will ignore the default values for these log files if the environment variable VPC_CNI_LOG_FILE is set. The ECS agent does set this environment variable using the value overridden by the Bottlerocket patch. Thus, I decided to skip the creation of a patch similar to this, since even if the default log file location is changed, it won't have any effect. All CNI plugins will log to the same file in VPC_CNI_LOG_FILE, which is /var/log/ecs/ecs-cni-bridge-plugin.log in Bottlerocket.

I'm planning to do more clean up in the patches that are carried along with some suggestions that were given for the ECS agent spec file. I'll do that after this PR is merged.

Testing done:

  • I confirmed internal testing for AppMesh is passing
  • I followed this tutorial, with the appropiate changes to make it work for Bottlerocket (update the AMI id to be the one that I built, update the user data format), and confirmed that I can call the service that was created:
❯ curl $colorapp/color
{"color":"red", "stats": {"blue":0.21,"red":0.5,"white":0.29}}⏎
❯ curl $colorapp/color
{"color":"red", "stats": {"blue":0.2,"red":0.53,"white":0.27}}⏎
❯ curl $colorapp/color
{"color":"blue", "stats": {"blue":0.25,"red":0.5,"white":0.25}}⏎
❯ curl $colorapp/color
{"color":"white", "stats": {"blue":0.24,"red":0.47,"white":0.29}}⏎
❯ curl $colorapp/color
{"color":"blue", "stats": {"blue":0.28,"red":0.44,"white":0.28}}⏎
❯ curl $colorapp/color
{"color":"red", "stats": {"blue":0.26,"red":0.47,"white":0.26}}⏎
❯ curl $colorapp/color
{"color":"white", "stats": {"blue":0.25,"red":0.45,"white":0.3}}⏎

Terms of contribution:

By submitting this pull request, I agree that this contribution is dual-licensed under the terms of both the Apache License, version 2.0, and the MIT license.

Now, the AppMesh AWS CNI plugin is compiled as part of the ECS agent RPM
spec. As a result, a patch to remove this CNI plugin from the list of
supported plugins was deleted.

Signed-off-by: Arnaldo Garcia Rincon <agarrcia@amazon.com>
Copy link
Contributor

@stmcginnis stmcginnis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

@arnaldo2792 arnaldo2792 merged commit e4cc12a into bottlerocket-os:develop Jul 15, 2023
38 checks passed
@arnaldo2792 arnaldo2792 deleted the app-mesh branch July 20, 2023 22:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[ECS] App Mesh support
4 participants