BUG/MINOR: fix k8s resync for all objects expect endpoints, endpoints…

…lices. Endpoint and endpointslices fix is done in a previous commit. With informers, resync are sent as Update, not Create. We were doing nothing on an Update on a resync. If an event was skipped for any reason, we were never resyncing.
haproxytech · Oct 5, 2023 · 6e322c4 · 6e322c4
1 parent 0ce0bec
commit 6e322c4
Show file tree

Hide file tree

Showing 7 changed files with 401 additions and 60 deletions.
diff --git a/deploy/tests/integration/pod-maxconn/pod-maxconn_test.go b/deploy/tests/integration/pod-maxconn/pod-maxconn_test.go
@@ -0,0 +1,268 @@
+// Copyright 2019 HAProxy Technologies LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package podmaxconn
+
+import (
+	"github.com/haproxytech/kubernetes-ingress/pkg/k8s"
+	"github.com/haproxytech/kubernetes-ingress/pkg/store"
+	networkingv1 "k8s.io/api/networking/v1"
+)
+
+func (suite *PodMaxConnSuite) TestPodMaxConnConfigMap() {
+	suite.StartController()
+	cm := suite.setupTest()
+
+	///////////////////////////////////////
+	// pod-maxconn setup in:
+	// - configmap only
+	cm.Status = store.MODIFIED
+	cm.Annotations["pod-maxconn"] = "128"
+	svc := newAppSvc()
+	ing := newAppIngress()
+	events := []k8s.SyncDataEvent{
+		{SyncType: k8s.CONFIGMAP, Namespace: configMapNamespace, Name: configMapName, Data: cm},
+		{SyncType: k8s.POD, Namespace: configMapName, Name: "ic1", Data: store.PodEvent{
+			Status: store.ADDED,
+			Name:   "ic1",
+		}},
+		{SyncType: k8s.POD, Namespace: configMapName, Name: "ic2", Data: store.PodEvent{
+			Status: store.ADDED,
+			Name:   "ic2",
+		}},
+		{SyncType: k8s.SERVICE, Namespace: appNs, Name: serviceName, Data: svc},
+		{SyncType: k8s.INGRESS, Namespace: appNs, Name: ingressName, Data: ing},
+	}
+	suite.fixture(events...)
+	// Expected occurrences of "default-server check maxconn 64"
+	// 64 = 128 / 2 instances of IC
+	// 1- backend haproxy-controller_default-local-service_http
+	// 1- backend appNs_appSvcName_https
+	suite.ExpectHaproxyConfigContains("default-server check maxconn 64", 2)
+
+	suite.StopController()
+}
+
+func (suite *PodMaxConnSuite) TestPodMaxConnConfigMapMisc() {
+	suite.StartController()
+	cm := suite.setupTest()
+
+	///////////////////////////////////////
+	// pod-maxconn setup in:
+	// - configmap only
+	cm.Status = store.MODIFIED
+	cm.Annotations["pod-maxconn"] = "128"
+	svc := newAppSvc()
+	ing := newAppIngress()
+	events := []k8s.SyncDataEvent{
+		{SyncType: k8s.CONFIGMAP, Namespace: configMapNamespace, Name: configMapName, Data: cm},
+		{SyncType: k8s.POD, Namespace: configMapName, Name: "ic1", Data: store.PodEvent{
+			Status: store.ADDED,
+			Name:   "ic1",
+		}},
+		{SyncType: k8s.POD, Namespace: configMapName, Name: "ic2", Data: store.PodEvent{
+			Status: store.ADDED,
+			Name:   "ic2",
+		}},
+		{SyncType: k8s.SERVICE, Namespace: appNs, Name: serviceName, Data: svc},
+		{SyncType: k8s.INGRESS, Namespace: appNs, Name: ingressName, Data: ing},
+	}
+	suite.fixture(events...)
+	// Expected occurrences of "default-server check maxconn 64"
+	// 64 = 128 / 2 instances of IC
+	// 1- backend haproxy-controller_default-local-service_http
+	// 1- backend appNs_appSvcName_https
+	suite.ExpectHaproxyConfigContains("default-server check maxconn 64", 2)
+
+	// -------------------------------------
+	// Resend ADDED => should change nothing
+	events = []k8s.SyncDataEvent{
+		{SyncType: k8s.POD, Namespace: configMapName, Name: "ic1", Data: store.PodEvent{
+			Status: store.ADDED,
+			Name:   "ic1",
+		}},
+		{SyncType: k8s.POD, Namespace: configMapName, Name: "ic2", Data: store.PodEvent{
+			Status: store.ADDED,
+			Name:   "ic2",
+		}},
+	}
+	suite.fixture(events...)
+	// Expected occurrences of "default-server check maxconn 64"
+	// 64 = 128 / 2 instances of IC
+	// 1- backend haproxy-controller_default-local-service_http
+	// 1- backend appNs_appSvcName_https
+	suite.ExpectHaproxyConfigContains("default-server check maxconn 64", 2)
+
+	// -------------------------------------
+	// SEND MODIFIED => should change nothing
+	events = []k8s.SyncDataEvent{
+		{SyncType: k8s.POD, Namespace: configMapName, Name: "ic1", Data: store.PodEvent{
+			Status: store.MODIFIED,
+			Name:   "ic1",
+		}},
+		{SyncType: k8s.POD, Namespace: configMapName, Name: "ic2", Data: store.PodEvent{
+			Status: store.MODIFIED,
+			Name:   "ic2",
+		}},
+	}
+	suite.fixture(events...)
+	// Expected occurrences of "default-server check maxconn 64"
+	// 64 = 128 / 2 instances of IC
+	// 1- backend haproxy-controller_default-local-service_http
+	// 1- backend appNs_appSvcName_https
+	suite.ExpectHaproxyConfigContains("default-server check maxconn 64", 2)
+
+	// --------------------------------------------------
+	// Send MODIFIED on a non-existing POD -> should increment
+	events = []k8s.SyncDataEvent{
+		{SyncType: k8s.POD, Namespace: configMapName, Name: "ic3", Data: store.PodEvent{
+			Status: store.MODIFIED,
+			Name:   "ic3",
+		}},
+		{SyncType: k8s.POD, Namespace: configMapName, Name: "ic4", Data: store.PodEvent{
+			Status: store.MODIFIED,
+			Name:   "ic4",
+		}},
+	}
+	suite.fixture(events...)
+	// Expected occurrences of "default-server check maxconn 32" (divided by 4)
+	// 64 = 128 / 2 instances of IC
+	// 1- backend haproxy-controller_default-local-service_http
+	// 1- backend appNs_appSvcName_https
+	suite.ExpectHaproxyConfigContains("default-server check maxconn 32", 2)
+
+	suite.StopController()
+}
+
+func (suite *PodMaxConnSuite) TestPodMaxConnService() {
+	suite.StartController()
+	cm := suite.setupTest()
+
+	///////////////////////////////////////
+	// pod-maxconn setup in:
+	// - configmap (128)
+	// - ingress (126)
+	// - app svc (124) (appNs_appSvcName_https)
+	cm.Status = store.MODIFIED
+	cm.Annotations["pod-maxconn"] = "128"
+	ing := newAppIngress()
+	ing.Annotations["pod-maxconn"] = "126"
+	svc := newAppSvc()
+	svc.Annotations["pod-maxconn"] = "124"
+
+	events := []k8s.SyncDataEvent{
+		{SyncType: k8s.CONFIGMAP, Namespace: configMapNamespace, Name: configMapName, Data: cm},
+		{SyncType: k8s.POD, Namespace: configMapName, Name: "ic1", Data: store.PodEvent{
+			Status: store.ADDED,
+			Name:   "ic1",
+		}},
+		{SyncType: k8s.POD, Namespace: configMapName, Name: "ic2", Data: store.PodEvent{
+			Status: store.ADDED,
+			Name:   "ic2",
+		}},
+		{SyncType: k8s.SERVICE, Namespace: appNs, Name: serviceName, Data: svc},
+		{SyncType: k8s.INGRESS, Namespace: appNs, Name: ingressName, Data: ing},
+	}
+	suite.fixture(events...)
+	// -- Expected occurrences of "default-server check maxconn 64" #1 (from configmap)
+	// backend haproxy-controller_default-local-service_http
+	// -- Expected occurrences of "default-server check maxconn 62": #1 (from service)
+	// backend appNs_appSvcName_https
+	suite.ExpectHaproxyConfigContains("default-server check maxconn 64", 1)
+	suite.ExpectHaproxyConfigContains("default-server check maxconn 62", 1)
+	suite.ExpectHaproxyConfigContains("default-server check maxconn 63", 0)
+
+	suite.StopController()
+}
+
+func (suite *PodMaxConnSuite) TestPodMaxConnIngress() {
+	suite.StartController()
+	cm := suite.setupTest()
+
+	// pod-maxconn setup in:
+	// timeout server setup in:
+	// - configmap (77000)
+	// - ingress (76000)
+	cm.Status = store.MODIFIED
+	cm.Annotations["pod-maxconn"] = "128"
+	ing := newAppIngress()
+	ing.Annotations["pod-maxconn"] = "126"
+	svc := newAppSvc()
+
+	events := []k8s.SyncDataEvent{
+		{SyncType: k8s.CONFIGMAP, Namespace: configMapNamespace, Name: configMapName, Data: cm},
+		{SyncType: k8s.POD, Namespace: configMapName, Name: "ic1", Data: store.PodEvent{
+			Status: store.ADDED,
+			Name:   "ic1",
+		}},
+		{SyncType: k8s.POD, Namespace: configMapName, Name: "ic2", Data: store.PodEvent{
+			Status: store.ADDED,
+			Name:   "ic2",
+		}},
+		{SyncType: k8s.SERVICE, Namespace: appNs, Name: serviceName, Data: svc},
+		{SyncType: k8s.INGRESS, Namespace: appNs, Name: ingressName, Data: ing},
+	}
+	suite.fixture(events...)
+	// -- Expected occurrences of "default-server check maxconn 64" #1 (from configmap)
+	// backend haproxy-controller_default-local-service_http
+	// -- Expected occurrences of "default-server check maxconn 62": #1 (from service)
+	// backend appNs_appSvcName_https
+	suite.ExpectHaproxyConfigContains("default-server check maxconn 64", 1)
+	suite.ExpectHaproxyConfigContains("default-server check maxconn 62", 0)
+	suite.ExpectHaproxyConfigContains("default-server check maxconn 63", 1)
+
+	suite.StopController()
+}
+
+func newAppSvc() *store.Service {
+	return &store.Service{
+		Annotations: map[string]string{},
+		Name:        serviceName,
+		Namespace:   appNs,
+		Ports: []store.ServicePort{
+			{
+				Name:     "https",
+				Protocol: "TCP",
+				Port:     443,
+				Status:   store.ADDED,
+			},
+		},
+		Status: store.ADDED,
+	}
+}
+
+func newAppIngress() *store.Ingress {
+	return &store.Ingress{
+		IngressCore: store.IngressCore{
+			APIVersion:  store.NETWORKINGV1,
+			Name:        ingressName,
+			Namespace:   appNs,
+			Annotations: map[string]string{},
+			Rules: map[string]*store.IngressRule{
+				"": {
+					Paths: map[string]*store.IngressPath{
+						string(networkingv1.PathTypePrefix) + "-/": {
+							Path:          "/",
+							PathTypeMatch: string(networkingv1.PathTypePrefix),
+							SvcNamespace:  appNs,
+							SvcPortString: "https",
+							SvcName:       serviceName,
+						},
+					},
+				},
+			},
+		},
+		Status: store.ADDED,
+	}
+}
diff --git a/deploy/tests/integration/pod-maxconn/suite_test.go b/deploy/tests/integration/pod-maxconn/suite_test.go
@@ -0,0 +1,88 @@
+// Copyright 2023 HAProxy Technologies LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package podmaxconn
+
+import (
+	"testing"
+
+	"github.com/haproxytech/kubernetes-ingress/deploy/tests/integration"
+	"github.com/haproxytech/kubernetes-ingress/pkg/k8s"
+	"github.com/haproxytech/kubernetes-ingress/pkg/store"
+	"github.com/stretchr/testify/suite"
+)
+
+var (
+	appNs              = "appNs"
+	serviceName        = "appSvcName"
+	ingressName        = "appIngName"
+	configMapNamespace = "haproxy-controller"
+	configMapName      = "haproxy-kubernetes-ingress"
+)
+
+type PodMaxConnSuite struct {
+	integration.BaseSuite
+}
+
+func TestPodMaxConn(t *testing.T) {
+	t.Parallel()
+	suite.Run(t, new(PodMaxConnSuite))
+}
+
+func (suite *PodMaxConnSuite) BeforeTest(suiteName, testName string) {
+	suite.BaseSuite.BeforeTest(suiteName, testName)
+	// Add any needed update to the controller setting
+	// by updating suite.TestControllers[suite.T().Name()].XXXXX
+	testController := suite.TestControllers[suite.T().Name()]
+	testController.OSArgs.ConfigMap.Name = configMapName
+	testController.OSArgs.ConfigMap.Namespace = configMapNamespace
+}
+
+func newConfigMap() *store.ConfigMap {
+	return &store.ConfigMap{
+		Annotations: map[string]string{},
+		Namespace:   configMapNamespace,
+		Name:        configMapName,
+		Status:      store.ADDED,
+	}
+}
+
+func (suite *PodMaxConnSuite) setupTest() *store.ConfigMap {
+	testController := suite.TestControllers[suite.T().Name()]
+
+	ns := store.Namespace{Name: appNs, Status: store.ADDED}
+	cm := newConfigMap()
+	testController.EventChan <- k8s.SyncDataEvent{SyncType: k8s.NAMESPACE, Namespace: ns.Name, Data: &ns}
+	testController.EventChan <- k8s.SyncDataEvent{
+		SyncType: k8s.CONFIGMAP, Namespace: configMapNamespace, Name: configMapName, Data: newConfigMap(),
+	}
+	testController.EventChan <- k8s.SyncDataEvent{SyncType: k8s.COMMAND}
+	controllerHasWorked := make(chan struct{})
+	testController.EventChan <- k8s.SyncDataEvent{SyncType: k8s.COMMAND, EventProcessed: controllerHasWorked}
+	<-controllerHasWorked
+	return cm
+}
+
+func (suite *PodMaxConnSuite) fixture(events ...k8s.SyncDataEvent) {
+	testController := suite.TestControllers[suite.T().Name()]
+
+	// Now sending store events for test setup
+	for _, e := range events {
+		testController.EventChan <- e
+	}
+	testController.EventChan <- k8s.SyncDataEvent{SyncType: k8s.COMMAND}
+	controllerHasWorked := make(chan struct{})
+	testController.EventChan <- k8s.SyncDataEvent{SyncType: k8s.COMMAND, EventProcessed: controllerHasWorked}
+	<-controllerHasWorked
+}
diff --git a/pkg/annotations/service/maxconn.go b/pkg/annotations/service/maxconn.go
@@ -35,8 +35,8 @@ func (a *Maxconn) Process(k store.K8s, annotations ...map[string]string) error {
 		return err
 	}
 	// adjust backend maxconn when using multiple HAProxy Instances
-	if k.NbrHAProxyInst != 0 {
-		v /= k.NbrHAProxyInst
+	if len(k.HaProxyPods) != 0 {
+		v /= int64(len(k.HaProxyPods))
 	}
 	if a.backend.DefaultServer == nil {
 		a.backend.DefaultServer = &models.DefaultServer{}