Update reconcileSecurityGroup logic for Azure, add tests.

This commit is contained in:
Dong Liu 2017-03-22 13:27:33 +08:00
parent 4f44bf5e5a
commit 54664d08dd
2 changed files with 117 additions and 26 deletions

View File

@ -38,7 +38,7 @@ const ServiceAnnotationLoadBalancerInternal = "service.beta.kubernetes.io/azure-
// GetLoadBalancer returns whether the specified load balancer exists, and
// if so, what its status is.
func (az *Cloud) GetLoadBalancer(clusterName string, service *v1.Service) (status *v1.LoadBalancerStatus, exists bool, err error) {
isInternal := isLoadBalancerInternal(service)
isInternal := requiresInternalLoadBalancer(service)
lbName := getLoadBalancerName(clusterName, isInternal)
serviceName := getServiceName(service)
@ -113,7 +113,7 @@ func (az *Cloud) getPublicIPName(clusterName string, service *v1.Service) (strin
// EnsureLoadBalancer creates a new load balancer 'name', or updates the existing one. Returns the status of the balancer
func (az *Cloud) EnsureLoadBalancer(clusterName string, service *v1.Service, nodes []*v1.Node) (*v1.LoadBalancerStatus, error) {
isInternal := isLoadBalancerInternal(service)
isInternal := requiresInternalLoadBalancer(service)
lbName := getLoadBalancerName(clusterName, isInternal)
// When a client updates the internal load balancer annotation,
@ -128,7 +128,7 @@ func (az *Cloud) EnsureLoadBalancer(clusterName string, service *v1.Service, nod
if err != nil {
return nil, err
}
sg, sgNeedsUpdate, err := az.reconcileSecurityGroup(sg, clusterName, service)
sg, sgNeedsUpdate, err := az.reconcileSecurityGroup(sg, clusterName, service, true /* wantLb */)
if err != nil {
return nil, err
}
@ -266,7 +266,7 @@ func (az *Cloud) UpdateLoadBalancer(clusterName string, service *v1.Service, nod
// have multiple underlying components, meaning a Get could say that the LB
// doesn't exist even if some part of it is still laying around.
func (az *Cloud) EnsureLoadBalancerDeleted(clusterName string, service *v1.Service) error {
isInternal := isLoadBalancerInternal(service)
isInternal := requiresInternalLoadBalancer(service)
lbName := getLoadBalancerName(clusterName, isInternal)
serviceName := getServiceName(service)
@ -279,7 +279,7 @@ func (az *Cloud) EnsureLoadBalancerDeleted(clusterName string, service *v1.Servi
return err
}
if existsSg {
reconciledSg, sgNeedsUpdate, reconcileErr := az.reconcileSecurityGroup(sg, clusterName, service)
reconciledSg, sgNeedsUpdate, reconcileErr := az.reconcileSecurityGroup(sg, clusterName, service, false /* wantLb */)
if reconcileErr != nil {
return reconcileErr
}
@ -306,9 +306,6 @@ func (az *Cloud) ensureLoadBalancerDeleted(clusterName string, service *v1.Servi
glog.V(10).Infof("ensure lb deleted: clusterName=%q, serviceName=%s, lbName=%q", clusterName, serviceName, lbName)
// reconcile logic is capable of fully reconcile, so we can use this to delete
service.Spec.Ports = []v1.ServicePort{}
lb, existsLb, err := az.getAzureLoadBalancer(lbName)
if err != nil {
return err
@ -395,7 +392,7 @@ func (az *Cloud) ensurePublicIPDeleted(serviceName, pipName string) error {
// This also reconciles the Service's Ports with the LoadBalancer config.
// This entails adding rules/probes for expected Ports and removing stale rules/ports.
func (az *Cloud) reconcileLoadBalancer(lb network.LoadBalancer, fipConfigurationProperties *network.FrontendIPConfigurationPropertiesFormat, clusterName string, service *v1.Service, nodes []*v1.Node) (network.LoadBalancer, bool, error) {
isInternal := isLoadBalancerInternal(service)
isInternal := requiresInternalLoadBalancer(service)
lbName := getLoadBalancerName(clusterName, isInternal)
serviceName := getServiceName(service)
lbFrontendIPConfigName := getFrontendIPConfigName(service)
@ -403,7 +400,7 @@ func (az *Cloud) reconcileLoadBalancer(lb network.LoadBalancer, fipConfiguration
lbBackendPoolName := getBackendPoolName(clusterName)
lbBackendPoolID := az.getBackendPoolID(lbName, lbBackendPoolName)
wantLb := len(service.Spec.Ports) > 0
wantLb := fipConfigurationProperties != nil
dirtyLb := false
// Ensure LoadBalancer's Backend Pool Configuration
@ -473,9 +470,15 @@ func (az *Cloud) reconcileLoadBalancer(lb network.LoadBalancer, fipConfiguration
}
// update probes/rules
expectedProbes := make([]network.Probe, len(service.Spec.Ports))
expectedRules := make([]network.LoadBalancingRule, len(service.Spec.Ports))
for i, port := range service.Spec.Ports {
var ports []v1.ServicePort
if wantLb {
ports = service.Spec.Ports
} else {
ports = []v1.ServicePort{}
}
expectedProbes := make([]network.Probe, len(ports))
expectedRules := make([]network.LoadBalancingRule, len(ports))
for i, port := range ports {
lbRuleName := getRuleName(service, port)
transportProto, _, probeProto, err := getProtocolsFromKubernetesProtocol(port.Protocol)
@ -614,9 +617,14 @@ func (az *Cloud) reconcileLoadBalancer(lb network.LoadBalancer, fipConfiguration
// This reconciles the Network Security Group similar to how the LB is reconciled.
// This entails adding required, missing SecurityRules and removing stale rules.
func (az *Cloud) reconcileSecurityGroup(sg network.SecurityGroup, clusterName string, service *v1.Service) (network.SecurityGroup, bool, error) {
func (az *Cloud) reconcileSecurityGroup(sg network.SecurityGroup, clusterName string, service *v1.Service, wantLb bool) (network.SecurityGroup, bool, error) {
serviceName := getServiceName(service)
wantLb := len(service.Spec.Ports) > 0
var ports []v1.ServicePort
if wantLb {
ports = service.Spec.Ports
} else {
ports = []v1.ServicePort{}
}
sourceRanges, err := serviceapi.GetLoadBalancerSourceRanges(service)
if err != nil {
@ -624,15 +632,17 @@ func (az *Cloud) reconcileSecurityGroup(sg network.SecurityGroup, clusterName st
}
var sourceAddressPrefixes []string
if sourceRanges == nil || serviceapi.IsAllowAll(sourceRanges) {
sourceAddressPrefixes = []string{"Internet"}
if !requiresInternalLoadBalancer(service) {
sourceAddressPrefixes = []string{"Internet"}
}
} else {
for _, ip := range sourceRanges {
sourceAddressPrefixes = append(sourceAddressPrefixes, ip.String())
}
}
expectedSecurityRules := make([]network.SecurityRule, len(service.Spec.Ports)*len(sourceAddressPrefixes))
expectedSecurityRules := make([]network.SecurityRule, len(ports)*len(sourceAddressPrefixes))
for i, port := range service.Spec.Ports {
for i, port := range ports {
securityRuleName := getRuleName(service, port)
_, securityProto, _, err := getProtocolsFromKubernetesProtocol(port.Protocol)
if err != nil {
@ -799,8 +809,8 @@ func (az *Cloud) ensureHostInPool(serviceName string, nodeName types.NodeName, b
return nil
}
// Check if service requests an internal load balancer.
func isLoadBalancerInternal(service *v1.Service) bool {
// Check if service requires an internal load balancer.
func requiresInternalLoadBalancer(service *v1.Service) bool {
if l, ok := service.Annotations[ServiceAnnotationLoadBalancerInternal]; ok {
return l == "true"
}

View File

@ -87,6 +87,37 @@ func TestReconcileLoadBalancerNodeHealth(t *testing.T) {
}
// Test removing all services results in removing the frontend ip configuration
func TestReconcileLoadBalancerRemoveService(t *testing.T) {
az := getTestCloud()
svc := getTestService("servicea", 80, 443)
lb := getTestLoadBalancer()
configProperties := getTestPublicFipConfigurationProperties()
nodes := []*v1.Node{}
lb, updated, err := az.reconcileLoadBalancer(lb, &configProperties, testClusterName, &svc, nodes)
if err != nil {
t.Errorf("Unexpected error: %q", err)
}
validateLoadBalancer(t, lb, svc)
lb, updated, err = az.reconcileLoadBalancer(lb, nil, testClusterName, &svc, nodes)
if err != nil {
t.Errorf("Unexpected error: %q", err)
}
if !updated {
t.Error("Expected the loadbalancer to need an update")
}
// ensure we abandoned the frontend ip configuration
if len(*lb.FrontendIPConfigurations) != 0 {
t.Error("Expected the loadbalancer to have no frontend ip configuration")
}
validateLoadBalancer(t, lb)
}
// Test removing all service ports results in removing the frontend ip configuration
func TestReconcileLoadBalancerRemoveAllPortsRemovesFrontendConfig(t *testing.T) {
az := getTestCloud()
svc := getTestService("servicea", 80)
@ -98,6 +129,7 @@ func TestReconcileLoadBalancerRemoveAllPortsRemovesFrontendConfig(t *testing.T)
if err != nil {
t.Errorf("Unexpected error: %q", err)
}
validateLoadBalancer(t, lb, svc)
svcUpdated := getTestService("servicea")
lb, updated, err = az.reconcileLoadBalancer(lb, nil, testClusterName, &svcUpdated, nodes)
@ -164,7 +196,7 @@ func TestReconcileSecurityGroupNewServiceAddsPort(t *testing.T) {
sg := getTestSecurityGroup()
sg, _, err := az.reconcileSecurityGroup(sg, testClusterName, &svc1)
sg, _, err := az.reconcileSecurityGroup(sg, testClusterName, &svc1, true)
if err != nil {
t.Errorf("Unexpected error: %q", err)
}
@ -172,6 +204,36 @@ func TestReconcileSecurityGroupNewServiceAddsPort(t *testing.T) {
validateSecurityGroup(t, sg, svc1)
}
func TestReconcileSecurityGroupNewInternalServiceAddsPort(t *testing.T) {
az := getTestCloud()
svc1 := getInternalTestService("serviceea", 80)
sg := getTestSecurityGroup()
sg, _, err := az.reconcileSecurityGroup(sg, testClusterName, &svc1, true)
if err != nil {
t.Errorf("Unexpected error: %q", err)
}
validateSecurityGroup(t, sg, svc1)
}
func TestReconcileSecurityGroupRemoveService(t *testing.T) {
service1 := getTestService("servicea", 81)
service2 := getTestService("serviceb", 82)
sg := getTestSecurityGroup(service1, service2)
validateSecurityGroup(t, sg, service1, service2)
az := getTestCloud()
sg, _, err := az.reconcileSecurityGroup(sg, testClusterName, &service1, false)
if err != nil {
t.Errorf("Unexpected error: %q", err)
}
validateSecurityGroup(t, sg, service2)
}
func TestReconcileSecurityGroupRemoveServiceRemovesPort(t *testing.T) {
az := getTestCloud()
svc := getTestService("servicea", 80, 443)
@ -179,7 +241,7 @@ func TestReconcileSecurityGroupRemoveServiceRemovesPort(t *testing.T) {
sg := getTestSecurityGroup(svc)
svcUpdated := getTestService("servicea", 80)
sg, _, err := az.reconcileSecurityGroup(sg, testClusterName, &svcUpdated)
sg, _, err := az.reconcileSecurityGroup(sg, testClusterName, &svcUpdated, true)
if err != nil {
t.Errorf("Unexpected error: %q", err)
}
@ -196,7 +258,7 @@ func TestReconcileSecurityWithSourceRanges(t *testing.T) {
}
sg := getTestSecurityGroup(svc)
sg, _, err := az.reconcileSecurityGroup(sg, testClusterName, &svc)
sg, _, err := az.reconcileSecurityGroup(sg, testClusterName, &svc, true)
if err != nil {
t.Errorf("Unexpected error: %q", err)
}
@ -249,6 +311,14 @@ func getTestService(identifier string, requestedPorts ...int32) v1.Service {
svc.Name = identifier
svc.Namespace = "default"
svc.UID = types.UID(identifier)
svc.Annotations = make(map[string]string)
return svc
}
func getInternalTestService(identifier string, requestedPorts ...int32) v1.Service {
svc := getTestService(identifier, requestedPorts...)
svc.Annotations[ServiceAnnotationLoadBalancerInternal] = "true"
return svc
}
@ -288,8 +358,11 @@ func getTestLoadBalancer(services ...v1.Service) network.LoadBalancer {
func getServiceSourceRanges(service *v1.Service) []string {
if len(service.Spec.LoadBalancerSourceRanges) == 0 {
return []string{"Internet"}
if !requiresInternalLoadBalancer(service) {
return []string{"Internet"}
}
}
return service.Spec.LoadBalancerSourceRanges
}
@ -324,7 +397,11 @@ func getTestSecurityGroup(services ...v1.Service) network.SecurityGroup {
func validateLoadBalancer(t *testing.T, loadBalancer network.LoadBalancer, services ...v1.Service) {
expectedRuleCount := 0
expectedFrontendIPCount := 0
for _, svc := range services {
if len(svc.Spec.Ports) > 0 {
expectedFrontendIPCount++
}
for _, wantedRule := range svc.Spec.Ports {
expectedRuleCount++
wantedRuleName := getRuleName(&svc, wantedRule)
@ -371,6 +448,11 @@ func validateLoadBalancer(t *testing.T, loadBalancer network.LoadBalancer, servi
}
}
frontendIPCount := len(*loadBalancer.FrontendIPConfigurations)
if frontendIPCount != expectedFrontendIPCount {
t.Errorf("Expected the loadbalancer to have %d frontend IPs. Found %d.\n%v", expectedFrontendIPCount, frontendIPCount, loadBalancer.FrontendIPConfigurations)
}
lenRules := len(*loadBalancer.LoadBalancingRules)
if lenRules != expectedRuleCount {
t.Errorf("Expected the loadbalancer to have %d rules. Found %d.\n%v", expectedRuleCount, lenRules, loadBalancer.LoadBalancingRules)
@ -386,10 +468,9 @@ func validateSecurityGroup(t *testing.T, securityGroup network.SecurityGroup, se
for _, svc := range services {
for _, wantedRule := range svc.Spec.Ports {
sources := getServiceSourceRanges(&svc)
wantedRuleName := getRuleName(&svc, wantedRule)
for _, source := range sources {
expectedRuleCount++
wantedRuleName := getRuleName(&svc, wantedRule)
foundRule := false
for _, actualRule := range *securityGroup.SecurityRules {
if strings.EqualFold(*actualRule.Name, wantedRuleName) &&