WIP: When using subnet discovery, you can disable the primary subnet with kubernetes.io/role/cni=0

pkg/awsutils/awsutils.go

@@ -54,14 +54,16 @@ const (
 
 	// AllocENI need to choose a first free device number between 0 and maxENI
 	// 100 is a hard limit because we use vlanID + 100 for pod networking table names
-	maxENIs                 = 100
-	clusterNameEnvVar       = "CLUSTER_NAME"
-	eniNodeTagKey           = "node.k8s.amazonaws.com/instance_id"
-	eniCreatedAtTagKey      = "node.k8s.amazonaws.com/createdAt"
-	eniClusterTagKey        = "cluster.k8s.amazonaws.com/name"
-	additionalEniTagsEnvVar = "ADDITIONAL_ENI_TAGS"
-	reservedTagKeyPrefix    = "k8s.amazonaws.com"
-	subnetDiscoveryTagKey   = "kubernetes.io/role/cni"
+	maxENIs                         = 100
+	clusterNameEnvVar               = "CLUSTER_NAME"
+	eniNodeTagKey                   = "node.k8s.amazonaws.com/instance_id"
+	eniCreatedAtTagKey              = "node.k8s.amazonaws.com/createdAt"
+	eniClusterTagKey                = "cluster.k8s.amazonaws.com/name"
+	additionalEniTagsEnvVar         = "ADDITIONAL_ENI_TAGS"
+	reservedTagKeyPrefix            = "k8s.amazonaws.com"
+	subnetDiscoveryTagKey           = "kubernetes.io/role/cni"
+	subnetDiscoveryTagValueActive   = "1"
+	subnetDiscoveryTagValueInactive = "0"
 	// UnknownInstanceType indicates that the instance type is not yet supported
 	UnknownInstanceType = "vpc ip resource(eni ip limit): unknown instance type"
 
@@ -206,11 +208,12 @@ type EC2InstanceMetadataCache struct {
 	region           string
 	vpcID            string
 
-	unmanagedENIs          StringSet
-	useCustomNetworking    bool
-	multiCardENIs          StringSet
-	useSubnetDiscovery     bool
-	enablePrefixDelegation bool
+	unmanagedENIs              StringSet
+	useCustomNetworking        bool
+	multiCardENIs              StringSet
+	useSubnetDiscovery         bool
+	subnetDiscoverySkipPrimary bool
+	enablePrefixDelegation     bool
 
 	clusterName       string
 	additionalENITags map[string]string
@@ -375,6 +378,8 @@ func New(useSubnetDiscovery, useCustomNetworking, disableLeakedENICleanup, v4Ena
 	log.Infof("Custom networking enabled %v", cache.useCustomNetworking)
 	cache.useSubnetDiscovery = useSubnetDiscovery
 	log.Infof("Subnet discovery enabled %v", cache.useSubnetDiscovery)
+	cache.subnetDiscoverySkipPrimary = true // Populate in nodeInit...
+	log.Infof("Subnet discovery skip primary %v", cache.subnetDiscoverySkipPrimary)
 	cache.v4Enabled = v4Enabled
 	cache.v6Enabled = v6Enabled
 
@@ -466,6 +471,13 @@ func (cache *EC2InstanceMetadataCache) initWithEC2Metadata(ctx context.Context)
 	}
 	log.Debugf("Found vpc-id: %s ", cache.vpcID)
 
+	// Now that we have VPC, we can checking if subnet discovery disable usage of IPs
+	cache.subnetDiscoverySkipPrimary, err = cache.isSubnetDiscoveryDisabledOnSubnet(cache.subnetID)
+	if err != nil {
+		awsAPIErrInc("isSubnetDiscoveryDisabledOnSubnet", err)
+		return err
+	}
+
 	// We use the ctx here for testing, since we spawn go-routines above which will run forever.
 	select {
 	case <-ctx.Done():
@@ -990,7 +1002,7 @@ func (cache *EC2InstanceMetadataCache) getVpcSubnets() ([]*ec2.Subnet, error) {
 
 func validTag(subnet *ec2.Subnet) bool {
 	for _, tag := range subnet.Tags {
-		if *tag.Key == subnetDiscoveryTagKey {
+		if (*tag.Key == subnetDiscoveryTagKey) && (*tag.Value != subnetDiscoveryTagValueInactive) {
 			return true
 		}
 	}
@@ -2127,6 +2139,41 @@ func (cache *EC2InstanceMetadataCache) IsPrimaryENI(eniID string) bool {
 	return false
 }
 
+// Check tags on Subnet and return if it's tagged with subnetDiscoveryTagKey = subnetDiscoveryTagValueInactive
+func (cache *EC2InstanceMetadataCache) isSubnetDiscoveryDisabledOnSubnet(subnetId string) (bool, error) {
+	log.Debugf("Checking Subnet Discovery on %s", subnetId)
+	subnetResult, vpcErr := cache.getVpcSubnets()
+	if vpcErr != nil {
+		log.Warnf("Failed to call ec2:DescribeSubnets: %v", vpcErr)
+		log.Info("Defaulting to use the primary subnet")
+		return false, nil
+	} else {
+		log.Debugf("Got %d subnets", len(subnetResult))
+		for _, subnet := range subnetResult {
+			if *subnet.SubnetId != subnetId {
+				log.Debugf("Not the subnet we are in: %v", *subnet.SubnetId)
+				continue
+			}
+			log.Debugf("Checking tags on subnet %v", *subnet.SubnetId)
+			for _, tag := range subnet.Tags {
+				log.Debugf("Checking on tags %s: %s", *tag.Key, *tag.Value)
+				if *tag.Key == subnetDiscoveryTagKey {
+					if *tag.Value == subnetDiscoveryTagValueInactive {
+						log.Infof("Subnet: %s is tagged with %s, %s so skipPrimary will be enable for SubnetDiscovery", subnetId, subnetDiscoveryTagKey, subnetDiscoveryTagValueInactive)
+						return true, nil
+
+					} else {
+						log.Infof("Subnet: %s is tagged with %s, but not with %s so skipPrimary will be disabled for SubnetDiscovery", subnetId, subnetDiscoveryTagKey, subnetDiscoveryTagValueInactive)
+						return false, nil
+					}
+				}
+
+			}
+		}
+	}
+	return false, nil
+}
+
 func checkAPIErrorAndBroadcastEvent(err error, api string) {
 	if aerr, ok := err.(awserr.Error); ok {
 		if aerr.Code() == "UnauthorizedOperation" {

pkg/awsutils/awsutils_test.go

@@ -2075,3 +2075,90 @@ func Test_loadAdditionalENITags(t *testing.T) {
 		})
 	}
 }
+
+func Test_isSubnetDiscoveryDisableOnSubnet(t *testing.T) {
+	ctrl, mockEC2 := setup(t)
+	defer ctrl.Finish()
+
+	secondSubnetId := "subnet-7b245523"
+	mockMetadata := testMetadata(nil)
+
+	ipAddressCount := int64(100)
+
+	cache := &EC2InstanceMetadataCache{
+		ec2SVC:             mockEC2,
+		imds:               TypedIMDS{mockMetadata},
+		instanceType:       "c5n.18xlarge",
+		useSubnetDiscovery: true,
+	}
+
+	tests := []struct {
+		name         string
+		subnetResult *ec2.DescribeSubnetsOutput
+		param        string
+		want         bool
+	}{
+		{
+			name: "We check a disabled subnet",
+			subnetResult: &ec2.DescribeSubnetsOutput{
+				Subnets: []*ec2.Subnet{{
+					AvailableIpAddressCount: aws.Int64(ipAddressCount),
+					SubnetId:                aws.String(subnetID),
+					Tags: []*ec2.Tag{
+						{
+							Key:   aws.String("kubernetes.io/role/cni"),
+							Value: aws.String("0"),
+						},
+					},
+				},
+					{
+						AvailableIpAddressCount: aws.Int64(ipAddressCount),
+						SubnetId:                aws.String(secondSubnetId),
+						Tags: []*ec2.Tag{
+							{
+								Key:   aws.String("kubernetes.io/role/cni"),
+								Value: aws.String("1"),
+							},
+						},
+					}},
+			},
+			param: subnetID,
+			want:  true,
+		},
+		{
+			name: "We check an enabled subnet",
+			subnetResult: &ec2.DescribeSubnetsOutput{
+				Subnets: []*ec2.Subnet{{
+					AvailableIpAddressCount: aws.Int64(ipAddressCount),
+					SubnetId:                aws.String(subnetID),
+					Tags: []*ec2.Tag{
+						{
+							Key:   aws.String("kubernetes.io/role/cni"),
+							Value: aws.String("0"),
+						},
+					},
+				},
+					{
+						AvailableIpAddressCount: aws.Int64(ipAddressCount),
+						SubnetId:                aws.String(secondSubnetId),
+						Tags: []*ec2.Tag{
+							{
+								Key:   aws.String("kubernetes.io/role/cni"),
+								Value: aws.String("1"),
+							},
+						},
+					}},
+			},
+			param: secondSubnetId,
+			want:  false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			mockEC2.EXPECT().DescribeSubnetsWithContext(gomock.Any(), gomock.Any(), gomock.Any()).Return(tt.subnetResult, nil)
+			got, _ := cache.isSubnetDiscoveryDisabledOnSubnet(tt.param)
+			assert.Equal(t, tt.want, got)
+		})
+	}
+}

pkg/ipamd/ipamd.go

@@ -122,6 +122,9 @@ const (
 	// This environment is used to specify whether we should use enhanced subnet selection or not when creating ENIs (default true).
 	envSubnetDiscovery = "ENABLE_SUBNET_DISCOVERY"
 
+	// This environment is used to specify whether we should skip the primary subnet when tag is applied in the enhanced subnet selection (default true)
+	envSubnetDiscoverySkipPrimary = "SUBNET_DISCOVERY_SKIP_PRIMARY"
+
 	// eniNoManageTagKey is the tag that may be set on an ENI to indicate ipamd
 	// should not manage it in any form.
 	eniNoManageTagKey = "node.k8s.amazonaws.com/no_manage"
@@ -194,20 +197,21 @@ var (
 
 // IPAMContext contains node level control information
 type IPAMContext struct {
-	awsClient                 awsutils.APIs
-	dataStore                 *datastore.DataStore
-	k8sClient                 client.Client
-	enableIPv4                bool
-	enableIPv6                bool
-	useCustomNetworking       bool
-	manageENIsNonScheduleable bool
-	useSubnetDiscovery        bool
-	networkClient             networkutils.NetworkAPIs
-	maxIPsPerENI              int
-	maxENI                    int
-	maxPrefixesPerENI         int
-	unmanagedENI              int
-	numNetworkCards           int
+	awsClient                  awsutils.APIs
+	dataStore                  *datastore.DataStore
+	k8sClient                  client.Client
+	enableIPv4                 bool
+	enableIPv6                 bool
+	useCustomNetworking        bool
+	manageENIsNonScheduleable  bool
+	useSubnetDiscovery         bool
+	subnetDiscoverySkipPrimary bool
+	networkClient              networkutils.NetworkAPIs
+	maxIPsPerENI               int
+	maxENI                     int
+	maxPrefixesPerENI          int
+	unmanagedENI               int
+	numNetworkCards            int
 
 	warmENITarget        int
 	warmIPTarget         int
@@ -339,6 +343,7 @@ func New(k8sClient client.Client) (*IPAMContext, error) {
 	c.useCustomNetworking = UseCustomNetworkCfg()
 	c.manageENIsNonScheduleable = ManageENIsOnNonSchedulableNode()
 	c.useSubnetDiscovery = UseSubnetDiscovery()
+	c.subnetDiscoverySkipPrimary = SubnetDiscoverySkipPrimaryEnable()
 	c.enablePrefixDelegation = usePrefixDelegation()
 	c.enableIPv4 = isIPv4Enabled()
 	c.enableIPv6 = isIPv6Enabled()
@@ -922,7 +927,7 @@ func (c *IPAMContext) tryAssignIPs() (increasedPool bool, err error) {
 	}
 
 	// Find an ENI where we can add more IPs
-	enis := c.dataStore.GetAllocatableENIs(c.maxIPsPerENI, c.useCustomNetworking)
+	enis := c.dataStore.GetAllocatableENIs(c.maxIPsPerENI, c.skipPrimary())
 	for _, eni := range enis {
 		if len(eni.AvailableIPv4Cidrs) < c.maxIPsPerENI {
 			currentNumberOfAllocatedIPs := len(eni.AvailableIPv4Cidrs)
@@ -1014,7 +1019,7 @@ func (c *IPAMContext) tryAssignPrefixes() (increasedPool bool, err error) {
 	toAllocate := c.getPrefixesNeeded()
 	// Returns an ENI which has space for more prefixes to be attached, but this
 	// ENI might not suffice the WARM_IP_TARGET/WARM_PREFIX_TARGET
-	enis := c.dataStore.GetAllocatableENIs(c.maxPrefixesPerENI, c.useCustomNetworking)
+	enis := c.dataStore.GetAllocatableENIs(c.maxPrefixesPerENI, c.skipPrimary())
 	for _, eni := range enis {
 		currentNumberOfAllocatedPrefixes := len(eni.AvailableIPv4Cidrs)
 		resourcesToAllocate := min((c.maxPrefixesPerENI - currentNumberOfAllocatedPrefixes), toAllocate)
@@ -1702,6 +1707,14 @@ func UseSubnetDiscovery() bool {
 	return parseBoolEnvVar(envSubnetDiscovery, true)
 }
 
+func SubnetDiscoverySkipPrimaryEnable() bool {
+	return parseBoolEnvVar(envSubnetDiscoverySkipPrimary, true)
+}
+
+func (c *IPAMContext) skipPrimary() bool {
+	return UseCustomNetworkCfg() || c.subnetDiscoverySkipPrimary
+}
+
 func parseBoolEnvVar(envVariableName string, defaultVal bool) bool {
 	if strValue := os.Getenv(envVariableName); strValue != "" {
 		parsedValue, err := strconv.ParseBool(strValue)
@@ -1927,11 +1940,12 @@ func (c *IPAMContext) isNodeNonSchedulable() bool {
 // GetConfigForDebug returns the active values of the configuration env vars (for debugging purposes).
 func GetConfigForDebug() map[string]interface{} {
 	return map[string]interface{}{
-		envWarmIPTarget:             getWarmIPTarget(),
-		envWarmENITarget:            getWarmENITarget(),
-		envCustomNetworkCfg:         UseCustomNetworkCfg(),
-		envManageENIsNonSchedulable: ManageENIsOnNonSchedulableNode(),
-		envSubnetDiscovery:          UseSubnetDiscovery(),
+		envWarmIPTarget:               getWarmIPTarget(),
+		envWarmENITarget:              getWarmENITarget(),
+		envCustomNetworkCfg:           UseCustomNetworkCfg(),
+		envManageENIsNonSchedulable:   ManageENIsOnNonSchedulableNode(),
+		envSubnetDiscovery:            UseSubnetDiscovery(),
+		envSubnetDiscoverySkipPrimary: SubnetDiscoverySkipPrimaryEnable(),
 	}
 }