Remove any Egress IP from Azure public LB backend

arghosh93 · arghosh93 · commit dac96380cba8 · 2025-12-12T18:35:15.000+05:30
The consensus is to not add egress IP to public load balancer
backend pool regardless of the presence of an OutBoundRule.
During upgrade this PR let cobtroller removes any egress IP
added to public load balancer backend pool previously.

Signed-off-by: Arnab Ghosh &lt;arnabghosh89@gmail.com&gt;
diff --git a/cmd/cloud-network-config-controller/main.go b/cmd/cloud-network-config-controller/main.go
@@ -27,6 +27,7 @@ import (
 	kubeinformers "k8s.io/client-go/informers"
 	"k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/rest"
+	"k8s.io/client-go/tools/cache"
 	"k8s.io/client-go/tools/clientcmd"
 	"k8s.io/client-go/tools/leaderelection"
 	"k8s.io/client-go/tools/leaderelection/resourcelock"
@@ -139,14 +140,29 @@ func main() {
 					klog.Exitf("Error building cloudnetwork clientset: %s", err.Error())
 				}
 
-				cloudProviderClient, err := cloudprovider.NewCloudProviderClient(platformCfg, platformStatus, featureGates)
+				kubeInformerFactory := kubeinformers.NewSharedInformerFactoryWithOptions(kubeClient, time.Minute*2, kubeinformers.WithNamespace(controllerNamespace))
+				cloudNetworkInformerFactory := cloudnetworkinformers.NewSharedInformerFactory(cloudNetworkClient, time.Minute*2)
+				cloudPrivateIPConfigLister := cloudNetworkInformerFactory.Cloud().V1().CloudPrivateIPConfigs().Lister()
+				nodeLister := kubeInformerFactory.Core().V1().Nodes().Lister()
+				cloudNetworkInformerFactory.Start(stopCh)
+				kubeInformerFactory.Start(stopCh)
+				if ok := cache.WaitForCacheSync(stopCh,
+					cloudNetworkInformerFactory.Cloud().V1().CloudPrivateIPConfigs().Informer().HasSynced,
+					kubeInformerFactory.Core().V1().Nodes().Informer().HasSynced,
+				); !ok {
+					klog.Fatal("Timed out waiting for informer caches to sync")
+				}
+
+				cloudProviderClient, err := cloudprovider.NewCloudProviderClient(platformCfg,
+					platformStatus,
+					featureGates,
+					cloudPrivateIPConfigLister,
+					nodeLister,
+				)
 				if err != nil {
 					klog.Fatalf("Error building cloud provider client, err: %v", err)
 				}
 
-				kubeInformerFactory := kubeinformers.NewSharedInformerFactoryWithOptions(kubeClient, time.Minute*2, kubeinformers.WithNamespace(controllerNamespace))
-				cloudNetworkInformerFactory := cloudnetworkinformers.NewSharedInformerFactory(cloudNetworkClient, time.Minute*2)
-
 				cloudPrivateIPConfigController, err := cloudprivateipconfigcontroller.NewCloudPrivateIPConfigController(
 					ctx,
 					cloudProviderClient,
@@ -179,9 +195,6 @@ func main() {
 					klog.Fatalf("Error getting secret controller, err: %v", err)
 				}
 
-				cloudNetworkInformerFactory.Start(stopCh)
-				kubeInformerFactory.Start(stopCh)
-
 				wg.Add(1)
 				go func() {
 					defer wg.Done()
diff --git a/pkg/cloudprovider/aws.go b/pkg/cloudprovider/aws.go
@@ -33,6 +33,14 @@ type AWS struct {
 	client *ec2.EC2
 }
 
+func (a *AWS) init() error {
+	if err := a.initCredentials(); err != nil {
+		return err
+	}
+
+	return nil
+}
+
 func (a *AWS) initCredentials() error {
 	f, err := sharedCredentialsFileFromDirectory(a.cfg.CredentialDir)
 	if err != nil {
diff --git a/pkg/cloudprovider/azure.go b/pkg/cloudprovider/azure.go
@@ -9,23 +9,27 @@ import (
 	"sync"
 	"time"
 
-	"github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime"
-	"k8s.io/utils/ptr"
-
 	"github.com/Azure/azure-sdk-for-go/sdk/azcore"
 	"github.com/Azure/azure-sdk-for-go/sdk/azcore/arm"
 	"github.com/Azure/azure-sdk-for-go/sdk/azcore/cloud"
 	"github.com/Azure/azure-sdk-for-go/sdk/azcore/policy"
+	"github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime"
 	"github.com/Azure/azure-sdk-for-go/sdk/azidentity"
 	"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute"
 	"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/network/armnetwork/v6"
 	azureapi "github.com/Azure/go-autorest/autorest/azure"
 	"github.com/Azure/msi-dataplane/pkg/dataplane"
 	configv1 "github.com/openshift/api/config/v1"
+	cloudnetworklisters "github.com/openshift/client-go/cloudnetwork/listers/cloudnetwork/v1"
+	"github.com/openshift/cloud-network-config-controller/pkg/cloudprivateipconfig"
 	corev1 "k8s.io/api/core/v1"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/util/sets"
+	corelisters "k8s.io/client-go/listers/core/v1"
 	"k8s.io/klog/v2"
 	utilnet "k8s.io/utils/net"
+	"k8s.io/utils/ptr"
 )
 
 const (
@@ -51,6 +55,10 @@ type Azure struct {
 	nodeMapLock                  sync.Mutex
 	nodeLockMap                  map[string]*sync.Mutex
 	azureWorkloadIdentityEnabled bool
+	cloudPrivateIPConfigLister   cloudnetworklisters.CloudPrivateIPConfigLister
+	nodeLister                   corelisters.NodeLister
+	lbBackendPoolSynced          map[string]bool
+	lbBackendPoolSyncedLock      sync.Mutex
 }
 
 type azureCredentialsConfig struct {
@@ -113,6 +121,20 @@ func (a *Azure) readAzureCredentialsConfig() (*azureCredentialsConfig, error) {
 
 	return &cfg, nil
 }
+
+func (a *Azure) init() error {
+	if err := a.initCredentials(); err != nil {
+		return err
+	}
+
+	// This is mostly Azure specific and may be removed later.
+	if err := a.SyncLBBackend(); err != nil {
+		return err
+	}
+
+	return nil
+}
+
 func (a *Azure) initCredentials() error {
 	cfg, err := a.readAzureCredentialsConfig()
 	if err != nil {
@@ -172,11 +194,11 @@ func (a *Azure) AssignPrivateIP(ip net.IP, node *corev1.Node) error {
 	defer nodeLock.Unlock()
 	instance, err := a.getInstance(node)
 	if err != nil {
-		return err
+		return fmt.Errorf("error while retrieving instance details from Azure: %w", err)
 	}
 	networkInterfaces, err := a.getNetworkInterfaces(instance)
 	if err != nil {
-		return err
+		return fmt.Errorf("error while retrieving interface details from Azure: %w", err)
 	}
 	if networkInterfaces[0].Properties == nil {
 		return fmt.Errorf("nil network interface properties")
@@ -219,9 +241,18 @@ func (a *Azure) AssignPrivateIP(ip net.IP, node *corev1.Node) error {
 		"omitting backend address pool when adding secondary IP", ipc)
 	poller, err := a.createOrUpdate(networkInterface)
 	if err != nil {
-		return err
+		return fmt.Errorf("error while updating network interface: %w", err)
+	}
+	if err = a.waitForCompletion(poller); err != nil {
+		return fmt.Errorf("error while updating network interface: %w", err)
 	}
-	return a.waitForCompletion(poller)
+	// setting lbBackendPoolSynced to true here to make sure that we dont try to
+	// sync LB backend for any new egress IP later
+	cacheKey := getIPCacheKey(ip, node.Name)
+	a.lbBackendPoolSyncedLock.Lock()
+	a.lbBackendPoolSynced[cacheKey] = true
+	a.lbBackendPoolSyncedLock.Unlock()
+	return nil
 }
 
 func (a *Azure) ReleasePrivateIP(ip net.IP, node *corev1.Node) error {
@@ -231,11 +262,11 @@ func (a *Azure) ReleasePrivateIP(ip net.IP, node *corev1.Node) error {
 	defer nodeLock.Unlock()
 	instance, err := a.getInstance(node)
 	if err != nil {
-		return err
+		return fmt.Errorf("error while retrieving instance details from Azure: %w", err)
 	}
 	networkInterfaces, err := a.getNetworkInterfaces(instance)
 	if err != nil {
-		return err
+		return fmt.Errorf("error while retrieving interface details from Azure: %w", err)
 	}
 	// Perform the operation against the first interface listed, which will be
 	// the primary interface (if it's defined as such) or the first one returned
@@ -262,9 +293,16 @@ func (a *Azure) ReleasePrivateIP(ip net.IP, node *corev1.Node) error {
 	// Send the request
 	poller, err := a.createOrUpdate(networkInterface)
 	if err != nil {
-		return err
+		return fmt.Errorf("error while updating network interface: %w", err)
+	}
+	if err = a.waitForCompletion(poller); err != nil {
+		return fmt.Errorf("error while updating network interface: %w", err)
 	}
-	return a.waitForCompletion(poller)
+	cacheKey := getIPCacheKey(ip, node.Name)
+	a.lbBackendPoolSyncedLock.Lock()
+	delete(a.lbBackendPoolSynced, cacheKey)
+	a.lbBackendPoolSyncedLock.Unlock()
+	return nil
 }
 
 func (a *Azure) GetNodeEgressIPConfiguration(node *corev1.Node, cpicIPs sets.Set[string]) ([]*NodeEgressIPConfiguration, error) {
@@ -302,6 +340,90 @@ func (a *Azure) GetNodeEgressIPConfiguration(node *corev1.Node, cpicIPs sets.Set
 	return []*NodeEgressIPConfiguration{config}, nil
 }
 
+// The consensus is to not add egress IP to public load balancer
+// backend pool regardless of the presence of an OutBoundRule.
+// During upgrade this function removes any egress IP added to
+// public load balancer backend pool previously.
+func (a *Azure) SyncLBBackend() error {
+	cloudPrivateIPConfigs, err := a.cloudPrivateIPConfigLister.List(labels.Everything())
+	if err != nil {
+		return fmt.Errorf("error listing cloud private ip config, err: %v", err)
+	}
+	for _, cloudPrivateIPConfig := range cloudPrivateIPConfigs {
+		ip, _, err := cloudprivateipconfig.NameToIP(cloudPrivateIPConfig.Name)
+		if err != nil {
+			return fmt.Errorf("error parsing CloudPrivateIPConfig %s: %v", cloudPrivateIPConfig.Name, err)
+		}
+		cacheKey := getIPCacheKey(ip, cloudPrivateIPConfig.Spec.Node)
+		a.lbBackendPoolSyncedLock.Lock()
+		if synced, ok := a.lbBackendPoolSynced[cacheKey]; ok && synced {
+			// nothing to do. Continue if LB backend has already synced
+			a.lbBackendPoolSyncedLock.Unlock()
+			continue
+		}
+		a.lbBackendPoolSyncedLock.Unlock()
+		ipc := ip.String()
+		node, err := a.nodeLister.Get(cloudPrivateIPConfig.Spec.Node)
+		if err != nil && apierrors.IsNotFound(err) {
+			klog.Warningf("source node: %s no longer exists for CloudPrivateIPConfig: %q",
+				cloudPrivateIPConfig.Spec.Node, cloudPrivateIPConfig.Name)
+			continue
+		} else if err != nil {
+			return fmt.Errorf("error getting node %s for CloudPrivateIPConfig %q: %w",
+				cloudPrivateIPConfig.Spec.Node, cloudPrivateIPConfig.Name, err)
+		}
+
+		klog.Infof("Acquiring node lock for modifying load balancer backend pool, node: %s, ip: %s", node.Name, ipc)
+		nodeLock := a.getNodeLock(node.Name)
+		if err := func() error {
+			nodeLock.Lock()
+			defer nodeLock.Unlock()
+			instance, err := a.getInstance(node)
+			if err != nil {
+				return fmt.Errorf("error while retrieving instance details from Azure: %w", err)
+			}
+			networkInterfaces, err := a.getNetworkInterfaces(instance)
+			if err != nil {
+				return fmt.Errorf("error while retrieving interface details from Azure: %w", err)
+			}
+			if networkInterfaces[0].Properties == nil {
+				return fmt.Errorf("nil network interface properties")
+			}
+			// Perform the operation against the first interface listed, which will be
+			// the primary interface (if it's defined as such) or the first one returned
+			// following the order Azure specifies.
+			networkInterface := networkInterfaces[0]
+			var loadBalancerBackendPoolModified bool
+			// omit Egress IP from LB backend pool
+			ipConfigurations := networkInterface.Properties.IPConfigurations
+			for _, ipCfg := range ipConfigurations {
+				if ptr.Deref(ipCfg.Properties.PrivateIPAddress, "") == ipc &&
+					ipCfg.Properties.LoadBalancerBackendAddressPools != nil {
+					ipCfg.Properties.LoadBalancerBackendAddressPools = nil
+					loadBalancerBackendPoolModified = true
+				}
+			}
+			if loadBalancerBackendPoolModified {
+				networkInterface.Properties.IPConfigurations = ipConfigurations
+				poller, err := a.createOrUpdate(networkInterface)
+				if err != nil {
+					return fmt.Errorf("error while updating network interface: %w", err)
+				}
+				if err = a.waitForCompletion(poller); err != nil {
+					return fmt.Errorf("error while updating network interface: %w", err)
+				}
+				a.lbBackendPoolSyncedLock.Lock()
+				a.lbBackendPoolSynced[cacheKey] = true
+				a.lbBackendPoolSyncedLock.Unlock()
+			}
+			return nil
+		}(); err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
 func (a *Azure) createOrUpdate(networkInterface armnetwork.Interface) (*runtime.Poller[armnetwork.InterfacesClientCreateOrUpdateResponse], error) {
 	ctx, cancel := context.WithTimeout(a.ctx, defaultAzureOperationTimeout)
 	defer cancel()
@@ -604,3 +726,7 @@ func ParseCloudEnvironment(env azureapi.Environment) cloud.Configuration {
 	}
 	return cloudConfig
 }
+
+func getIPCacheKey(ip net.IP, node string) string {
+	return fmt.Sprintf("%s|%s", node, ip.String())
+}
diff --git a/pkg/cloudprovider/cloudprovider.go b/pkg/cloudprovider/cloudprovider.go
@@ -11,7 +11,9 @@ import (
 
 	configv1 "github.com/openshift/api/config/v1"
 	apifeatures "github.com/openshift/api/features"
+	cloudnetworklisters "github.com/openshift/client-go/cloudnetwork/listers/cloudnetwork/v1"
 	"github.com/openshift/library-go/pkg/operator/configobserver/featuregates"
+	corelisters "k8s.io/client-go/listers/core/v1"
 
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/util/sets"
@@ -31,14 +33,16 @@ func UnexpectedURIError(uri string) error {
 }
 
 type CloudProviderIntf interface {
-	// initCredentials initializes the cloud API credentials by reading the
+	// init function initializes the cloud API credentials by reading the
 	// secret data which has been mounted in cloudProviderSecretLocation. The
 	// mounted secret data in Kubernetes is generated following a one-to-one
 	// mapping between each .data field and a corresponding file. Hence
 	// .data.foo will generate a file foo in that location with the decoded
 	// secret data, similarity we would have a file bar if .data.bar was
 	// defined.
-	initCredentials() error
+	// For Azure platform it additionally removes any egress IP which is
+	// already added to backend pool of a public load balancer.
+	init() error
 
 	// AssignPrivateIP attempts to assigning the IP address provided to the VM
 	// instance corresponding to the corev1.Node provided on the cloud the
@@ -134,7 +138,11 @@ func (n *NodeEgressIPConfiguration) String() string {
 	return fmt.Sprintf("%v", *n)
 }
 
-func NewCloudProviderClient(cfg CloudProviderConfig, platformStatus *configv1.PlatformStatus, featureGates featuregates.FeatureGate) (CloudProviderIntf, error) {
+func NewCloudProviderClient(cfg CloudProviderConfig,
+	platformStatus *configv1.PlatformStatus,
+	featureGates featuregates.FeatureGate,
+	cloudPrivateIPConfigLister cloudnetworklisters.CloudPrivateIPConfigLister,
+	nodeLister corelisters.NodeLister) (CloudProviderIntf, error) {
 	var cloudProviderIntf CloudProviderIntf
 
 	// Initialize a separate context from the main context, rationale: cloud
@@ -161,6 +169,9 @@ func NewCloudProviderClient(cfg CloudProviderConfig, platformStatus *configv1.Pl
 			platformStatus:               azurePlatformStatus,
 			nodeLockMap:                  make(map[string]*sync.Mutex),
 			azureWorkloadIdentityEnabled: featureGates.Enabled(apifeatures.FeatureGateAzureWorkloadIdentity),
+			lbBackendPoolSynced:          make(map[string]bool),
+			cloudPrivateIPConfigLister:   cloudPrivateIPConfigLister,
+			nodeLister:                   nodeLister,
 		}
 	case PlatformTypeAWS:
 		cloudProviderIntf = &AWS{
@@ -178,7 +189,7 @@ func NewCloudProviderClient(cfg CloudProviderConfig, platformStatus *configv1.Pl
 	default:
 		return nil, fmt.Errorf("unsupported cloud provider platform type: %s", cfg.PlatformType)
 	}
-	return cloudProviderIntf, cloudProviderIntf.initCredentials()
+	return cloudProviderIntf, cloudProviderIntf.init()
 }
 
 func (c *CloudProvider) readSecretData(secret string) (string, error) {
diff --git a/pkg/cloudprovider/cloudprovider_fake.go b/pkg/cloudprovider/cloudprovider_fake.go
@@ -30,7 +30,7 @@ func NewFakeCloudProvider(mockErrorOnAssign, mockErrorOnAssignWithExistingIPCond
 	}
 }
 
-func (f *FakeCloudProvider) initCredentials() error {
+func (f *FakeCloudProvider) init() error {
 	return nil
 }
 
diff --git a/pkg/cloudprovider/gcp.go b/pkg/cloudprovider/gcp.go
@@ -36,6 +36,14 @@ type GCP struct {
 	nodeLockMap map[string]*sync.Mutex
 }
 
+func (g *GCP) init() error {
+	if err := g.initCredentials(); err != nil {
+		return err
+	}
+
+	return nil
+}
+
 func (g *GCP) initCredentials() (err error) {
 	secret, err := g.readSecretData("service_account.json")
 	if err != nil {
diff --git a/pkg/cloudprovider/openstack.go b/pkg/cloudprovider/openstack.go
@@ -67,6 +67,14 @@ type OpenStack struct {
 
 var novaDeviceOwnerRegex = regexp.MustCompile("^compute:.*")
 
+func (o *OpenStack) init() error {
+	if err := o.initCredentials(); err != nil {
+		return err
+	}
+
+	return nil
+}
+
 // initCredentials initializes the cloud API credentials by reading the
 // secret data which has been mounted in cloudProviderSecretLocation. The
 // mounted secret data in Kubernetes is generated following a one-to-one
diff --git a/pkg/controller/node/node_controller.go b/pkg/controller/node/node_controller.go

Original file line number	Diff line number	Diff line change
`@@ -30,7 +30,7 @@ func NewFakeCloudProvider(mockErrorOnAssign, mockErrorOnAssignWithExistingIPCond`
`30`	`30`	`}`
`31`	`31`	`}`
`32`	`32`
`33`		`-func (f *FakeCloudProvider) initCredentials() error {`
	`33`	`+func (f *FakeCloudProvider) init() error {`
`34`	`34`	`return nil`
`35`	`35`	`}`
`36`	`36`