Create getNonDefaultSubnetName to generate NEG names from non-default

subnets.
kubernetes · Nov 7, 2024 · 6d74dbc · 6d74dbc
1 parent 72ba016
commit 6d74dbc
Show file tree

Hide file tree

Showing 2 changed files with 84 additions and 9 deletions.
diff --git a/pkg/neg/syncers/transaction.go b/pkg/neg/syncers/transaction.go
@@ -467,14 +467,10 @@ func (s *transactionSyncer) ensureNetworkEndpointGroups() error {
 
 		if subnetConfig.Name != defaultSubnet {
 			// Determine the NEG name for the non-default subnet NEGs.
-			negName = s.namer.NonDefaultSubnetNEG(s.NegSyncerKey.Namespace, s.NegSyncerKey.Name, subnetConfig.Name, s.NegSyncerKey.PortTuple.Port)
-			if s.customName {
-				if len(s.NegSyncerKey.NegName) > negtypes.MaxDefaultSubnetNegNameLength {
-					s.logger.Error(ErrCustomNEGNameTooLong, "Unable to create custom NEGs in non-default subnet", "customNegName", s.NegSyncerKey.Name)
-					errList = append(errList, ErrCustomNEGNameTooLong)
-					continue
-				}
-				negName = fmt.Sprintf("%s-%s", s.NegSyncerKey.Name, namer.SubnetHash(subnetConfig.Name))
+			negName, err = s.getNonDefaultSubnetName(subnetConfig.Name)
+			if err != nil {
+				errList = append(errList, ErrCustomNEGNameTooLong)
+				continue
 			}
 
 			// Determine the networkInfo for the non-default subnet NEGs.
@@ -940,6 +936,18 @@ func (s *transactionSyncer) computeEPSStaleness(endpointSlices []*discovery.Endp
 	}
 }
 
+func (s *transactionSyncer) getNonDefaultSubnetName(subnet string) (string, error) {
+	negName := s.namer.NonDefaultSubnetNEG(s.NegSyncerKey.Namespace, s.NegSyncerKey.Name, subnet, s.NegSyncerKey.PortTuple.Port)
+	if s.customName {
+		if len(s.NegSyncerKey.NegName) > negtypes.MaxDefaultSubnetNegNameLength {
+			s.logger.Error(ErrCustomNEGNameTooLong, "Unable to generate NEG name for custom NEGs in non-default subnet", "customNegName", s.NegSyncerKey.Name)
+			return "", ErrCustomNEGNameTooLong
+		}
+		negName = fmt.Sprintf("%s-%s", s.NegSyncerKey.Name, namer.SubnetHash(subnet))
+	}
+	return negName, nil
+}
+
 // computeDegradedModeCorrectness computes degraded mode correctness metrics based on the difference between degraded mode and normal calculation
 func computeDegradedModeCorrectness(notInDegraded, onlyInDegraded map[negtypes.EndpointGroupInfo]negtypes.NetworkEndpointSet, negType string, logger klog.Logger) {
 	logger.Info("Exporting degraded mode correctness metrics", "notInDegraded", notInDegraded, "onlyInDegraded", onlyInDegraded)

diff --git a/pkg/neg/syncers/transaction_test.go b/pkg/neg/syncers/transaction_test.go
@@ -73,6 +73,7 @@ const (
 	testUnreadyInstance2 = "unready-instance2"
 
 	defaultTestSubnet    = "default"
+	additionalTestSubnet = "additional-subnet"
 	secondaryTestSubnet1 = "secondary1"
 	secondaryTestSubnet2 = "secondary2"
 )
@@ -1346,7 +1347,6 @@ func TestEnsureNetworkEndpointGroupsMSC(t *testing.T) {
 	testNetworkURL := cloud.SelfLink(meta.VersionGA, "mock-project", "networks", meta.GlobalKey(defaultTestSubnet))
 	testSubnetworkURL := cloud.SelfLink(meta.VersionGA, "mock-project", "subnetworks", meta.RegionalKey(defaultTestSubnet, "test-region"))
 	testNegType := negtypes.VmIpPortEndpointType
-	additionalTestSubnet := "additional-subnet"
 	additionalTestSubnetworkURL := cloud.SelfLink(meta.VersionGA, "mock-project", "subnetworks", meta.RegionalKey(additionalTestSubnet, "test-region"))
 
 	nodeTopologyCrWithDefaultSubnetOnly := nodetopologyv1.NodeTopology{
@@ -2625,6 +2625,73 @@ func TestCollectLabelStats(t *testing.T) {
 	}
 }
 
+func TestGetNonDefaultSubnetName(t *testing.T) {
+	t.Parallel()
+	vals := gce.DefaultTestClusterValues()
+	vals.SubnetworkURL = defaultTestSubnetURL
+	fakeGCE := gce.NewFakeGCECloud(vals)
+	negtypes.MockNetworkEndpointAPIs(fakeGCE)
+	fakeCloud := negtypes.NewAdapter(fakeGCE)
+	testNegTypes := []negtypes.NetworkEndpointType{
+		negtypes.VmIpEndpointType,
+		negtypes.VmIpPortEndpointType,
+	}
+
+	testCases := []struct {
+		desc              string
+		customNEGName     string
+		expectedL4NegName string
+		expectedL7NegName string
+		expectError       bool
+	}{
+		{
+			desc:              "auto-generated NEG name",
+			expectedL4NegName: "k8s1-clusteri-test-ns-test-name-0-cc51aa-8a665f6c",
+			expectedL7NegName: "k8s1-clusteri-test-ns-test-name-80-cc51aa-137ee03a",
+			expectError:       false,
+		},
+		{
+			desc:              "custom NEG name not exceeding character limit",
+			customNEGName:     "custom-neg",
+			expectedL4NegName: "test-name-cc51aa",
+			expectedL7NegName: "test-name-cc51aa",
+			expectError:       false,
+		},
+		{
+			desc:          " custom NEG name exceeding character limit",
+			customNEGName: "012345678901234567890123456789012345678901234567890123456", // 57 characters
+			expectError:   true,
+		},
+	}
+
+	for _, testNegType := range testNegTypes {
+		for _, tc := range testCases {
+			t.Run(tc.desc, func(t *testing.T) {
+				_, syncer := newTestTransactionSyncer(fakeCloud, testNegType, tc.customNEGName != "")
+				if tc.customNEGName != "" {
+					syncer.NegSyncerKey.NegName = tc.customNEGName
+				}
+				got, err := syncer.getNonDefaultSubnetName(additionalTestSubnet)
+				t.Logf("NEG name: %q", syncer.NegSyncerKey.NegName)
+				if err == nil && tc.expectError {
+					t.Errorf("For NEG type %q, got err == nil, expected err != nil", testNegType)
+				}
+				if err != nil && !tc.expectError {
+					t.Errorf("For NEG type %q, got err = %v, expected err == nil", testNegType, err)
+				}
+				if !tc.expectError {
+					if testNegType == negtypes.VmIpEndpointType && got != tc.expectedL4NegName {
+						t.Errorf("For NEG type %q, got NEG name %q, expected %q", testNegType, got, tc.expectedL4NegName)
+					}
+					if testNegType == negtypes.VmIpPortEndpointType && got != tc.expectedL7NegName {
+						t.Errorf("For NEG type %q, got NEG name %q, expected %q", testNegType, got, tc.expectedL7NegName)
+					}
+				}
+			})
+		}
+	}
+}
+
 func newTestTransactionSyncer(fakeGCE negtypes.NetworkEndpointGroupCloud, negType negtypes.NetworkEndpointType, customName bool) (negtypes.NegSyncer, *transactionSyncer) {
 	testContext := negtypes.NewTestContext()
 	svcPort := negtypes.NegSyncerKey{