diff --git a/pkg/cloudprovider/providers/openstack/openstack_loadbalancer.go b/pkg/cloudprovider/providers/openstack/openstack_loadbalancer.go index 98b5b44ed77..471fdc31a61 100644 --- a/pkg/cloudprovider/providers/openstack/openstack_loadbalancer.go +++ b/pkg/cloudprovider/providers/openstack/openstack_loadbalancer.go @@ -292,8 +292,14 @@ func popMember(members []v2pools.Member, addr string, port int) []v2pools.Member return members } -func getSecurityGroupName(clusterName string, service *v1.Service) string { - return fmt.Sprintf("lb-sg-%s-%s-%s", clusterName, service.Namespace, service.Name) +func getSecurityGroupName(service *v1.Service) string { + securityGroupName := fmt.Sprintf("lb-sg-%s-%s-%s", service.UID, service.Namespace, service.Name) + //OpenStack requires that the name of a security group is shorter than 255 bytes. + if len(securityGroupName) > 255 { + securityGroupName = securityGroupName[:255] + } + + return securityGroupName } func getSecurityGroupRules(client *gophercloud.ServiceClient, opts rules.ListOpts) ([]rules.SecGroupRule, error) { @@ -868,6 +874,14 @@ func (lbaas *LbaasV2) EnsureLoadBalancer(clusterName string, apiService *v1.Serv _ = lbaas.EnsureLoadBalancerDeleted(clusterName, apiService) return status, err } + + // delete the old Security Group for the service + // Related to #53764 + // TODO(FengyunPan): Remove it at V1.10 + err = lbaas.EnsureOldSecurityGroupDeleted(clusterName, apiService) + if err != nil { + return status, fmt.Errorf("Failed to delete the Security Group for loadbalancer service %s/%s: %v", apiService.Namespace, apiService.Name, err) + } } return status, nil @@ -899,7 +913,7 @@ func (lbaas *LbaasV2) ensureSecurityGroup(clusterName string, apiService *v1.Ser } // ensure security group for LB - lbSecGroupName := getSecurityGroupName(clusterName, apiService) + lbSecGroupName := getSecurityGroupName(apiService) lbSecGroupID, err := groups.IDFromName(lbaas.network, lbSecGroupName) if err != nil { // check whether security group does not exist @@ -914,8 +928,8 @@ func (lbaas *LbaasV2) ensureSecurityGroup(clusterName string, apiService *v1.Ser if len(lbSecGroupID) == 0 { // create security group lbSecGroupCreateOpts := groups.CreateOpts{ - Name: getSecurityGroupName(clusterName, apiService), - Description: fmt.Sprintf("Securty Group for loadbalancer service %s/%s", apiService.Namespace, apiService.Name), + Name: getSecurityGroupName(apiService), + Description: fmt.Sprintf("Security Group for %s/%s Service LoadBalancer in cluster %s", apiService.Namespace, apiService.Name, clusterName), } lbSecGroup, err := groups.Create(lbaas.network, lbSecGroupCreateOpts).Extract() @@ -1174,7 +1188,7 @@ func (lbaas *LbaasV2) UpdateLoadBalancer(clusterName string, service *v1.Service if lbaas.opts.ManageSecurityGroups { err := lbaas.updateSecurityGroup(clusterName, service, nodes, loadbalancer) if err != nil { - return fmt.Errorf("failed to update Securty Group for loadbalancer service %s/%s: %v", service.Namespace, service.Name, err) + return fmt.Errorf("failed to update Security Group for loadbalancer service %s/%s: %v", service.Namespace, service.Name, err) } } @@ -1197,7 +1211,7 @@ func (lbaas *LbaasV2) updateSecurityGroup(clusterName string, apiService *v1.Ser removals := original.Difference(current) // Generate Name - lbSecGroupName := getSecurityGroupName(clusterName, apiService) + lbSecGroupName := getSecurityGroupName(apiService) lbSecGroupID, err := groups.IDFromName(lbaas.network, lbSecGroupName) if err != nil { return fmt.Errorf("error occurred finding security group: %s: %v", lbSecGroupName, err) @@ -1368,50 +1382,131 @@ func (lbaas *LbaasV2) EnsureLoadBalancerDeleted(clusterName string, service *v1. // Delete the Security Group if lbaas.opts.ManageSecurityGroups { - // Generate Name - lbSecGroupName := getSecurityGroupName(clusterName, service) - lbSecGroupID, err := groups.IDFromName(lbaas.network, lbSecGroupName) + err := lbaas.EnsureSecurityGroupDeleted(clusterName, service) if err != nil { - // check whether security group does not exist - _, ok := err.(*gophercloud.ErrResourceNotFound) - if ok { - // It is OK when the security group has been deleted by others. - return nil - } else { - return fmt.Errorf("error occurred finding security group: %s: %v", lbSecGroupName, err) - } + return fmt.Errorf("Failed to delete Security Group for loadbalancer service %s/%s: %v", service.Namespace, service.Name, err) } - lbSecGroup := groups.Delete(lbaas.network, lbSecGroupID) - if lbSecGroup.Err != nil && !isNotFound(lbSecGroup.Err) { - return lbSecGroup.Err + // delete the old Security Group for the service + // Related to #53764 + // TODO(FengyunPan): Remove it at V1.10 + err = lbaas.EnsureOldSecurityGroupDeleted(clusterName, service) + if err != nil { + return fmt.Errorf("Failed to delete the Security Group for loadbalancer service %s/%s: %v", service.Namespace, service.Name, err) } + } - if len(lbaas.opts.NodeSecurityGroupIDs) == 0 { - // Just happen when nodes have not Security Group, or should not happen - // UpdateLoadBalancer and EnsureLoadBalancer can set lbaas.opts.NodeSecurityGroupIDs when it is empty - // And service controller call UpdateLoadBalancer to set lbaas.opts.NodeSecurityGroupIDs when controller manager service is restarted. - glog.Warningf("Can not find node-security-group from all the nodes of this cluser when delete loadbalancer service %s/%s", - service.Namespace, service.Name) + return nil +} + +// EnsureSecurityGroupDeleted deleting security group for specific loadbalancer service. +func (lbaas *LbaasV2) EnsureSecurityGroupDeleted(clusterName string, service *v1.Service) error { + // Generate Name + lbSecGroupName := getSecurityGroupName(service) + lbSecGroupID, err := groups.IDFromName(lbaas.network, lbSecGroupName) + if err != nil { + // check whether security group does not exist + _, ok := err.(*gophercloud.ErrResourceNotFound) + if ok { + // It is OK when the security group has been deleted by others. + return nil } else { - // Delete the rules in the Node Security Group - for _, nodeSecurityGroupID := range lbaas.opts.NodeSecurityGroupIDs { - opts := rules.ListOpts{ - SecGroupID: nodeSecurityGroupID, - RemoteGroupID: lbSecGroupID, - } - secGroupRules, err := getSecurityGroupRules(lbaas.network, opts) + return fmt.Errorf("Error occurred finding security group: %s: %v", lbSecGroupName, err) + } + } - if err != nil && !isNotFound(err) { - msg := fmt.Sprintf("Error finding rules for remote group id %s in security group id %s: %v", lbSecGroupID, nodeSecurityGroupID, err) - return fmt.Errorf(msg) - } + lbSecGroup := groups.Delete(lbaas.network, lbSecGroupID) + if lbSecGroup.Err != nil && !isNotFound(lbSecGroup.Err) { + return lbSecGroup.Err + } - for _, rule := range secGroupRules { - res := rules.Delete(lbaas.network, rule.ID) - if res.Err != nil && !isNotFound(res.Err) { - return fmt.Errorf("error occurred deleting security group rule: %s: %v", rule.ID, res.Err) - } + if len(lbaas.opts.NodeSecurityGroupIDs) == 0 { + // Just happen when nodes have not Security Group, or should not happen + // UpdateLoadBalancer and EnsureLoadBalancer can set lbaas.opts.NodeSecurityGroupIDs when it is empty + // And service controller call UpdateLoadBalancer to set lbaas.opts.NodeSecurityGroupIDs when controller manager service is restarted. + glog.Warningf("Can not find node-security-group from all the nodes of this cluster when delete loadbalancer service %s/%s", + service.Namespace, service.Name) + } else { + // Delete the rules in the Node Security Group + for _, nodeSecurityGroupID := range lbaas.opts.NodeSecurityGroupIDs { + opts := rules.ListOpts{ + SecGroupID: nodeSecurityGroupID, + RemoteGroupID: lbSecGroupID, + } + secGroupRules, err := getSecurityGroupRules(lbaas.network, opts) + + if err != nil && !isNotFound(err) { + msg := fmt.Sprintf("Error finding rules for remote group id %s in security group id %s: %v", lbSecGroupID, nodeSecurityGroupID, err) + return fmt.Errorf(msg) + } + + for _, rule := range secGroupRules { + res := rules.Delete(lbaas.network, rule.ID) + if res.Err != nil && !isNotFound(res.Err) { + return fmt.Errorf("Error occurred deleting security group rule: %s: %v", rule.ID, res.Err) + } + } + } + } + + return nil +} + +// getOldSecurityGroupName is used to get the old security group name +// Related to #53764 +// TODO(FengyunPan): Remove it at V1.10 +func getOldSecurityGroupName(clusterName string, service *v1.Service) string { + return fmt.Sprintf("lb-sg-%s-%v", clusterName, service.Name) +} + +// EnsureOldSecurityGroupDeleted deleting old security group for specific loadbalancer service. +// Related to #53764 +// TODO(FengyunPan): Remove it at V1.10 +func (lbaas *LbaasV2) EnsureOldSecurityGroupDeleted(clusterName string, service *v1.Service) error { + glog.V(4).Infof("EnsureOldSecurityGroupDeleted(%v, %v)", clusterName, service) + // Generate Name + lbSecGroupName := getOldSecurityGroupName(clusterName, service) + lbSecGroupID, err := groups.IDFromName(lbaas.network, lbSecGroupName) + if err != nil { + // check whether security group does not exist + _, ok := err.(*gophercloud.ErrResourceNotFound) + if ok { + // It is OK when the security group has been deleted by others. + return nil + } else { + return fmt.Errorf("Error occurred finding security group: %s: %v", lbSecGroupName, err) + } + } + + lbSecGroup := groups.Delete(lbaas.network, lbSecGroupID) + if lbSecGroup.Err != nil && !isNotFound(lbSecGroup.Err) { + return lbSecGroup.Err + } + + if len(lbaas.opts.NodeSecurityGroupIDs) == 0 { + // Just happen when nodes have not Security Group, or should not happen + // UpdateLoadBalancer and EnsureLoadBalancer can set lbaas.opts.NodeSecurityGroupIDs when it is empty + // And service controller call UpdateLoadBalancer to set lbaas.opts.NodeSecurityGroupIDs when controller manager service is restarted. + glog.Warningf("Can not find node-security-group from all the nodes of this cluster when delete loadbalancer service %s/%s", + service.Namespace, service.Name) + } else { + // Delete the rules in the Node Security Group + for _, nodeSecurityGroupID := range lbaas.opts.NodeSecurityGroupIDs { + opts := rules.ListOpts{ + SecGroupID: nodeSecurityGroupID, + RemoteGroupID: lbSecGroupID, + } + secGroupRules, err := getSecurityGroupRules(lbaas.network, opts) + + if err != nil && !isNotFound(err) { + msg := fmt.Sprintf("Error finding rules for remote group id %s in security group id %s: %v", lbSecGroupID, nodeSecurityGroupID, err) + return fmt.Errorf(msg) + } + + for _, rule := range secGroupRules { + res := rules.Delete(lbaas.network, rule.ID) + if res.Err != nil && !isNotFound(res.Err) { + return fmt.Errorf("Error occurred deleting security group rule: %s: %v", rule.ID, res.Err) } } }