Wire-in IP compatibility detection and override (#4752)

Author: amogh09

README.md

@@ -211,6 +211,7 @@ additional details on each available environment variable.
 | `ECS_IMAGE_PULL_INACTIVITY_TIMEOUT` | 1m | The time to wait after docker pulls complete waiting for extraction of a container. Useful for tuning large Windows containers. | 1m | 3m |
 | `ECS_IMAGE_PULL_TIMEOUT` | 1h | The time to wait for pulling docker image. | 2h | 2h |
 | `ECS_INSTANCE_ATTRIBUTES` | `{"stack": "prod"}` | These attributes take effect only during initial registration. After the agent has joined an ECS cluster, use the PutAttributes API action to add additional attributes. For more information, see [Amazon ECS Container Agent Configuration](http://docs.aws.amazon.com/AmazonECS/latest/developerguide/ecs-agent-config.html) in the Amazon ECS Developer Guide.| `{}` | `{}` |
+| `ECS_INSTANCE_IP_COMPATIBILITY` | `ipv4` | `ipv6` | Option to override Agent's auto detection of instance's IP compatibility. When unset, Agent consults the instance's default IPv4 and IPv6 routes to detect the IP compatibility of the instance. Use this option to override auto detection with the value you provide. For IPv6-only instances, Agent uses dualstack endpoints when making any AWS service calls and sets up features to be IPv6-compatible. For dualstack instances, just use `ipv4`. Neither this option nor the auto detection of IP compatibility are supported on Windows and Agent always works in IPv4-compatible mode. | Automatic detection based on default routes | Not Supported on Windows |
 | `ECS_ENABLE_TASK_ENI` | `false` | Whether to enable task networking for task to be launched with its own network interface | `false` | Not applicable |
 | `ECS_ENABLE_HIGH_DENSITY_ENI` | `false` | Whether to enable high density eni feature when using task networking | `true` | Not applicable |
 | `ECS_CNI_PLUGINS_PATH` | `/ecs/cni` | The path where the cni binary file is located | `/amazon-ecs-cni-plugins` | Not applicable |

agent/config/config.go

@@ -605,9 +605,7 @@ func environmentConfig() (Config, error) {
 		DynamicHostPortRange:                parseDynamicHostPortRange("ECS_DYNAMIC_HOST_PORT_RANGE"),
 		TaskPidsLimit:                       parseTaskPidsLimit(),
 		FirelensAsyncEnabled:                parseBooleanDefaultTrueConfig("ECS_ENABLE_FIRELENS_ASYNC"),
-
-		// TODO:feat:ipv6-only Enable InstanceIPCompatibility parameter when the feature is ready
-		// InstanceIPCompatibility:             parseInstanceIPCompatibility(),
+		InstanceIPCompatibility:             parseInstanceIPCompatibility(),
 	}, err
 }
 

agent/config/config_unix.go

@@ -24,6 +24,7 @@ import (
 	"github.com/aws/amazon-ecs-agent/agent/dockerclient"
 	"github.com/aws/amazon-ecs-agent/agent/utils"
 	"github.com/aws/amazon-ecs-agent/ecs-agent/ipcompatibility"
+	commonutils "github.com/aws/amazon-ecs-agent/ecs-agent/utils"
 	netutils "github.com/aws/amazon-ecs-agent/ecs-agent/utils/net"
 	"github.com/aws/amazon-ecs-agent/ecs-agent/utils/netlinkwrapper"
 
@@ -138,15 +139,13 @@ func DefaultConfig(ipCompatOverride ipcompatibility.IPCompatibility) Config {
 		FirelensAsyncEnabled:                BooleanDefaultTrue{Value: ExplicitlyEnabled},
 	}
 
-	// TODO:feat:ipv6-only Enable IP compatibility detection when the feature is ready
-	// if commonutils.ZeroOrNil(ipCompatOverride) {
-	// 	logger.Info("No IP compatibility override provided, detecting instance IP compatibility for default config")
-	// 	cfg.determineIPCompatibility()
-	// 	cfg.setIPv6PortBindingDefault(cfg.InstanceIPCompatibility)
-	// } else {
-	// 	cfg.setIPv6PortBindingDefault(ipCompatOverride)
-	// }
-	cfg.setIPv6PortBindingDefault(ipCompatOverride)
+	if commonutils.ZeroOrNil(ipCompatOverride) {
+		logger.Info("No IP compatibility override provided, detecting instance IP compatibility for default config")
+		cfg.determineIPCompatibility()
+	} else {
+		cfg.InstanceIPCompatibility = ipCompatOverride
+	}
+	cfg.setIPv6PortBindingDefault(cfg.InstanceIPCompatibility)
 
 	return cfg
 }

agent/config/config_unix_test.go

@@ -386,35 +386,121 @@ func TestShouldExcludeIPv6PortBindingDefault(t *testing.T) {
 	testCases := []struct {
 		name                    string
 		instanceIPCompatibility ipcompatibility.IPCompatibility
-		expectedExcludeIPv6     bool
+		expectedExcludeIPv6     BooleanDefaultTrue
 	}{
 		{
 			name:                    "ipv6-only instance",
 			instanceIPCompatibility: ipcompatibility.NewIPv6OnlyCompatibility(),
-			expectedExcludeIPv6:     false, // IPv6 port bindings should be included for IPv6-only instances
+			expectedExcludeIPv6:     BooleanDefaultTrue{Value: ExplicitlyDisabled},
 		},
 		{
 			name:                    "dual-stack instance",
 			instanceIPCompatibility: ipcompatibility.NewDualStackCompatibility(),
-			expectedExcludeIPv6:     true, // IPv6 port bindings should be excluded by default
+			expectedExcludeIPv6:     BooleanDefaultTrue{Value: ExplicitlyEnabled},
 		},
 		{
 			name:                    "ipv4-only instance",
 			instanceIPCompatibility: ipcompatibility.NewIPv4OnlyCompatibility(),
-			expectedExcludeIPv6:     true, // IPv6 port bindings should be excluded by default
+			expectedExcludeIPv6:     BooleanDefaultTrue{Value: ExplicitlyEnabled},
 		},
 		{
 			name:                    "no ip compatibility",
 			instanceIPCompatibility: ipcompatibility.NewIPCompatibility(false, false),
-			expectedExcludeIPv6:     true, // IPv6 port bindings should be excluded by default
+			expectedExcludeIPv6:     BooleanDefaultTrue{Value: ExplicitlyEnabled},
 		},
 	}
 
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
 			cfg := &Config{}
 			cfg.setIPv6PortBindingDefault(tc.instanceIPCompatibility)
-			assert.Equal(t, tc.expectedExcludeIPv6, cfg.ShouldExcludeIPv6PortBinding.Enabled())
+			assert.Equal(t, tc.expectedExcludeIPv6, cfg.ShouldExcludeIPv6PortBinding)
+		})
+	}
+}
+
+func TestDefaultConfigIPCompatibilityAndExcludeIPv6PortBinding(t *testing.T) {
+	ipv4Gw := net.ParseIP("10.0.0.1")
+	ipv6Gw := net.ParseIP("1:2:3:4::")
+	require.NotNil(t, ipv4Gw)
+	require.NotNil(t, ipv6Gw)
+	ipv4Route := netlink.Route{Gw: ipv4Gw, Dst: nil}
+	ipv6Route := netlink.Route{Gw: ipv6Gw, Dst: nil}
+
+	testCases := []struct {
+		name                     string
+		ipCompatOverride         ipcompatibility.IPCompatibility
+		setNetlinkExpectations   func(*mock_netlinkwrapper.MockNetLink)
+		expectedInstanceIPCompat ipcompatibility.IPCompatibility
+		expectedExcludeIPv6      BooleanDefaultTrue
+	}{
+		{
+			name:                     "Override with IPv4-only",
+			ipCompatOverride:         ipcompatibility.NewIPv4OnlyCompatibility(),
+			expectedInstanceIPCompat: ipcompatibility.NewIPv4OnlyCompatibility(),
+			expectedExcludeIPv6:      BooleanDefaultTrue{Value: ExplicitlyEnabled},
+		},
+		{
+			name:                     "Override with IPv6-only",
+			ipCompatOverride:         ipcompatibility.NewIPv6OnlyCompatibility(),
+			expectedInstanceIPCompat: ipcompatibility.NewIPv6OnlyCompatibility(),
+			expectedExcludeIPv6:      BooleanDefaultTrue{Value: ExplicitlyDisabled},
+		},
+		{
+			name:                     "Override with dual-stack",
+			ipCompatOverride:         ipcompatibility.NewDualStackCompatibility(),
+			expectedInstanceIPCompat: ipcompatibility.NewDualStackCompatibility(),
+			expectedExcludeIPv6:      BooleanDefaultTrue{Value: ExplicitlyEnabled},
+		},
+
+		{
+			name:             "No override - auto-detect IPv4-only",
+			ipCompatOverride: ipcompatibility.IPCompatibility{}, // Zero value triggers auto-detection
+			setNetlinkExpectations: func(nl *mock_netlinkwrapper.MockNetLink) {
+				nl.EXPECT().RouteList(nil, netlink.FAMILY_V4).Return([]netlink.Route{ipv4Route}, nil)
+				nl.EXPECT().RouteList(nil, netlink.FAMILY_V6).Return([]netlink.Route{}, nil)
+			},
+			expectedInstanceIPCompat: ipcompatibility.NewIPv4OnlyCompatibility(),
+			expectedExcludeIPv6:      BooleanDefaultTrue{Value: ExplicitlyEnabled},
+		},
+		{
+			name:             "No override - auto-detect IPv6-only",
+			ipCompatOverride: ipcompatibility.IPCompatibility{}, // Zero value triggers auto-detection
+			setNetlinkExpectations: func(nl *mock_netlinkwrapper.MockNetLink) {
+				nl.EXPECT().RouteList(nil, netlink.FAMILY_V4).Return([]netlink.Route{}, nil)
+				nl.EXPECT().RouteList(nil, netlink.FAMILY_V6).Return([]netlink.Route{ipv6Route}, nil)
+			},
+			expectedInstanceIPCompat: ipcompatibility.NewIPv6OnlyCompatibility(),
+			// IPv6 port bindings included for IPv6-only
+			expectedExcludeIPv6: BooleanDefaultTrue{Value: ExplicitlyDisabled},
+		},
+		{
+			name:             "No override - auto-detect dual-stack",
+			ipCompatOverride: ipcompatibility.IPCompatibility{}, // Zero value triggers auto-detection
+			setNetlinkExpectations: func(nl *mock_netlinkwrapper.MockNetLink) {
+				nl.EXPECT().RouteList(nil, netlink.FAMILY_V4).Return([]netlink.Route{ipv4Route}, nil)
+				nl.EXPECT().RouteList(nil, netlink.FAMILY_V6).Return([]netlink.Route{ipv6Route}, nil)
+			},
+			expectedInstanceIPCompat: ipcompatibility.NewDualStackCompatibility(),
+			expectedExcludeIPv6:      BooleanDefaultTrue{Value: ExplicitlyEnabled},
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			ctrl := gomock.NewController(t)
+			defer ctrl.Finish()
+
+			mockNLWrapper := mock_netlinkwrapper.NewMockNetLink(ctrl)
+			defer setMockNLWrapper(mockNLWrapper)()
+
+			if tc.setNetlinkExpectations != nil {
+				tc.setNetlinkExpectations(mockNLWrapper)
+			}
+
+			cfg := DefaultConfig(tc.ipCompatOverride)
+			assert.Equal(t, tc.expectedInstanceIPCompat, cfg.InstanceIPCompatibility)
+			assert.Equal(t, tc.expectedExcludeIPv6, cfg.ShouldExcludeIPv6PortBinding)
 		})
 	}
 }

agent/config/parse.go

@@ -45,10 +45,6 @@ const (
 	// location of the credentials-fetcher location on the machine
 	envCredentialsFetcherHostDir = "CREDENTIALS_FETCHER_HOST_DIR"
 	// envInstanceIPCompatibility is an environment setting to override IP compatibility detection
-	//
-	// TODO:feat:IPv6-only - Remove lint rule below
-	//
-	//lint:ignore U1000 Constant will be used in the future
 	envInstanceIPCompatibility = "ECS_INSTANCE_IP_COMPATIBILITY"
 )
 

agent/config/parse_linux.go

@@ -158,10 +158,6 @@ func parseTaskPidsLimit() int {
 //   - "ipv4": Returns IPv4-only compatibility
 //   - "ipv6": Returns IPv6-only compatibility
 //   - empty/unset or invalid: Returns zero value
-//
-// TODO:feat:IPv6-only - Remove lint rule below
-//
-//lint:ignore U1000 Function will be used in the future
 func parseInstanceIPCompatibility() ipcompatibility.IPCompatibility {
 	val := strings.TrimSpace(os.Getenv(envInstanceIPCompatibility))
 	if val == "" {

agent/config/parse_windows.go

@@ -202,10 +202,6 @@ func parseTaskPidsLimit() int {
 
 // parseInstanceIPCompatibility always returns zero-value IP compatibility as the config
 // parameter to override instance IP compatibility is not supported on Windows.
-//
-// TODO:feat:IPv6-only - Remove lint rule below
-//
-//lint:ignore U1000 Constant will be used in the future
 func parseInstanceIPCompatibility() ipcompatibility.IPCompatibility {
 	if os.Getenv(envInstanceIPCompatibility) != "" {
 		logger.Warn(envInstanceIPCompatibility + " is not supported on Windows")