Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: refactor backend http setting port resolution #1090

Merged
merged 3 commits into from
Jan 22, 2021
Merged

fix: refactor backend http setting port resolution #1090

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c782d2a
refactor backend http setting port resolution
akshaysngupta Dec 14, 2020
87356e9
order
akshaysngupta Dec 16, 2020
6976288
add comments and augment test
akshaysngupta Jan 22, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
235 changes: 118 additions & 117 deletions pkg/appgw/backendhttpsettings.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,9 @@ import (

n "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2020-05-01/network"
"github.com/Azure/go-autorest/autorest/to"
"k8s.io/klog/v2"
v1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/util/intstr"
"k8s.io/klog/v2"

"github.com/Azure/application-gateway-kubernetes-ingress/pkg/annotations"
"github.com/Azure/application-gateway-kubernetes-ingress/pkg/brownfield"
Expand Down Expand Up @@ -73,148 +73,149 @@ func (c *appGwConfigBuilder) getBackendsAndSettingsMap(cbCtx *ConfigBuilderConte
return *c.mem.settings, *c.mem.settingsByBackend, *c.mem.serviceBackendPairsByBackend, nil
}

serviceBackendPairsMap := make(map[backendIdentifier]map[serviceBackendPortPair]interface{})
defaultHTTPSetting := defaultBackendHTTPSettings(c.appGwIdentifier, n.HTTP)
serviceBackendPairMap := make(map[backendIdentifier]serviceBackendPortPair)
backendHTTPSettingsMap := make(map[backendIdentifier]*n.ApplicationGatewayBackendHTTPSettings)
finalServiceBackendPairMap := make(map[backendIdentifier]serviceBackendPortPair)

var unresolvedBackendID []backendIdentifier
httpSettingsCollection := make(map[string]n.ApplicationGatewayBackendHTTPSettings)
httpSettingsCollection[*defaultHTTPSetting.Name] = defaultHTTPSetting
for backendID := range c.newBackendIdsFiltered(cbCtx) {
resolvedBackendPorts := make(map[serviceBackendPortPair]interface{})

service := c.k8sContext.GetService(backendID.serviceKey())
if service == nil {
// This should never happen since newBackendIdsFiltered() already filters out backends for non-existent Services
logLine := fmt.Sprintf("Unable to get the service [%s]", backendID.serviceKey())
c.recorder.Event(backendID.Ingress, v1.EventTypeWarning, events.ReasonServiceNotFound, logLine)
klog.Errorf(logLine)
pair := serviceBackendPortPair{
ServicePort: Port(backendID.Backend.ServicePort.IntVal),
BackendPort: Port(backendID.Backend.ServicePort.IntVal),
}
resolvedBackendPorts[pair] = nil
} else {
for _, sp := range service.Spec.Ports {
// find the backend port number
// check if any service ports matches the specified ports
if sp.Protocol != v1.ProtocolTCP {
// ignore UDP ports
continue
}
if fmt.Sprint(sp.Port) == backendID.Backend.ServicePort.String() ||
sp.Name == backendID.Backend.ServicePort.String() ||
sp.TargetPort.String() == backendID.Backend.ServicePort.String() {
// matched a service port with a port from the service

if sp.TargetPort.String() == "" {
// targetPort is not defined, by default targetPort == port
pair := serviceBackendPortPair{
ServicePort: Port(sp.Port),
BackendPort: Port(sp.Port),
}
resolvedBackendPorts[pair] = nil
} else {
// target port is defined as name or port number
if sp.TargetPort.Type == intstr.Int {
// port is defined as port number
pair := serviceBackendPortPair{
ServicePort: Port(sp.Port),
BackendPort: Port(sp.TargetPort.IntVal),
}
resolvedBackendPorts[pair] = nil
} else {
// if service port is defined by name, need to resolve
klog.V(5).Infof("resolving port name [%s] for service [%s] and service port [%s] for Ingress [%s]", sp.Name, backendID.serviceKey(), backendID.Backend.ServicePort.String(), backendID.Ingress.Name)

// k8s matches service port name against endpoints port name retrieved by passing backendID service key to endpoint api.
targetPortsResolved := c.resolvePortName(sp.Name, &backendID)
for targetPort := range targetPortsResolved {
pair := serviceBackendPortPair{
ServicePort: Port(sp.Port),
BackendPort: Port(targetPort),
}
resolvedBackendPorts[pair] = nil
}
}
}
break
}
}
backendPort, err := c.resolveBackendPort(backendID)
if err != nil {
c.recorder.Event(backendID.Ingress, v1.EventTypeWarning, events.ReasonPortResolutionError, err.Error())
klog.Error(err.Error())
}

if len(resolvedBackendPorts) == 0 {
logLine := fmt.Sprintf("unable to resolve any backend port for service [%s] and service port [%s] for Ingress [%s]", backendID.serviceKey(), backendID.Backend.ServicePort.String(), backendID.Ingress.Name)
c.recorder.Event(backendID.Ingress, v1.EventTypeWarning, events.ReasonPortResolutionError, logLine)
klog.Error(logLine)
httpSettings := c.generateHTTPSettings(backendID, backendPort, cbCtx)
klog.Infof("Created backend http settings %s for ingress %s/%s and service %s", *httpSettings.Name, backendID.Ingress.Namespace, backendID.Ingress.Name, backendID.serviceKey())

unresolvedBackendID = append(unresolvedBackendID, backendID)
break
// TODO(aksgupta): Only backend port is used in the output; remove service port.
serviceBackendPairMap[backendID] = serviceBackendPortPair{
ServicePort: backendPort,
BackendPort: backendPort,
}

// Merge serviceBackendPairsMap[backendID] into resolvedBackendPorts
if _, ok := serviceBackendPairsMap[backendID]; !ok {
serviceBackendPairsMap[backendID] = make(map[serviceBackendPortPair]interface{})
}
for portPair := range resolvedBackendPorts {
serviceBackendPairsMap[backendID][portPair] = nil
}
httpSettingsCollection[*httpSettings.Name] = httpSettings
backendHTTPSettingsMap[backendID] = &httpSettings
}

if len(unresolvedBackendID) > 0 {
klog.Warningf("Unable to resolve %d backends: %+v", len(unresolvedBackendID), unresolvedBackendID)
httpSettings := make([]n.ApplicationGatewayBackendHTTPSettings, 0, len(httpSettingsCollection))
for _, backend := range httpSettingsCollection {
httpSettings = append(httpSettings, backend)
}

httpSettingsCollection := make(map[string]n.ApplicationGatewayBackendHTTPSettings)
defaultBackend := defaultBackendHTTPSettings(c.appGwIdentifier, n.HTTP)
httpSettingsCollection[*defaultBackend.Name] = defaultBackend
c.mem.settings = &httpSettings
c.mem.settingsByBackend = &backendHTTPSettingsMap
c.mem.serviceBackendPairsByBackend = &serviceBackendPairMap
return httpSettings, backendHTTPSettingsMap, serviceBackendPairMap, nil
}

// enforce single pair relationship between service port and backend port
for backendID, serviceBackendPairs := range serviceBackendPairsMap {
func (c *appGwConfigBuilder) resolveBackendPort(backendID backendIdentifier) (Port, error) {
var e error
service := c.k8sContext.GetService(backendID.serviceKey())
if service == nil {
// This should never happen since newBackendIdsFiltered() already filters out backends for non-existent Services
e = controllererrors.NewErrorf(
controllererrors.ErrorServiceNotFound,
"Service not found %s",
backendID.serviceKey())

backendPort := Port(80)
if backendID.Backend.ServicePort.Type == intstr.Int && backendID.Backend.ServicePort.IntVal < 65536 {
backendPort = Port(backendID.Backend.ServicePort.IntVal)
}

// in case there are multiple backend ports found, using the smallest port in http setting
var backendport Port
backendport = 65536
return backendPort, e
}

// this will store all the ports found
var ports []string
// find the target port number for service port specified in the ingress manifest
servicePortInIngress := backendID.Backend.ServicePort.String()
resolvedBackendPorts := make(map[serviceBackendPortPair]interface{})
for _, servicePort := range service.Spec.Ports {
// ignore UDP ports
if servicePort.Protocol != v1.ProtocolTCP {
continue
}

// match service by either port, port name or target port
if fmt.Sprint(servicePort.Port) != servicePortInIngress &&
servicePort.Name != servicePortInIngress &&
servicePort.TargetPort.String() != servicePortInIngress {
continue
}

var uniquePair serviceBackendPortPair
for k := range serviceBackendPairs {
ports = append(ports, fmt.Sprintf("%d", k.BackendPort))
if k.BackendPort <= backendport {
uniquePair = k
backendport = k.BackendPort
// if target port is not specified, use the service port as backend port
if servicePort.TargetPort.String() == "" {
// targetPort is not defined, by default targetPort == port
pair := serviceBackendPortPair{
ServicePort: Port(servicePort.Port),
BackendPort: Port(servicePort.Port),
}
resolvedBackendPorts[pair] = nil
continue
}

if len(serviceBackendPairs) > 1 {
// if target port is an int, use it as backend port
if servicePort.TargetPort.Type == intstr.Int {
// port is defined as port number
pair := serviceBackendPortPair{
ServicePort: Port(servicePort.Port),
BackendPort: Port(servicePort.TargetPort.IntVal),
}
resolvedBackendPorts[pair] = nil
continue
}

// more than one possible backend port exposed through ingress
e := controllererrors.NewErrorf(
controllererrors.ErrorMultipleServiceBackendPortBinding,
"service:port [%s:%s] has more than one service-backend port binding which is not an ideal scenario, choosing the smallest service-backend port %d. Ports found %s.",
backendID.serviceKey(), backendID.Backend.ServicePort.String(), backendport, strings.Join(ports, ","),
)
// if target port is port name, then resolve the port number for the port name
// k8s matches service port name against endpoints port name retrieved by passing backendID service key to endpoint api.
klog.V(5).Infof("resolving port name '%s' for service '%s' and service port '%s' for Ingress '%s'", servicePort.Name, backendID.serviceKey(), backendID.Backend.ServicePort.String(), backendID.Ingress.Name)
targetPortsResolved := c.resolvePortName(servicePort.Name, &backendID)
for targetPort := range targetPortsResolved {
pair := serviceBackendPortPair{
ServicePort: Port(servicePort.Port),
BackendPort: Port(targetPort),
}
resolvedBackendPorts[pair] = nil
}
}

c.recorder.Event(backendID.Ingress, v1.EventTypeWarning, events.ReasonPortResolutionError, e.Error())
klog.Errorf(e.Error())
backendPort := Port(65536)
if len(resolvedBackendPorts) == 0 {
e = controllererrors.NewErrorf(
controllererrors.ErrorUnableToResolveBackendPortFromServicePort,
"No port matched %s",
backendID.serviceKey())
if backendID.Backend.ServicePort.Type == intstr.Int {
backendPort = Port(backendID.Backend.ServicePort.IntVal)
}
} else {
var ports []string
for k := range resolvedBackendPorts {
ports = append(ports, string(k.BackendPort))
if k.BackendPort <= backendPort {
backendPort = k.BackendPort
}
}

finalServiceBackendPairMap[backendID] = uniquePair
httpSettings := c.generateHTTPSettings(backendID, uniquePair.BackendPort, cbCtx)
klog.V(5).Infof("Created backend http settings %s for ingress %s/%s and service %s", *httpSettings.Name, backendID.Ingress.Namespace, backendID.Ingress.Name, backendID.serviceKey())
httpSettingsCollection[*httpSettings.Name] = httpSettings
backendHTTPSettingsMap[backendID] = &httpSettings
if len(resolvedBackendPorts) > 1 {
// found more than 1 backend port for the service port which is a conflicting scenario
e = controllererrors.NewErrorf(
controllererrors.ErrorMultipleServiceBackendPortBinding,
"service %s and service port %s has more than one service-backend port binding which is not an ideal scenario, choosing the smallest service-backend port %d. Ports found %s.",
backendID.serviceKey(), backendID.Backend.ServicePort.String(), backendPort, strings.Join(ports, ","),
)
}
}

httpSettings := make([]n.ApplicationGatewayBackendHTTPSettings, 0, len(httpSettingsCollection))
for _, backend := range httpSettingsCollection {
httpSettings = append(httpSettings, backend)
if backendPort >= Port(65536) {
e = controllererrors.NewErrorf(
controllererrors.ErrorServiceResolvedToInvalidPort,
"service %s and service port %s was resolved to an invalid service-backend port %d, defaulting to port 80",
backendID.serviceKey(), backendID.Backend.ServicePort.String(), backendPort,
)
backendPort = Port(80)
}

c.mem.settings = &httpSettings
c.mem.settingsByBackend = &backendHTTPSettingsMap
c.mem.serviceBackendPairsByBackend = &finalServiceBackendPairMap
return httpSettings, backendHTTPSettingsMap, finalServiceBackendPairMap, nil
return backendPort, e
}

func (c *appGwConfigBuilder) generateHTTPSettings(backendID backendIdentifier, port Port, cbCtx *ConfigBuilderContext) n.ApplicationGatewayBackendHTTPSettings {
Expand Down
49 changes: 47 additions & 2 deletions pkg/appgw/backendhttpsettings_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,18 +32,22 @@ var _ = Describe("Test the creation of Backend http settings from Ingress defini
// contains endpoint for the service ports. Multiple ports present with name as https-port// contains endpoint for the service ports. Multiple ports present with name as https-port
endpoint := tests.NewEndpointsFixtureWithSameNameMultiplePorts()

// service contains multiple service ports with port as 80, 443, etc and target port as 9876, pod port name
// service "--service-name--" contains multiple service ports with port as 80, 443, etc and target port as 9876, pod port name
service := tests.NewServiceFixture(*tests.NewServicePortsFixture()...)

pod := tests.NewPodTestFixture(service.Namespace, "mybackend")

// Ingress contains two rules with service port as 80 and 443
// Ingress "--name--" contains two rules with service port as 80 and 443
ingress := tests.NewIngressFixture()
_ = configBuilder.k8sContext.Caches.Pods.Add(&pod)
_ = configBuilder.k8sContext.Caches.Endpoints.Add(endpoint)
_ = configBuilder.k8sContext.Caches.Service.Add(service)
_ = configBuilder.k8sContext.Caches.Ingress.Add(ingress)

// Ingress "ingress-with-missing-service-and-service-with-invalid-port" with service missing "missing-service"
ingressWithInvalidServices, _ := tests.GetIngressWithMissingServiceAndServiceWithInvalidPort()
_ = configBuilder.k8sContext.Caches.Ingress.Add(ingressWithInvalidServices)

Context("test backend protocol annotation configures protocol on httpsettings and probes when no readiness probe on the pods", func() {

// checkBackendProtocolAnnotation function calls generates backend http settings map
Expand Down Expand Up @@ -165,4 +169,45 @@ var _ = Describe("Test the creation of Backend http settings from Ingress defini
}
})
})

Context("make sure all backends are processed", func() {
// ingress1 : Ingress "ingress-with-missing-service-and-service-with-invalid-port" with service missing "missing-service"
cbCtx := &ConfigBuilderContext{
IngressList: []*v1beta1.Ingress{ingressWithInvalidServices, ingress},
ServiceList: []*v1.Service{service},
DefaultAddressPoolID: to.StringPtr("xx"),
DefaultHTTPSettingsID: to.StringPtr("yy"),
}

configBuilder.mem = memoization{}
configBuilder.newProbesMap(cbCtx)
httpSettings, _, _, _ := configBuilder.getBackendsAndSettingsMap(cbCtx)

It("should configure all the backends even when a service is missing", func() {
expectedhttpSettingsLen := 5
Expect(expectedhttpSettingsLen).To(Equal(len(httpSettings)), "httpSetting count %d should be %d", len(httpSettings), expectedhttpSettingsLen)

for _, setting := range httpSettings {
if *setting.Name == DefaultBackendHTTPSettingsName {
Expect(int32(80)).To(Equal(*setting.Port), "default backend port %d should be 80", *setting.Port)
} else if strings.Contains(*setting.Name, strconv.Itoa(int(tests.ContainerPort))) {
// http setting for ingress with service port as 80
Expect(tests.ContainerPort).To(Equal(*setting.Port), "setting %s backend port %d should be 9876", *setting.Name, *setting.Port)
} else if strings.Contains(*setting.Name, "75") {
// http setting for the ingress with service port as 443. Target port is https-port which resolves to multiple backend port
// and the smallest backend port is chosen
Expect(int32(75)).To(Equal(*setting.Port), "setting %s backend port %d should be 75", *setting.Name, *setting.Port)
} else if strings.Contains(*setting.Name, "bp---namespace---missing-service-8080-8080-ingress-with-invalid-services") {
// http setting for missing service
Expect(int32(8080)).To(Equal(*setting.Port), "setting %s backend port %d should be 8080", *setting.Name, *setting.Port)
} else if strings.Contains(*setting.Name, "bp---namespace-----service-name---70000-80-ingress-with-invalid-services") {
// http setting for service with invalid port
Expect(int32(80)).To(Equal(*setting.Port), "setting %s backend port %d should be 80", *setting.Name, *setting.Port)
} else {
// Dummy Failure, This should not happen
Expect(23).To(Equal(75), "setting %s is not expected to be created", *setting.Name)
}
}
})
})
})
33 changes: 18 additions & 15 deletions pkg/controllererrors/errorcodes.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,21 +9,24 @@ package controllererrors
const (

// appgw package
ErrorMultipleServiceBackendPortBinding ErrorCode = "ErrorMultipleServiceBackendPortBinding"
ErrorGeneratingProbes ErrorCode = "ErrorGeneratingProbes"
ErrorGeneratingBackendSettings ErrorCode = "ErrorGeneratingBackendSettings"
ErrorCreatingBackendPools ErrorCode = "ErrorCreatingBackendPools"
ErrorGeneratingListeners ErrorCode = "ErrorGeneratingListeners"
ErrorGeneratingRoutingRules ErrorCode = "ErrorGeneratingRoutingRules"
ErrorNoDefaults ErrorCode = "ErrorNoDefaults"
ErrorEitherDefaults ErrorCode = "ErrorEitherDefaults"
ErrorNoBackendorRedirect ErrorCode = "ErrorNoBackendorRedirect"
ErrorEitherBackendorRedirect ErrorCode = "ErrorEitherBackendorRedirect"
ErrorNoPublicIP ErrorCode = "ErrorNoPublicIP"
ErrorNoPrivateIP ErrorCode = "ErrorNoPrivateIP"
ErrorEmptyConfig ErrorCode = "ErrorEmptyConfig"
ErrorIstioResolvePortsForServices ErrorCode = "ErrorIstioResolvePortsForServices"
ErrorIstioMultipleServiceBackendPortBinding ErrorCode = "ErrorIstioMultipleServiceBackendPortBinding"
ErrorServiceNotFound ErrorCode = "ErrorServiceNotFound"
ErrorMultipleServiceBackendPortBinding ErrorCode = "ErrorMultipleServiceBackendPortBinding"
ErrorUnableToResolveBackendPortFromServicePort ErrorCode = "ErrorUnableToResolveBackendPortFromServicePort"
ErrorServiceResolvedToInvalidPort ErrorCode = "ErrorServiceResolvedToInvalidPort"
ErrorGeneratingProbes ErrorCode = "ErrorGeneratingProbes"
ErrorGeneratingBackendSettings ErrorCode = "ErrorGeneratingBackendSettings"
ErrorCreatingBackendPools ErrorCode = "ErrorCreatingBackendPools"
ErrorGeneratingListeners ErrorCode = "ErrorGeneratingListeners"
ErrorGeneratingRoutingRules ErrorCode = "ErrorGeneratingRoutingRules"
ErrorNoDefaults ErrorCode = "ErrorNoDefaults"
ErrorEitherDefaults ErrorCode = "ErrorEitherDefaults"
ErrorNoBackendorRedirect ErrorCode = "ErrorNoBackendorRedirect"
ErrorEitherBackendorRedirect ErrorCode = "ErrorEitherBackendorRedirect"
ErrorNoPublicIP ErrorCode = "ErrorNoPublicIP"
ErrorNoPrivateIP ErrorCode = "ErrorNoPrivateIP"
ErrorEmptyConfig ErrorCode = "ErrorEmptyConfig"
ErrorIstioResolvePortsForServices ErrorCode = "ErrorIstioResolvePortsForServices"
ErrorIstioMultipleServiceBackendPortBinding ErrorCode = "ErrorIstioMultipleServiceBackendPortBinding"

// k8sContext package
ErrorEnpdointsNotFound ErrorCode = "ErrorEnpdointsNotFound"
Expand Down