Skip to content

Commit

Permalink
MEDIUM: Don't skip any Ingress event
Browse files Browse the repository at this point in the history 
When an ingress resource is received from k8s, IC controller used to
verify if the resource is different from the one in the cache (if there
is any) before starting the sync method. This verification process is
complex and not easy to maintain and also seemed to result in some
missed information in the cache (like missing ingress rules in the
cached Ingress resource).
In this commit the verification has been removed which means an ingress
update from k8s will always trigger the sync method.
This will guarantee that we don't miss any update but also increases the
risk of triggering the sync method for no reason in case the k8s update
is irrelevant.
  • Loading branch information
@Mo3m3n
Mo3m3n committed Mar 15, 2022
1 parent cb71dd2 commit 07e10cd
Show file tree
Hide file tree
Showing 10 changed files with 37 additions and 420 deletions.
8 changes: 3 additions & 5 deletions controller/annotations/ingress/httpsRedirect.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,10 +90,8 @@ func tlsEnabled(ingress *store.Ingress) bool {
if ingress == nil {
return false
}
for _, tls := range ingress.TLS {
if tls.Status != store.DELETED {
return true
}
if len(ingress.TLS) == 0 {
return false
}
return false
return true
}
3 changes: 0 additions & 3 deletions controller/controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -160,9 +160,6 @@ func (c *HAProxyController) updateHAProxy() {
continue
}
for _, ingResource := range namespace.Ingresses {
if ingResource.Status == DELETED {
continue
}
i := ingress.New(c.Store, ingResource, c.OSArgs.IngressClass, c.OSArgs.EmptyIngressClass)
if i == nil {
logger.Debugf("ingress '%s/%s' ignored: no matching IngressClass", ingResource.Namespace, ingResource.Name)
Expand Down
8 changes: 1 addition & 7 deletions controller/ingress/ingress.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ func New(k store.K8s, resource *store.Ingress, class string, emptyClass bool) *I
func (i Ingress) supported(k8s store.K8s) (supported bool) {
var igClassAnn, igClassSpec string
igClassAnn = annotations.String("ingress.class", i.resource.Annotations)
if igClassResource := k8s.IngressClasses[i.resource.Class]; igClassResource != nil && igClassResource.Status != store.DELETED {
if igClassResource := k8s.IngressClasses[i.resource.Class]; igClassResource != nil {
igClassSpec = igClassResource.Controller
}

Expand Down Expand Up @@ -87,9 +87,6 @@ func (i *Ingress) handlePath(k store.K8s, cfg *configuration.ControllerCfg, api
if err != nil {
return
}
if path.Status == store.DELETED {
return
}
// Backend
backendReload, err := svc.HandleBackend(api, k)
if err != nil {
Expand Down Expand Up @@ -198,9 +195,6 @@ func (i *Ingress) Update(k store.K8s, cfg *configuration.ControllerCfg, api api.
// Ingress secrets
logger.Tracef("Ingress '%s/%s': processing secrets...", i.resource.Namespace, i.resource.Name)
for _, tls := range i.resource.TLS {
if tls.Status == store.DELETED {
continue
}
secret, secErr := k.GetSecret(i.resource.Namespace, tls.SecretName)
if secErr != nil {
logger.Warningf("Ingress '%s/%s': %s", i.resource.Namespace, i.resource.Name, secErr)
Expand Down
37 changes: 8 additions & 29 deletions controller/kubernetes.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -385,25 +385,15 @@ func (k *K8s) EventsIngressClass(channel chan SyncDataEvent, stop chan struct{},
channel <- SyncDataEvent{SyncType: INGRESS_CLASS, Data: item}
},
UpdateFunc: func(oldObj, newObj interface{}) {
item1, err := store.ConvertToIngressClass(oldObj)
if err != nil {
k.Logger.Errorf("%s: Invalid data from k8s api, %s", INGRESS_CLASS, oldObj)
return
}
item1.Status = MODIFIED

item2, err := store.ConvertToIngressClass(newObj)
item, err := store.ConvertToIngressClass(newObj)
if err != nil {
k.Logger.Errorf("%s: Invalid data from k8s api, %s", INGRESS, oldObj)
return
}
item1.Status = MODIFIED
item.Status = MODIFIED

if item2.Equal(item1) {
return
}
k.Logger.Tracef("%s %s: %s", INGRESS_CLASS, item2.Status, item2.Name)
channel <- SyncDataEvent{SyncType: INGRESS_CLASS, Data: item2}
k.Logger.Tracef("%s %s: %s", INGRESS_CLASS, item.Status, item.Name)
channel <- SyncDataEvent{SyncType: INGRESS_CLASS, Data: item}
},
},
)
Expand Down Expand Up @@ -433,25 +423,14 @@ func (k *K8s) EventsIngresses(channel chan SyncDataEvent, stop chan struct{}, in
channel <- SyncDataEvent{SyncType: INGRESS, Namespace: item.Namespace, Data: item}
},
UpdateFunc: func(oldObj, newObj interface{}) {
item1, err := store.ConvertToIngress(oldObj)
if err != nil {
k.Logger.Errorf("%s: Invalid data from k8s api, %s", INGRESS, oldObj)
return
}
item1.Status = MODIFIED

item2, err := store.ConvertToIngress(newObj)
item, err := store.ConvertToIngress(newObj)
if err != nil {
k.Logger.Errorf("%s: Invalid data from k8s api, %s", INGRESS, oldObj)
return
}
item1.Status = MODIFIED

if item2.Equal(item1) {
return
}
k.Logger.Tracef("%s %s: %s", INGRESS, item2.Status, item2.Name)
channel <- SyncDataEvent{SyncType: INGRESS, Namespace: item2.Namespace, Data: item2}
item.Status = MODIFIED
k.Logger.Tracef("%s %s: %s", INGRESS, item.Status, item.Name)
channel <- SyncDataEvent{SyncType: INGRESS, Namespace: item.Namespace, Data: item}
},
},
)
Expand Down
2 changes: 1 addition & 1 deletion controller/monitor.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ func (c *HAProxyController) SyncData() {
case NAMESPACE:
change = c.Store.EventNamespace(ns, job.Data.(*store.Namespace))
case INGRESS:
change = c.Store.EventIngress(ns, job.Data.(*store.Ingress), c.OSArgs.IngressClass)
change = c.Store.EventIngress(ns, job.Data.(*store.Ingress))
case INGRESS_CLASS:
change = c.Store.EventIngressClass(job.Data.(*store.IngressClass))
case ENDPOINTS:
Expand Down
54 changes: 6 additions & 48 deletions controller/store/convert.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,6 @@ func (n ingressNetworkingV1Beta1Strategy) ConvertIngress() *Ingress {
SvcName: k8sPath.Backend.ServiceName,
SvcPortInt: int64(k8sPath.Backend.ServicePort.IntValue()),
SvcPortString: k8sPath.Backend.ServicePort.StrVal,
Status: "",
}
}
if rule, ok := rules[k8sRule.Host]; ok {
Expand All @@ -119,9 +118,8 @@ func (n ingressNetworkingV1Beta1Strategy) ConvertIngress() *Ingress {
}
} else {
rules[k8sRule.Host] = &IngressRule{
Host: k8sRule.Host,
Paths: paths,
Status: "",
Host: k8sRule.Host,
Paths: paths,
}
}
}
Expand All @@ -137,7 +135,6 @@ func (n ingressNetworkingV1Beta1Strategy) ConvertIngress() *Ingress {
SvcPortInt: int64(ingressBackend.ServicePort.IntValue()),
SvcPortString: ingressBackend.ServicePort.StrVal,
IsDefaultBackend: true,
Status: "",
}
}(n.ig.Spec.Backend),
TLS: func(ingressTLS []networkingv1beta1.IngressTLS) map[string]*IngressTLS {
Expand All @@ -147,18 +144,11 @@ func (n ingressNetworkingV1Beta1Strategy) ConvertIngress() *Ingress {
tls[host] = &IngressTLS{
Host: host,
SecretName: k8sTLS.SecretName,
Status: EMPTY,
}
}
}
return tls
}(n.ig.Spec.TLS),
Status: func() Status {
if n.ig.ObjectMeta.GetDeletionTimestamp() != nil {
return DELETED
}
return ADDED
}(),
}
}

Expand All @@ -167,12 +157,6 @@ func (n ingressNetworkingV1Beta1Strategy) ConvertClass() *IngressClass {
APIVersion: NETWORKINGV1BETA1,
Name: n.class.GetName(),
Controller: n.class.Spec.Controller,
Status: func() Status {
if n.class.ObjectMeta.GetDeletionTimestamp() != nil {
return DELETED
}
return ADDED
}(),
}
}

Expand Down Expand Up @@ -208,7 +192,6 @@ func (e ingressExtensionsStrategy) ConvertIngress() *Ingress {
SvcName: k8sPath.Backend.ServiceName,
SvcPortInt: int64(k8sPath.Backend.ServicePort.IntValue()),
SvcPortString: k8sPath.Backend.ServicePort.StrVal,
Status: "",
}
}
if rule, ok := rules[k8sRule.Host]; ok {
Expand All @@ -217,9 +200,8 @@ func (e ingressExtensionsStrategy) ConvertIngress() *Ingress {
}
} else {
rules[k8sRule.Host] = &IngressRule{
Host: k8sRule.Host,
Paths: paths,
Status: "",
Host: k8sRule.Host,
Paths: paths,
}
}
}
Expand All @@ -235,7 +217,6 @@ func (e ingressExtensionsStrategy) ConvertIngress() *Ingress {
SvcPortInt: int64(ingressBackend.ServicePort.IntValue()),
SvcPortString: ingressBackend.ServicePort.StrVal,
IsDefaultBackend: true,
Status: "",
}
}(e.ig.Spec.Backend),
TLS: func(ingressTLS []extensionsv1beta1.IngressTLS) map[string]*IngressTLS {
Expand All @@ -245,18 +226,11 @@ func (e ingressExtensionsStrategy) ConvertIngress() *Ingress {
tls[host] = &IngressTLS{
Host: host,
SecretName: k8sTLS.SecretName,
Status: EMPTY,
}
}
}
return tls
}(e.ig.Spec.TLS),
Status: func() Status {
if e.ig.ObjectMeta.GetDeletionTimestamp() != nil {
return DELETED
}
return ADDED
}(),
}
}

Expand Down Expand Up @@ -292,7 +266,6 @@ func (n ingressNetworkingV1Strategy) ConvertIngress() *Ingress {
Path: k8sPath.Path,
PathTypeMatch: pathType,
SvcNamespace: n.ig.GetNamespace(),
Status: "",
}
if k8sPath.Backend.Service != nil {
paths[pathKey].SvcName = k8sPath.Backend.Service.Name
Expand All @@ -306,9 +279,8 @@ func (n ingressNetworkingV1Strategy) ConvertIngress() *Ingress {
}
} else {
rules[k8sRule.Host] = &IngressRule{
Host: k8sRule.Host,
Paths: paths,
Status: "",
Host: k8sRule.Host,
Paths: paths,
}
}
}
Expand All @@ -321,7 +293,6 @@ func (n ingressNetworkingV1Strategy) ConvertIngress() *Ingress {
ingPath := &IngressPath{
SvcNamespace: n.ig.GetNamespace(),
IsDefaultBackend: true,
Status: "",
}
if ingressBackend.Service != nil {
ingPath.SvcName = ingressBackend.Service.Name
Expand All @@ -337,18 +308,11 @@ func (n ingressNetworkingV1Strategy) ConvertIngress() *Ingress {
tls[host] = &IngressTLS{
Host: host,
SecretName: k8sTLS.SecretName,
Status: EMPTY,
}
}
}
return tls
}(n.ig.Spec.TLS),
Status: func() Status {
if n.ig.ObjectMeta.GetDeletionTimestamp() != nil {
return DELETED
}
return ADDED
}(),
}
}

Expand All @@ -357,12 +321,6 @@ func (n ingressNetworkingV1Strategy) ConvertClass() *IngressClass {
APIVersion: NETWORKINGV1,
Name: n.class.GetName(),
Controller: n.class.Spec.Controller,
Status: func() Status {
if n.class.ObjectMeta.GetDeletionTimestamp() != nil {
return DELETED
}
return ADDED
}(),
}
}

Expand Down
Loading

0 comments on commit 07e10cd

Please sign in to comment.