Skip to content

Commit

Permalink
Ensure orphan public IPs deleted
Browse files Browse the repository at this point in the history
  • Loading branch information
@feiskyer
feiskyer committed Oct 31, 2018
1 parent d7de3e5 commit 9aeb005
Show file tree
Hide file tree
Showing 2 changed files with 95 additions and 22 deletions.
101 changes: 87 additions & 14 deletions pkg/cloudprovider/providers/azure/azure_loadbalancer.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ func (az *Cloud) EnsureLoadBalancer(ctx context.Context, clusterName string, ser
return nil, err
}

if _, err := az.reconcilePublicIP(clusterName, updateService, true /* wantLb */); err != nil {
if _, err := az.reconcilePublicIP(clusterName, updateService, lb, true /* wantLb */); err != nil {
return nil, err
}

Expand Down Expand Up @@ -185,7 +185,7 @@ func (az *Cloud) EnsureLoadBalancerDeleted(ctx context.Context, clusterName stri
return err
}

if _, err := az.reconcilePublicIP(clusterName, service, false /* wantLb */); err != nil {
if _, err := az.reconcilePublicIP(clusterName, service, nil, false /* wantLb */); err != nil {
return err
}

Expand Down Expand Up @@ -1301,7 +1301,7 @@ func deduplicate(collection *[]string) *[]string {
}

// This reconciles the PublicIP resources similar to how the LB is reconciled.
func (az *Cloud) reconcilePublicIP(clusterName string, service *v1.Service, wantLb bool) (*network.PublicIPAddress, error) {
func (az *Cloud) reconcilePublicIP(clusterName string, service *v1.Service, lb *network.LoadBalancer, wantLb bool) (*network.PublicIPAddress, error) {
isInternal := requiresInternalLoadBalancer(service)
serviceName := getServiceName(service)
var desiredPipName string
Expand All @@ -1320,7 +1320,8 @@ func (az *Cloud) reconcilePublicIP(clusterName string, service *v1.Service, want
return nil, err
}

for _, pip := range pips {
for i := range pips {
pip := pips[i]
if pip.Tags != nil &&
(pip.Tags)["service"] != nil &&
*(pip.Tags)["service"] == serviceName {
Expand All @@ -1331,17 +1332,9 @@ func (az *Cloud) reconcilePublicIP(clusterName string, service *v1.Service, want
// Public ip resource with match service tag
} else {
glog.V(2).Infof("reconcilePublicIP for service(%s): pip(%s) - deleting", serviceName, pipName)
glog.V(10).Infof("DeletePublicIPWithRetry(%s, %q): start", pipResourceGroup, pipName)
err = az.DeletePublicIPWithRetry(service, pipResourceGroup, pipName)
if err != nil {
glog.V(2).Infof("ensure(%s) abort backoff: pip(%s) - deleting", serviceName, pipName)
// We let err to pass through
// It may be ignorable
}
glog.V(10).Infof("DeletePublicIPWithRetry(%s, %q): end", pipResourceGroup, pipName) // response not read yet...

err = ignoreStatusNotFoundFromError(err)
err := az.safeDeletePublicIP(service, pipResourceGroup, &pip, lb)
if err != nil {
glog.Errorf("safeDeletePublicIP(%s) failed with error: %v", pipName, err)
return nil, err
}
glog.V(2).Infof("reconcilePublicIP for service(%s): pip(%s) - finished", serviceName, pipName)
Expand All @@ -1362,6 +1355,86 @@ func (az *Cloud) reconcilePublicIP(clusterName string, service *v1.Service, want
return nil, nil
}

// safeDeletePublicIP deletes public IP by removing its reference first.
func (az *Cloud) safeDeletePublicIP(service *v1.Service, pipResourceGroup string, pip *network.PublicIPAddress, lb *network.LoadBalancer) error {
// Remove references if pip.IPConfiguration is not nil.
if pip.PublicIPAddressPropertiesFormat != nil &&
pip.PublicIPAddressPropertiesFormat.IPConfiguration != nil &&
lb != nil && lb.LoadBalancerPropertiesFormat != nil &&
lb.LoadBalancerPropertiesFormat.FrontendIPConfigurations != nil {
referencedLBRules := []network.SubResource{}
frontendIPConfigUpdated := false
loadBalancerRuleUpdated := false

// Check whether there are still frontend IP configurations referring to it.
ipConfigurationID := to.String(pip.PublicIPAddressPropertiesFormat.IPConfiguration.ID)
if ipConfigurationID != "" {
lbFrontendIPConfigs := *lb.LoadBalancerPropertiesFormat.FrontendIPConfigurations
for i := len(lbFrontendIPConfigs) - 1; i >= 0; i-- {
config := lbFrontendIPConfigs[i]
if strings.EqualFold(ipConfigurationID, to.String(config.ID)) {
if config.FrontendIPConfigurationPropertiesFormat != nil &&
config.FrontendIPConfigurationPropertiesFormat.LoadBalancingRules != nil {
referencedLBRules = *config.FrontendIPConfigurationPropertiesFormat.LoadBalancingRules
}

frontendIPConfigUpdated = true
lbFrontendIPConfigs = append(lbFrontendIPConfigs[:i], lbFrontendIPConfigs[i+1:]...)
break
}
}

if frontendIPConfigUpdated {
lb.LoadBalancerPropertiesFormat.FrontendIPConfigurations = &lbFrontendIPConfigs
}
}

// Check whether there are still load balancer rules referring to it.
if len(referencedLBRules) > 0 {
referencedLBRuleIDs := sets.NewString()
for _, refer := range referencedLBRules {
referencedLBRuleIDs.Insert(to.String(refer.ID))
}

if lb.LoadBalancerPropertiesFormat.LoadBalancingRules != nil {
lbRules := *lb.LoadBalancerPropertiesFormat.LoadBalancingRules
for i := len(lbRules) - 1; i >= 0; i-- {
ruleID := to.String(lbRules[i].ID)
if ruleID != "" && referencedLBRuleIDs.Has(ruleID) {
loadBalancerRuleUpdated = true
lbRules = append(lbRules[:i], lbRules[i+1:]...)
}
}

if loadBalancerRuleUpdated {
lb.LoadBalancerPropertiesFormat.LoadBalancingRules = &lbRules
}
}
}

// Update load balancer when frontendIPConfigUpdated or loadBalancerRuleUpdated.
if frontendIPConfigUpdated || loadBalancerRuleUpdated {
err := az.CreateOrUpdateLBWithRetry(service, *lb)
if err != nil {
glog.Errorf("safeDeletePublicIP for service(%s) failed with error: %v", getServiceName(service), err)
return err
}
}
}

pipName := to.String(pip.Name)
glog.V(10).Infof("DeletePublicIPWithRetry(%s, %q): start", pipResourceGroup, pipName)
err := az.DeletePublicIPWithRetry(service, pipResourceGroup, pipName)
if err != nil {
if err = ignoreStatusNotFoundFromError(err); err != nil {
return err
}
}
glog.V(10).Infof("DeletePublicIPWithRetry(%s, %q): end", pipResourceGroup, pipName)

return nil
}

func findProbe(probes []network.Probe, probe network.Probe) bool {
for _, existingProbe := range probes {
if strings.EqualFold(to.String(existingProbe.Name), to.String(probe.Name)) && to.Int32(existingProbe.Port) == to.Int32(probe.Port) {
Expand Down
16 changes: 8 additions & 8 deletions pkg/cloudprovider/providers/azure/azure_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -858,13 +858,13 @@ func TestReconcilePublicIPWithNewService(t *testing.T) {
az := getTestCloud()
svc := getTestService("servicea", v1.ProtocolTCP, 80, 443)

pip, err := az.reconcilePublicIP(testClusterName, &svc, true /* wantLb*/)
pip, err := az.reconcilePublicIP(testClusterName, &svc, nil, true /* wantLb*/)
if err != nil {
t.Errorf("Unexpected error: %q", err)
}
validatePublicIP(t, pip, &svc, true)

pip2, err := az.reconcilePublicIP(testClusterName, &svc, true /* wantLb */)
pip2, err := az.reconcilePublicIP(testClusterName, &svc, nil, true /* wantLb */)
if err != nil {
t.Errorf("Unexpected error: %q", err)
}
Expand All @@ -879,15 +879,15 @@ func TestReconcilePublicIPRemoveService(t *testing.T) {
az := getTestCloud()
svc := getTestService("servicea", v1.ProtocolTCP, 80, 443)

pip, err := az.reconcilePublicIP(testClusterName, &svc, true /* wantLb*/)
pip, err := az.reconcilePublicIP(testClusterName, &svc, nil, true /* wantLb*/)
if err != nil {
t.Errorf("Unexpected error: %q", err)
}

validatePublicIP(t, pip, &svc, true)

// Remove the service
pip, err = az.reconcilePublicIP(testClusterName, &svc, false /* wantLb */)
pip, err = az.reconcilePublicIP(testClusterName, &svc, nil, false /* wantLb */)
if err != nil {
t.Errorf("Unexpected error: %q", err)
}
Expand All @@ -899,7 +899,7 @@ func TestReconcilePublicIPWithInternalService(t *testing.T) {
az := getTestCloud()
svc := getInternalTestService("servicea", 80, 443)

pip, err := az.reconcilePublicIP(testClusterName, &svc, true /* wantLb*/)
pip, err := az.reconcilePublicIP(testClusterName, &svc, nil, true /* wantLb*/)
if err != nil {
t.Errorf("Unexpected error: %q", err)
}
Expand All @@ -911,22 +911,22 @@ func TestReconcilePublicIPWithExternalAndInternalSwitch(t *testing.T) {
az := getTestCloud()
svc := getInternalTestService("servicea", 80, 443)

pip, err := az.reconcilePublicIP(testClusterName, &svc, true /* wantLb*/)
pip, err := az.reconcilePublicIP(testClusterName, &svc, nil, true /* wantLb*/)
if err != nil {
t.Errorf("Unexpected error: %q", err)
}
validatePublicIP(t, pip, &svc, true)

// Update to external service
svcUpdated := getTestService("servicea", v1.ProtocolTCP, 80)
pip, err = az.reconcilePublicIP(testClusterName, &svcUpdated, true /* wantLb*/)
pip, err = az.reconcilePublicIP(testClusterName, &svcUpdated, nil, true /* wantLb*/)
if err != nil {
t.Errorf("Unexpected error: %q", err)
}
validatePublicIP(t, pip, &svcUpdated, true)

// Update to internal service again
pip, err = az.reconcilePublicIP(testClusterName, &svc, true /* wantLb*/)
pip, err = az.reconcilePublicIP(testClusterName, &svc, nil, true /* wantLb*/)
if err != nil {
t.Errorf("Unexpected error: %q", err)
}
Expand Down

0 comments on commit 9aeb005

Please sign in to comment.