Avoid potential race by getting all the resources needed to generate a new model at once

[ElvinEfendi]

syncIngress in controller.go currently is responsible of generating a new Nginx configuration for the given state of cluster. It does so by calling couple of children functions, n.getBackendServers being one of the most important one. getBackendServers. It first fetches the existing ingresses in the cluster at that time and stores them in a local variable and then getBackendServers iterates through them and generates backend servers. In every iteration it gets annotations of an ingress from the store. While this iteration happens store object can potentially change because of the events happening in the cluster. Imagine in the middle of the iteration the next to next ingress gets deleted. Store will get updated, but the corresponding ingres won't be deleted from the local variable and getBackendServers will still iterate over it. And when that happens it will fail to get its annotations because store won't have it. This results in issues such as #2494. When there are many ingresses in the cluster and controller pod is under pressure this issue becomes more visible because iteration slows down.

The issue shows itself with:

Error getting Ingress annotations "<namespace>/<ingress name>": no object matching key "<namespace>/<ingress name>" in local store	

In order to avoid this we should fetch all resources from store at once and only then start generating the model. In case of getBackendServers we can introduce something like

func (s k8sStore) ListIngressesAndAnnotations() ([]*extensions.Ingress, map[string]*annotations.Ingress)

and then use this function in getBackendServers instead of n.store.ListIngresses to get both ingresses and their annotations.

[aledbf]

@ElvinEfendi you mean something like https://github.com/kubernetes/ingress-nginx/compare/master...aledbf:model-at-once?expand=1 ?

[aledbf]

Maybe we should create another struct embedding extensions.Ingress and adding the annotations as part of the object, using only this new type, like

type Ingress type {
    extensions.Ingress
   ParsedAnnotations *annotations.Ingress
}

[ElvinEfendi]

you mean something like

yes @aledbf that's exactly what I meant

--

Introducing a new struct would do it as well - since there's only single extra attribute (ParsedAnnotations) at the moment I don't have any preference, either works.

[wayt]

I'll have a look into that.

I like the Ingress struct option.