skipper icon indicating copy to clipboard operation
skipper copied to clipboard

Use k8s server-side filtering for Ingresses

Open azarakovskiy opened this issue 2 years ago • 4 comments

Is your feature request related to a problem? Please describe.

Check additional context to understand better :)

Before we used a custom data client that doesn't get updated in runtime, and when on our dev environment Skipper app was idle under little traffic, and you could check in the metrics that it doesn't do much.

We are working to replace it with a K8S client that would use Ingress templates.

We enabled a K8S client, and the CPU usage went up a lot. It started to constantly do something even without any traffic. By looking in code, we concluded it's func (c *clusterClient) fetchClusterState() being called often. It loads all Ingresses/Services/Endpoints/Secrets, parses all of them to a data struct, and filters some of them out.

This sounds to me like a very CPU-intensive thing to do and it explains what Skipper is busy with -- loading it all again and again.

Are we doing something wrong?

Describe the solution you would like

Ingresses are filtered by IngressClass after all of them are loaded. Also, in K8S API almost every read endpoint supports labelSelector, including .../ingresses.

Since the Ingress class can be set to ingress templates, we can also set any labels, and then use server-side filtering. Is there any special reason why this is not done is Skipper? Why is everything loaded?

Services/Endpoints/Secrets would get the same filtering options, in that case, I guess. Details of how exactly it should work are not yet defined

Describe alternatives you've considered (optional)

Additional context (optional)

We are running a Skipper deployment in a k8s cluster A. It is a custom application that uses Skipper as a go dependency, and we have built some logic on top of it. It routes traffic to a couple of other clusters B & C, and also to A.

Currently, the definition of all routes that we have in a system lives in a JSON file which is generated externally, committed to the repository from time to time, read at a start of a pod, and fed to a custom data client. We want to replace it with Ingress templates, and Skipper to consume them.

So we started to look at the K8S data client that is already in Skipper.

Would you like to work on it? Yes

azarakovskiy avatar Jul 27 '22 13:07 azarakovskiy

@azarakovskiy we poll all resources even if we filter ingressClass, because of simplicity. Keep also in mind the v1 ingress with v1 ingressClass is different than the annotation from the past. Our current solution is to run a few routesrv and query these from skipper-ingress to offload traffic from kube-apiserver. https://github.com/zalando/skipper/tree/master/cmd/routesrv

szuecs avatar Jul 27 '22 18:07 szuecs

@szuecs thanks, we had the similar option in mind too but decided to first verify if we could optimize the querying somehow.

I am currently playing with a POC that solves our specific issues. I am wondering if this would be something you would consider merging into Skipper. Of course, after adjusting it to serve a more generic purpose.


The POC works as following:

We have different dev teams opting in to receive traffic via Skipper. To do that, they generate an Ingress template

apiVersion: networking.k8s.io/v1
kind: Ingress
metadata:
  name: test-skipper-ingress-staging  
  labels:
    gateway/route-spec: my-team  # this is new
  annotations:    
    kubernetes.io/ingress.class: skipper
    zalando.org/skipper-filter: setPath("/some-test-endpoint")
  
spec:
  rules:
  - http:
      paths:
      - backend:
          service:
            name: my-service
            port:
              number: 80
        path: /hello-world
        pathType: ImplementationSpecific

The template has a custom label which is set in the k8s options struct, and the value in a template is the name of the team my-team.

kubernetes.Options{
    ...
    IngressLabel:    "gateway/route-spec",
    ...
})

When a cluster client is created, in case IngressLabel is set, it appends a selector param to IngressURI

if o.IngressLabel != "" {
	ingressURI = fmt.Sprintf("%s%s", ingressURI, "?labelSelector="+o.IngressLabel)
}

Now only Ingresses that have the label will be loaded in fetchClusterState() We collect all the values of that label in the ingresses list

teams := []string{}
for _, ingressItem := range ingresses {
	teams = append(teams, ingressItem.Metadata.Labels[c.ingressLabel])
}
teamsStr := ""
if len(teams) > 0 {
	teamsStr = fmt.Sprintf("team in (%s)", strings.Join(teams, ","))
}

And then add teamStr to URI when we query services/endpoint/etc if teamsStr is not empty.

c.getJSON(fmt.Sprintf("%s%s", c.servicesURI, teamsStr), &services)

Now only resources that we know should be loaded for that Ingress are loaded because they all have a label with a value we set in Ingress.

apiVersion: v1
kind: Service
metadata:
  labels:
    ...
    team: my-team
    ...
spec:
  ...

The value of that label should probably allow building any LabelSelector query. I will need to give it a proper thought, now it's solving a very specific issue for the POC.

In the end, users would be able to tell Skipper "if the label is set, load templates with that label only, the label has instructions of what services/endpoints to load to resolve routes"

Do you think this could be a nice feature to have in Skipper?

azarakovskiy avatar Jul 28 '22 08:07 azarakovskiy

If we just check what HTTP REST would say from client API and filtering point of view, it sounds like a legit feature. On the other hand Kubernetes is having different ideas with this ingress class, but they also have the madness of client-go and informers. In my point of view it sounds like a nice and clean way to achieve what you want. So good to go from my side.

@AlexanderYastrebov when you have time (vacation time) please check this and tell if you think also that this would be great to add this as a feature.

szuecs avatar Jul 28 '22 21:07 szuecs

I made a draft PR to showcase the implementation. I'm still in progress with it (have some issues with the tests :( ) but accidentally opened it in the main repo instead of one in my own fork :shame:

https://github.com/zalando/skipper/pull/2048

The implementation is slightly different compared to my message above.

K8S data client can now receive a map of labels that must be set on a resource to load during routes update.

map[string]string {
  "skipper-enabled": "",
}

map[string]string {
  "skipper-enabled": "true",
}

map[string]string {
  "skipper-enabled": "true",
  "foo": "bar"
}

A similar map can be set for any kind of resource. If skipped, defaults to the one in Ingress.

If set, it limits the number of resources returned by the server. If nothing is set, everything is loaded.


Usage Say, you have a team that wants to opt-in their ingress to Skipper, but other teams don't want that. This team sets a label "skipper-enabled": "true"`, and your Skipper is already configured to load only resources with this label. This way it load this ingress and other stuff but doesn't load anything else from any team that doesn't want that. 🪄

azarakovskiy avatar Aug 02 '22 15:08 azarakovskiy