Support specifying a custom subnet for ILB ip

unit test for verifying custom subnet config.

simplified ip address logic.

Modified init code to always initialize eventRecorder.

removed extra import from automerge with master.

staging/src/k8s.io/legacy-cloud-providers/gce/gce.go

@@ -607,11 +607,9 @@ func (g *Cloud) Initialize(clientBuilder cloudprovider.ControllerClientBuilder,
 	g.clientBuilder = clientBuilder
 	g.client = clientBuilder.ClientOrDie("cloud-provider")
 
-	if g.OnXPN() {
-		g.eventBroadcaster = record.NewBroadcaster()
-		g.eventBroadcaster.StartRecordingToSink(&v1core.EventSinkImpl{Interface: g.client.CoreV1().Events("")})
-		g.eventRecorder = g.eventBroadcaster.NewRecorder(scheme.Scheme, v1.EventSource{Component: "g-cloudprovider"})
-	}
+	g.eventBroadcaster = record.NewBroadcaster()
+	g.eventBroadcaster.StartRecordingToSink(&v1core.EventSinkImpl{Interface: g.client.CoreV1().Events("")})
+	g.eventRecorder = g.eventBroadcaster.NewRecorder(scheme.Scheme, v1.EventSource{Component: "g-cloudprovider"})
 
 	go g.watchClusterID(stop)
 }

staging/src/k8s.io/legacy-cloud-providers/gce/gce_alpha.go

@@ -27,6 +27,9 @@ const (
 	// AlphaFeatureILBSubsets allows InternalLoadBalancer services to include a subset
 	// of cluster nodes as backends instead of all nodes.
 	AlphaFeatureILBSubsets = "ILBSubsets"
+	// AlphaFeatureILBCustomSubnet allows InternalLoadBalancer services to specify a
+	// network subnet to allocate ip addresses from.
+	AlphaFeatureILBCustomSubnet = "ILBCustomSubnet"
 )
 
 // AlphaFeatureGate contains a mapping of alpha features to whether they are enabled

staging/src/k8s.io/legacy-cloud-providers/gce/gce_annotations.go

@@ -58,6 +58,11 @@ const (
 	// created in.
 	ServiceAnnotationILBAllowGlobalAccess = "networking.gke.io/internal-load-balancer-allow-global-access"
 
+	// ServiceAnnotationILBSubnet is annotated on a service with the name of the subnetwork
+	// the ILB IP Address should be assigned from. By default, this is the subnetwork that the
+	// cluster is created in.
+	ServiceAnnotationILBSubnet = "networking.gke.io/internal-load-balancer-subnet"
+
 	// NetworkTierAnnotationKey is annotated on a Service object to indicate which
 	// network tier a GCP LB should use. The valid values are "Standard" and
 	// "Premium" (default).
@@ -132,6 +137,8 @@ func GetServiceNetworkTier(service *v1.Service) (cloud.NetworkTier, error) {
 type ILBOptions struct {
 	// AllowGlobalAccess Indicates whether global access is allowed for the LoadBalancer
 	AllowGlobalAccess bool
+	// SubnetName indicates which subnet the LoadBalancer VIPs should be assigned from
+	SubnetName string
 }
 
 // GetLoadBalancerAnnotationAllowGlobalAccess returns if global access is enabled
@@ -139,3 +146,11 @@ type ILBOptions struct {
 func GetLoadBalancerAnnotationAllowGlobalAccess(service *v1.Service) bool {
 	return service.Annotations[ServiceAnnotationILBAllowGlobalAccess] == "true"
 }
+
+// GetLoadBalancerAnnotationSubnet returns the configured subnet to assign LoadBalancer IP from.
+func GetLoadBalancerAnnotationSubnet(service *v1.Service) string {
+	if val, exists := service.Annotations[ServiceAnnotationILBSubnet]; exists {
+		return val
+	}
+	return ""
+}

staging/src/k8s.io/legacy-cloud-providers/gce/gce_loadbalancer_internal.go

@@ -58,6 +58,12 @@ func (g *Cloud) ensureInternalLoadBalancer(clusterName, clusterID string, svc *v
 		g.eventRecorder.Event(svc, v1.EventTypeWarning, "ILBOptionsIgnored", "Internal LoadBalancer options are not supported with Legacy Networks.")
 		options = ILBOptions{}
 	}
+	if !g.AlphaFeatureGate.Enabled(AlphaFeatureILBCustomSubnet) {
+		if options.SubnetName != "" {
+			g.eventRecorder.Event(svc, v1.EventTypeWarning, "ILBCustomSubnetOptionIgnored", "Internal LoadBalancer CustomSubnet options ignored as the feature gate is disabled.")
+			options.SubnetName = ""
+		}
+	}
 
 	loadBalancerName := g.GetLoadBalancerName(context.TODO(), clusterName, svc)
 	sharedBackend := shareBackendService(svc)
@@ -98,23 +104,32 @@ func (g *Cloud) ensureInternalLoadBalancer(clusterName, clusterID string, svc *v
 		return nil, err
 	}
 
+	subnetworkURL := g.SubnetworkURL()
+	if g.AlphaFeatureGate.Enabled(AlphaFeatureILBCustomSubnet) {
+		// If this feature is enabled, changes to subnet annotation will be
+		// picked up and reflected in the forwarding rule.
+		// Removing the annotation will set the forwarding rule to use the default subnet.
+		if options.SubnetName != "" {
+			subnetworkURL = gceSubnetworkURL("", g.networkProjectID, g.region, options.SubnetName)
+		}
+	} else {
+		// TODO(84885) remove this once ILBCustomSubnet goes beta.
+		if existingFwdRule != nil && existingFwdRule.Subnetwork != "" {
+			// If the ILB already exists, continue using the subnet that it's already using.
+			// This is to support existing ILBs that were setup using the wrong subnet - https://github.com/kubernetes/kubernetes/pull/57861
+			subnetworkURL = existingFwdRule.Subnetwork
+		}
+	}
 	// Determine IP which will be used for this LB. If no forwarding rule has been established
 	// or specified in the Service spec, then requestedIP = "".
-	requestedIP := determineRequestedIP(svc, existingFwdRule)
-	ipToUse := requestedIP
+	ipToUse := ilbIPToUse(svc, existingFwdRule, subnetworkURL)
 
-	// If the ILB already exists, continue using the subnet that it's already using.
-	// This is to support existing ILBs that were setup using the wrong subnet.
-	subnetworkURL := g.SubnetworkURL()
-	if existingFwdRule != nil && existingFwdRule.Subnetwork != "" {
-		// external LBs have an empty Subnetwork field.
-		subnetworkURL = existingFwdRule.Subnetwork
-	}
+	klog.V(2).Infof("ensureInternalLoadBalancer(%v): Using subnet %s for LoadBalancer IP %s", loadBalancerName, options.SubnetName, ipToUse)
 
 	var addrMgr *addressManager
 	// If the network is not a legacy network, use the address manager
 	if !g.IsLegacyNetwork() {
-		addrMgr = newAddressManager(g, nm.String(), g.Region(), subnetworkURL, loadBalancerName, requestedIP, cloud.SchemeInternal)
+		addrMgr = newAddressManager(g, nm.String(), g.Region(), subnetworkURL, loadBalancerName, ipToUse, cloud.SchemeInternal)
 		ipToUse, err = addrMgr.HoldAddress()
 		if err != nil {
 			return nil, err
@@ -758,20 +773,27 @@ func getNameFromLink(link string) string {
 	return fields[len(fields)-1]
 }
 
-func determineRequestedIP(svc *v1.Service, fwdRule *compute.ForwardingRule) string {
+// ilbIPToUse determines which IP address needs to be used in the ForwardingRule. If an IP has been
+// specified by the user, that is used. If there is an existing ForwardingRule, the ip address from
+// that is reused. In case a subnetwork change is requested, the existing ForwardingRule IP is ignored.
+func ilbIPToUse(svc *v1.Service, fwdRule *compute.ForwardingRule, requestedSubnet string) string {
 	if svc.Spec.LoadBalancerIP != "" {
 		return svc.Spec.LoadBalancerIP
 	}
-
-	if fwdRule != nil {
-		return fwdRule.IPAddress
+	if fwdRule == nil {
+		return ""
 	}
-
-	return ""
+	if requestedSubnet != fwdRule.Subnetwork {
+		// reset ip address since subnet is being changed.
+		return ""
+	}
+	return fwdRule.IPAddress
 }
 
 func getILBOptions(svc *v1.Service) ILBOptions {
-	return ILBOptions{AllowGlobalAccess: GetLoadBalancerAnnotationAllowGlobalAccess(svc)}
+	return ILBOptions{AllowGlobalAccess: GetLoadBalancerAnnotationAllowGlobalAccess(svc),
+		SubnetName: GetLoadBalancerAnnotationSubnet(svc),
+	}
 }
 
 // forwardingRuleComposite is a composite type encapsulating both the GA and Beta ForwardingRules.
@@ -800,7 +822,8 @@ func (f *forwardingRuleComposite) Equal(other *forwardingRuleComposite) bool {
 		f.lbScheme == other.lbScheme &&
 		equalStringSets(f.ports, other.ports) &&
 		f.backendService == other.backendService &&
-		f.allowGlobalAccess == other.allowGlobalAccess
+		f.allowGlobalAccess == other.allowGlobalAccess &&
+		f.subnetwork == other.subnetwork
 }
 
 // toForwardingRuleComposite converts a compute beta or GA ForwardingRule into the composite type

staging/src/k8s.io/legacy-cloud-providers/gce/gce_loadbalancer_internal_test.go

@@ -1309,3 +1309,89 @@ func TestForwardingRuleCompositeEqual(t *testing.T) {
 		t.Errorf("Expected frcGA and frcBeta rules to be unequal, got true")
 	}
 }
+
+func TestEnsureInternalLoadBalancerCustomSubnet(t *testing.T) {
+	t.Parallel()
+
+	vals := DefaultTestClusterValues()
+	gce, err := fakeGCECloud(vals)
+	require.NoError(t, err)
+	gce.AlphaFeatureGate = NewAlphaFeatureGate([]string{AlphaFeatureILBCustomSubnet})
+
+	nodeNames := []string{"test-node-1"}
+	nodes, err := createAndInsertNodes(gce, nodeNames, vals.ZoneName)
+	require.NoError(t, err)
+	svc := fakeLoadbalancerService(string(LBTypeInternal))
+	status, err := createInternalLoadBalancer(gce, svc, nil, nodeNames, vals.ClusterName, vals.ClusterID, vals.ZoneName)
+	lbName := gce.GetLoadBalancerName(context.TODO(), "", svc)
+
+	if err != nil {
+		t.Errorf("Unexpected error %v", err)
+	}
+	assert.NotEmpty(t, status.Ingress)
+	fwdRule, err := gce.GetBetaRegionForwardingRule(lbName, gce.region)
+	if err != nil || fwdRule == nil {
+		t.Errorf("Unexpected error %v", err)
+	}
+	if fwdRule.Subnetwork != "" {
+		t.Errorf("Unexpected subnet value %s in ILB ForwardingRule", fwdRule.Subnetwork)
+	}
+
+	// Change service to include the global access annotation and request static ip
+	requestedIP := "4.5.6.7"
+	svc.Annotations[ServiceAnnotationILBSubnet] = "test-subnet"
+	svc.Spec.LoadBalancerIP = requestedIP
+	status, err = gce.EnsureLoadBalancer(context.Background(), vals.ClusterName, svc, nodes)
+	if err != nil {
+		t.Errorf("Unexpected error %v", err)
+	}
+	assert.NotEmpty(t, status.Ingress)
+	if status.Ingress[0].IP != requestedIP {
+		t.Errorf("Reserved IP %s not propagated, Got %s", requestedIP, status.Ingress[0].IP)
+	}
+	fwdRule, err = gce.GetBetaRegionForwardingRule(lbName, gce.region)
+	if err != nil || fwdRule == nil {
+		t.Errorf("Unexpected error %v", err)
+	}
+	if !strings.HasSuffix(fwdRule.Subnetwork, "test-subnet") {
+		t.Errorf("Unexpected subnet value %s in ILB ForwardingRule.", fwdRule.Subnetwork)
+	}
+
+	// Change to a different subnet
+	svc.Annotations[ServiceAnnotationILBSubnet] = "another-subnet"
+	status, err = gce.EnsureLoadBalancer(context.Background(), vals.ClusterName, svc, nodes)
+	if err != nil {
+		t.Errorf("Unexpected error %v", err)
+	}
+	assert.NotEmpty(t, status.Ingress)
+	if status.Ingress[0].IP != requestedIP {
+		t.Errorf("Reserved IP %s not propagated, Got %s", requestedIP, status.Ingress[0].IP)
+	}
+	fwdRule, err = gce.GetBetaRegionForwardingRule(lbName, gce.region)
+	if err != nil || fwdRule == nil {
+		t.Errorf("Unexpected error %v", err)
+	}
+	if !strings.HasSuffix(fwdRule.Subnetwork, "another-subnet") {
+		t.Errorf("Unexpected subnet value %s in ILB ForwardingRule.", fwdRule.Subnetwork)
+	}
+	// remove the annotation - ILB should revert to default subnet.
+	delete(svc.Annotations, ServiceAnnotationILBSubnet)
+	status, err = gce.EnsureLoadBalancer(context.Background(), vals.ClusterName, svc, nodes)
+	if err != nil {
+		t.Errorf("Unexpected error %v", err)
+	}
+	assert.NotEmpty(t, status.Ingress)
+	fwdRule, err = gce.GetBetaRegionForwardingRule(lbName, gce.region)
+	if err != nil {
+		t.Errorf("Unexpected error %v", err)
+	}
+	if fwdRule.Subnetwork != "" {
+		t.Errorf("Unexpected subnet value %s in ILB ForwardingRule.", fwdRule.Subnetwork)
+	}
+	// Delete the service
+	err = gce.EnsureLoadBalancerDeleted(context.Background(), vals.ClusterName, svc)
+	if err != nil {
+		t.Errorf("Unexpected error %v", err)
+	}
+	assertInternalLbResourcesDeleted(t, gce, svc, vals, true)
+}