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

Filter out invalid IPv6 port bindings from TestDynamicPortForward and TestMultipleDynamicPortForward #3043

Merged
merged 1 commit into from
Sep 28, 2021

Conversation

chienhanlin
Copy link
Contributor

@chienhanlin chienhanlin commented Sep 27, 2021

Summary

This PR filters out invalid IPv 6 port binding to fix failed TestDynamicPortForward and TestMultipleDynamicPortForward test when Docker version >= 20.10.6.

The error log is provided as follows.

1632523820118261648 [Debug] logger=structured msg="Sending task change event" taskARN="testDynamicPortForward" status="RUNNING" sentStatus="NONE" event="testDynamicPortForward -> RUNNING, Known Sent: NONE, PullStartedAt: 0001-01-01 00:00:00 +0000 UTC, PullStoppedAt: 0001-01-01 00:00:00 +0000 UTC, ExecutionStoppedAt: 0001-01-01 00:00:00 +0000 UTC"
    engine_unix_integ_test.go:377: PortBindings was not set; should have been len 1 [{24690 49153 0.0.0.0 0} {24690 49153 :: 0}]

The related PR #3025 was merged and released in Agent version 1.55.3.

Implementation details

  1. Filter out invalid IPv6 port binding when binding.BindIP == "::" as the environment variable ECS_EXCLUDE_IPV6_PORTBINDING is true by default.
  2. Add comments on getValidPortBinding() function to explain why IPv6 port bindings are filtering it out in the test code.

Testing

Ran ECS_LOGLEVEL=debug /usr/local/go/bin/go test -count=1 -v -cover -race -tags integration -timeout=30m ./agent/... on an EC2 instance, launched by Amazon ECS-optimized Amazon Linux 2 AMI (amzn2-ami-ecs-hvm-2.0.20210916-x86_64-ebs), which has Docker version 20.10.7 installed.

New tests cover the changes: no

Description for the changelog

N/A

Licensing

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

@chienhanlin chienhanlin changed the title [WIP] Filter out invalid IPv6 port bindings from TestDynamicPortForward and TestMultipleDynamicPortForward Filter out invalid IPv6 port bindings from TestDynamicPortForward and TestMultipleDynamicPortForward Sep 27, 2021
ubhattacharjya
ubhattacharjya previously approved these changes Sep 27, 2021
@chienhanlin chienhanlin marked this pull request as ready for review September 27, 2021 18:44
@@ -370,14 +383,15 @@ func TestDynamicPortForward(t *testing.T) {
require.Equal(t, event.(api.ContainerStateChange).Status, apicontainerstatus.ContainerRunning, "Expected container to be RUNNING")

portBindings := event.(api.ContainerStateChange).PortBindings
validPortBindings := getValidPortBinding(portBindings)
Copy link
Contributor

Choose a reason for hiding this comment

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

discussed offline, we will add a comment to mention why we are filtering it out in the test code

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will add comments to explain the fix in next revision. Thanks!

@chienhanlin chienhanlin merged commit 714d22f into aws:dev Sep 28, 2021
@chienhanlin chienhanlin deleted the fixIntegPortbinding branch September 28, 2021 16:51
@fierlion fierlion added this to the 1.55.4 milestone Sep 29, 2021
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.

5 participants