Implementing rule-priority annotation. #1268

[cmendible]

trafficstars

Checklist

• [x] The title of the PR is clear and informative
• [ ] If applicable, the changes made in the PR have proper test coverage
• [x] Issues addressed by the PR are mentioned in the description followed by Fixes.

Description

Enables Request Routing Rule Priority support through the new appgw.ingress.kubernetes.io/rule-priority annotation.

Have not created tests yet, because first I want to get feedback about the current implementation.

Fixes

#1268

[akshaysngupta]

what happens if two ingresses have same priority?

[cmendible]

what happens if two ingresses have same priority?

Missed that one! I added the check for that case in getListenerPriorities --> 27dfb60

[ghost]

get this committed, without priority settings for basic HTTP and HTTPs listeners that routing will behave erratically, based on the configuration order as submitted to ARM.

If priority is enabled on 1 listener, priority must also be defined on all other listeners

[cmendible]

go vet is failing here:

cmd/appgw-ingress/main.go:228:2: misuse of unbuffered os.Signal channel as argument to signal.Notify

which is note related to this PR code.

[sgerace]

Any ETA on when we can expect this?

[sgerace]

Please note that this also resolves #884.

[christiaanderidder]

This change is currently blocking us from implementing AGIC for our product. Is it correct to assume this is just stuck waiting for a code review?

[ricardomomm]

I'm stuck here too, I have a route /api*, /ws* and a default /, only the default works

[cmendible]

@akshaysngupta can you take a look at this one?

Thanks!

[naioja]

I'm stuck here too, I have a route /api*, /ws* and a default /, only the default works

Have you looked at this:

apiVersion: networking.k8s.io/v1
kind: Ingress
metadata:
  name: go-server-ingress-affinity
  namespace: test-ag
  annotations:
    kubernetes.io/ingress.class: azure/application-gateway
    appgw.ingress.kubernetes.io/cookie-based-affinity: "true"
    appgw.ingress.kubernetes.io/cookie-based-affinity-distinct-name: "true"
spec:
  rules:
  - http:
      paths:
      - path: /affinity1/
        pathType: Exact
        backend:
          service:
            name: affinity-service
            port:
              number: 80
      - path: /affinity2/
        pathType: Exact
        backend:
          service:
            name: affinity-service
            port:
              number: 80

From : https://azure.github.io/application-gateway-kubernetes-ingress/annotations/#backend-path-prefix

[ricardomomm]

I'm stuck here too, I have a route /api*, /ws* and a default /, only the default works

Have you looked at this:

apiVersion: networking.k8s.io/v1
kind: Ingress
metadata:
  name: go-server-ingress-affinity
  namespace: test-ag
  annotations:
    kubernetes.io/ingress.class: azure/application-gateway
    appgw.ingress.kubernetes.io/cookie-based-affinity: "true"
    appgw.ingress.kubernetes.io/cookie-based-affinity-distinct-name: "true"
spec:
  rules:
  - http:
      paths:
      - path: /affinity1/
        pathType: Exact
        backend:
          service:
            name: affinity-service
            port:
              number: 80
      - path: /affinity2/
        pathType: Exact
        backend:
          service:
            name: affinity-service
            port:
              number: 80

From : https://azure.github.io/application-gateway-kubernetes-ingress/annotations/#backend-path-prefix

not sure how this can help me with the priority, all 3 routes that I have a in different ingresses, I manager to get it to work with the workaround provided in https://github.com/Azure/application-gateway-kubernetes-ingress/issues/884. But thanks anyway, my ws endpoint is signalR and I'll need affinity for redis scaleout

[2743d2]

I'm waiting for this annotation since it at least provide a workaround when using wildcast and exact host. and Prioritization by hashing is very confusing and can easily cause prod accidents.

foo.bar *.bar

[rennerg]

When can we expect to see this change available?

[ssrahul96]

+1

When can we expect to see this change available?

[zgqit]

+2

When can we expect to see this change available?

[manikg321]

When can we expect to see this change available?

[maxarndt]

@cmendible @akshaysngupta this feature is not compatible with appgw.ingress.kubernetes.io/ssl-redirect: "true" annotation because same priority is applied to http and https rule.

E0728 08:28:52.784935       1 controller.go:142] network.ApplicationGatewaysClient#CreateOrUpdate: Failure sending request: StatusCode=400 -- Original Error: Code="ApplicationGatewayRequestRoutingRulePriorityMustBeUnique" Message="Priority must be unique across all the request routing rules. Rules /subscriptions/123456789/resourceGroups/ReG_aks-some-cluster/providers/Microsoft.Network/applicationGateways/aks-some-cluster-app-gateway-1/requestRoutingRules/rr-51aea8d877768c8759ff3cda6ba516f4 and /subscriptions/123456789/resourceGroups/ReG_aks-some-cluster/providers/Microsoft.Network/applicationGateways/aks-some-cluster-app-gateway-1/requestRoutingRules/rr-9ffe3bf76fbc4d64f6671769450bcf69 cannot have the same priority 10000." Details=[]

[juanpgarces]

I'm not sure if the documentation is updated for this annotation. https://azure.github.io/application-gateway-kubernetes-ingress/annotations/

[Laykou]

@cmendible @akshaysngupta this feature is not compatible with appgw.ingress.kubernetes.io/ssl-redirect: "true" annotation because same priority is applied to http and https rule.

E0728 08:28:52.784935       1 controller.go:142] network.ApplicationGatewaysClient#CreateOrUpdate: Failure sending request: StatusCode=400 -- Original Error: Code="ApplicationGatewayRequestRoutingRulePriorityMustBeUnique" Message="Priority must be unique across all the request routing rules. Rules /subscriptions/123456789/resourceGroups/ReG_aks-some-cluster/providers/Microsoft.Network/applicationGateways/aks-some-cluster-app-gateway-1/requestRoutingRules/rr-51aea8d877768c8759ff3cda6ba516f4 and /subscriptions/123456789/resourceGroups/ReG_aks-some-cluster/providers/Microsoft.Network/applicationGateways/aks-some-cluster-app-gateway-1/requestRoutingRules/rr-9ffe3bf76fbc4d64f6671769450bcf69 cannot have the same priority 10000." Details=[]

@maxarndt Is this compatibility still a problem?

[maxarndt]

@Laykou yes, it is:

E0326 09:00:49.636303       1 controller.go:142] network.ApplicationGatewaysClient#CreateOrUpdate: Failure sending request: StatusCode=400 -- Original Error: Code="ApplicationGatewayRequestRoutingRulePriorityMustBeUnique" Message="Priority must be unique across all the request routing rules. Rules /subscriptions/123456/resourceGroups/ReG_foo-bar/providers/Microsoft.Network/applicationGateways/foo-bar-app-gateway-1/requestRoutingRules/rr-51aea8d877768c8759ff3cda6ba516f4 and /subscriptions/123456/resourceGroups/ReG_foo-bar/providers/Microsoft.Network/applicationGateways/foo-bar-app-gateway-1/requestRoutingRules/rr-9ffe3bf76fbc4d64f6671769450bcf69 cannot have the same priority 10000." Details=[]
E0326 09:00:49.636316       1 worker.go:62] Error processing event.network.ApplicationGatewaysClient#CreateOrUpdate: Failure sending request: StatusCode=400 -- Original Error: Code="ApplicationGatewayRequestRoutingRulePriorityMustBeUnique" Message="Priority must be unique across all the request routing rules. Rules /subscriptions/123456/resourceGroups/ReG_foo-bar/providers/Microsoft.Network/applicationGateways/foo-bar-app-gateway-1/requestRoutingRules/rr-51aea8d877768c8759ff3cda6ba516f4 and /subscriptions/123456/resourceGroups/ReG_foo-bar/providers/Microsoft.Network/applicationGateways/foo-bar-app-gateway-1/requestRoutingRules/rr-9ffe3bf76fbc4d64f6671769450bcf69 cannot have the same priority 10000." Details=[]

Currently we run v1.7.2 of this component. The latest version v1.7.3 does not seem to address this issue.

[Laykou]

I found this open issue already https://github.com/Azure/application-gateway-kubernetes-ingress/issues/1515 so we can move the further discussion there