From 949918b398431f734ee4eea014c03775eef04b1f Mon Sep 17 00:00:00 2001 From: Damien Mathieu <42@dmathieu.com> Date: Thu, 27 Jan 2022 16:07:24 +0100 Subject: [PATCH] use functional options to reduce number of methods creating an EchoDeployment --- test/e2e/annotations/affinity.go | 2 +- test/e2e/annotations/affinitymode.go | 10 +++- test/e2e/annotations/authtls.go | 2 +- test/e2e/annotations/canary.go | 8 +-- test/e2e/annotations/cors.go | 2 +- test/e2e/annotations/customhttperrors.go | 2 +- test/e2e/annotations/proxyssl.go | 2 +- test/e2e/annotations/upstreamhashby.go | 2 +- test/e2e/framework/deployment.go | 55 ++++++++++++------- test/e2e/ingress/multiple_rules.go | 4 +- test/e2e/loadbalance/ewma.go | 2 +- test/e2e/loadbalance/round_robin.go | 2 +- test/e2e/lua/dynamic_configuration.go | 7 ++- test/e2e/settings/default_ssl_certificate.go | 2 +- test/e2e/settings/disable_catch_all.go | 2 +- .../settings/disable_service_external_name.go | 2 +- test/e2e/settings/ingress_class.go | 2 +- test/e2e/settings/namespace_selector.go | 2 +- test/e2e/status/update.go | 2 +- test/e2e/tcpudp/tcp.go | 2 +- 20 files changed, 69 insertions(+), 45 deletions(-) diff --git a/test/e2e/annotations/affinity.go b/test/e2e/annotations/affinity.go index 4798600437..4ca567e4c4 100644 --- a/test/e2e/annotations/affinity.go +++ b/test/e2e/annotations/affinity.go @@ -36,7 +36,7 @@ var _ = framework.DescribeAnnotation("affinity session-cookie-name", func() { f := framework.NewDefaultFramework("affinity") ginkgo.BeforeEach(func() { - f.NewEchoDeploymentWithReplicas(2) + f.NewEchoDeployment(framework.WithDeploymentReplicas(2)) }) ginkgo.It("should set sticky cookie SERVERID", func() { diff --git a/test/e2e/annotations/affinitymode.go b/test/e2e/annotations/affinitymode.go index 3b533906ec..6d22ea59f7 100644 --- a/test/e2e/annotations/affinitymode.go +++ b/test/e2e/annotations/affinitymode.go @@ -34,7 +34,10 @@ var _ = framework.DescribeAnnotation("affinitymode", func() { ginkgo.It("Balanced affinity mode should balance", func() { deploymentName := "affinitybalanceecho" replicas := 5 - f.NewEchoDeploymentWithNameAndReplicas(deploymentName, replicas) + f.NewEchoDeployment( + framework.WithDeploymentName(deploymentName), + framework.WithDeploymentReplicas(replicas), + ) host := "affinity-mode-balance.com" annotations := make(map[string]string) @@ -64,7 +67,10 @@ var _ = framework.DescribeAnnotation("affinitymode", func() { ginkgo.It("Check persistent affinity mode", func() { deploymentName := "affinitypersistentecho" replicas := 5 - f.NewEchoDeploymentWithNameAndReplicas(deploymentName, replicas) + f.NewEchoDeployment( + framework.WithDeploymentName(deploymentName), + framework.WithDeploymentReplicas(replicas), + ) host := "affinity-mode-persistent.com" annotations := make(map[string]string) diff --git a/test/e2e/annotations/authtls.go b/test/e2e/annotations/authtls.go index 093afe14e5..790165475f 100644 --- a/test/e2e/annotations/authtls.go +++ b/test/e2e/annotations/authtls.go @@ -30,7 +30,7 @@ var _ = framework.DescribeAnnotation("auth-tls-*", func() { f := framework.NewDefaultFramework("authtls") ginkgo.BeforeEach(func() { - f.NewEchoDeploymentWithReplicas(2) + f.NewEchoDeployment(framework.WithDeploymentReplicas(2)) }) ginkgo.It("should set sslClientCertificate, sslVerifyClient and sslVerifyDepth with auth-tls-secret", func() { diff --git a/test/e2e/annotations/canary.go b/test/e2e/annotations/canary.go index 31e7404341..4f1bdcad6f 100644 --- a/test/e2e/annotations/canary.go +++ b/test/e2e/annotations/canary.go @@ -39,10 +39,10 @@ var _ = framework.DescribeAnnotation("canary-*", func() { ginkgo.BeforeEach(func() { // Deployment for main backend - f.NewEchoDeploymentWithReplicas(1) + f.NewEchoDeployment() // Deployment for canary backend - f.NewEchoDeploymentWithNameAndReplicas(canaryService, 1) + f.NewEchoDeployment(framework.WithDeploymentName(canaryService)) }) ginkgo.Context("when canary is created", func() { @@ -132,7 +132,7 @@ var _ = framework.DescribeAnnotation("canary-*", func() { ginkgo.By("returning a 503 status when the mainline deployment has 0 replicas and a request is sent to the canary") - f.NewEchoDeploymentWithReplicas(0) + f.NewEchoDeployment(framework.WithDeploymentReplicas(0)) resp, _, errs := gorequest.New(). Get(f.GetURL(framework.HTTP)). @@ -145,7 +145,7 @@ var _ = framework.DescribeAnnotation("canary-*", func() { ginkgo.By("returning a 200 status when the canary deployment has 0 replicas and a request is sent to the mainline ingress") - f.NewEchoDeploymentWithReplicas(1) + f.NewEchoDeployment() f.NewDeployment(canaryService, "k8s.gcr.io/e2e-test-images/echoserver:2.3", 8080, 0) resp, _, errs = gorequest.New(). diff --git a/test/e2e/annotations/cors.go b/test/e2e/annotations/cors.go index 64b6331738..c249b38776 100644 --- a/test/e2e/annotations/cors.go +++ b/test/e2e/annotations/cors.go @@ -29,7 +29,7 @@ var _ = framework.DescribeAnnotation("cors-*", func() { f := framework.NewDefaultFramework("cors") ginkgo.BeforeEach(func() { - f.NewEchoDeploymentWithReplicas(2) + f.NewEchoDeployment(framework.WithDeploymentReplicas(2)) }) ginkgo.It("should enable cors", func() { diff --git a/test/e2e/annotations/customhttperrors.go b/test/e2e/annotations/customhttperrors.go index c0115cb524..7256b93fa5 100644 --- a/test/e2e/annotations/customhttperrors.go +++ b/test/e2e/annotations/customhttperrors.go @@ -101,7 +101,7 @@ var _ = framework.DescribeAnnotation("custom-http-errors", func() { ginkgo.By("using the custom default-backend from annotation for upstream") customDefaultBackend := "from-annotation" - f.NewEchoDeploymentWithNameAndReplicas(customDefaultBackend, 1) + f.NewEchoDeployment(framework.WithDeploymentName(customDefaultBackend)) err = framework.UpdateIngress(f.KubeClientSet, f.Namespace, host, func(ingress *networking.Ingress) error { ingress.ObjectMeta.Annotations["nginx.ingress.kubernetes.io/default-backend"] = customDefaultBackend diff --git a/test/e2e/annotations/proxyssl.go b/test/e2e/annotations/proxyssl.go index 0e928664e1..3672a4d819 100644 --- a/test/e2e/annotations/proxyssl.go +++ b/test/e2e/annotations/proxyssl.go @@ -150,7 +150,7 @@ var _ = framework.DescribeAnnotation("proxy-ssl-*", func() { ginkgo.It("proxy-ssl-location-only flag should change the nginx config server part", func() { host := "proxyssl.com" - f.NewEchoDeploymentWithNameAndReplicas("echodeployment", 1) + f.NewEchoDeployment(framework.WithDeploymentName("echodeployment")) secretName := "secretone" annotations := make(map[string]string) diff --git a/test/e2e/annotations/upstreamhashby.go b/test/e2e/annotations/upstreamhashby.go index 9474f2b2b1..c4732a18d5 100644 --- a/test/e2e/annotations/upstreamhashby.go +++ b/test/e2e/annotations/upstreamhashby.go @@ -77,7 +77,7 @@ var _ = framework.DescribeAnnotation("upstream-hash-by-*", func() { f := framework.NewDefaultFramework("upstream-hash-by") ginkgo.BeforeEach(func() { - f.NewEchoDeploymentWithReplicas(6) + f.NewEchoDeployment(framework.WithDeploymentReplicas(6)) }) ginkgo.It("should connect to the same pod", func() { diff --git a/test/e2e/framework/deployment.go b/test/e2e/framework/deployment.go index c5fded8565..3775af8bcf 100644 --- a/test/e2e/framework/deployment.go +++ b/test/e2e/framework/deployment.go @@ -40,30 +40,45 @@ const HTTPBinService = "httpbin" // NginxBaseImage use for testing const NginxBaseImage = "k8s.gcr.io/ingress-nginx/nginx:v20210926-g5662db450@sha256:1ef404b5e8741fe49605a1f40c3fdd8ef657aecdb9526ea979d1672eeabd0cd9" -// NewEchoDeployment creates a new single replica deployment of the echoserver image in a particular namespace -func (f *Framework) NewEchoDeployment() { - f.NewEchoDeploymentWithReplicas(1) +type deploymentOptions struct { + namespace string + name string + replicas int } -// NewEchoDeploymentWithReplicas creates a new deployment of the echoserver image in a particular namespace. Number of -// replicas is configurable -func (f *Framework) NewEchoDeploymentWithReplicas(replicas int) { - f.NewEchoDeploymentWithNameAndReplicas(EchoService, replicas) +// WithDeploymentNamespace allows configuring the deployment's namespace +func WithDeploymentNamespace(n string) func(*deploymentOptions) { + return func(o *deploymentOptions) { + o.namespace = n + } } -// NewEchoDeploymentWithNameAndReplicas creates a new deployment of the echoserver image in a particular namespace. Number of -// replicas is configurable and -// name is configurable -func (f *Framework) NewEchoDeploymentWithNameAndReplicas(name string, replicas int) { - f.newEchoDeployment(f.Namespace, name, replicas) +// WithDeploymentName allows configuring the deployment's names +func WithDeploymentName(n string) func(*deploymentOptions) { + return func(o *deploymentOptions) { + o.name = n + } } -func (f *Framework) NewEchoDeploymentWithNamespaceAndReplicas(namespace string, replicas int) { - f.newEchoDeployment(namespace, EchoService, replicas) +// WithDeploymentReplicas allows configuring the deployment's replicas count +func WithDeploymentReplicas(r int) func(*deploymentOptions) { + return func(o *deploymentOptions) { + o.replicas = r + } } -func (f *Framework) newEchoDeployment(namespace, name string, replicas int) { - deployment := newDeployment(name, namespace, "k8s.gcr.io/ingress-nginx/e2e-test-echo@sha256:131ece0637b29231470cfaa04690c2966a2e0b147d3c9df080a0857b78982410", 80, int32(replicas), +// NewEchoDeployment creates a new single replica deployment of the echoserver image in a particular namespace +func (f *Framework) NewEchoDeployment(opts ...func(*deploymentOptions)) { + options := &deploymentOptions{ + namespace: f.Namespace, + name: EchoService, + replicas: 1, + } + for _, o := range opts { + o(options) + } + + deployment := newDeployment(options.name, options.namespace, "k8s.gcr.io/ingress-nginx/e2e-test-echo@sha256:131ece0637b29231470cfaa04690c2966a2e0b147d3c9df080a0857b78982410", 80, int32(options.replicas), nil, []corev1.VolumeMount{}, []corev1.Volume{}, @@ -73,8 +88,8 @@ func (f *Framework) newEchoDeployment(namespace, name string, replicas int) { service := &corev1.Service{ ObjectMeta: metav1.ObjectMeta{ - Name: name, - Namespace: namespace, + Name: options.name, + Namespace: options.namespace, }, Spec: corev1.ServiceSpec{ Ports: []corev1.ServicePort{ @@ -86,14 +101,14 @@ func (f *Framework) newEchoDeployment(namespace, name string, replicas int) { }, }, Selector: map[string]string{ - "app": name, + "app": options.name, }, }, } f.EnsureService(service) - err := WaitForEndpoints(f.KubeClientSet, DefaultTimeout, name, namespace, replicas) + err := WaitForEndpoints(f.KubeClientSet, DefaultTimeout, options.name, options.namespace, options.replicas) assert.Nil(ginkgo.GinkgoT(), err, "waiting for endpoints to become ready") } diff --git a/test/e2e/ingress/multiple_rules.go b/test/e2e/ingress/multiple_rules.go index 030be172ab..07f5c1427c 100644 --- a/test/e2e/ingress/multiple_rules.go +++ b/test/e2e/ingress/multiple_rules.go @@ -31,8 +31,8 @@ var _ = framework.IngressNginxDescribe("single ingress - multiple hosts", func() f := framework.NewDefaultFramework("simh") pathprefix := networking.PathTypePrefix ginkgo.BeforeEach(func() { - f.NewEchoDeploymentWithNameAndReplicas("first-service", 1) - f.NewEchoDeploymentWithNameAndReplicas("second-service", 1) + f.NewEchoDeployment(framework.WithDeploymentName("first-service")) + f.NewEchoDeployment(framework.WithDeploymentName("second-service")) }) ginkgo.It("should set the correct $service_name NGINX variable", func() { diff --git a/test/e2e/loadbalance/ewma.go b/test/e2e/loadbalance/ewma.go index 52e2355691..15289f3720 100644 --- a/test/e2e/loadbalance/ewma.go +++ b/test/e2e/loadbalance/ewma.go @@ -32,7 +32,7 @@ var _ = framework.DescribeSetting("[Load Balancer] EWMA", func() { f := framework.NewDefaultFramework("ewma") ginkgo.BeforeEach(func() { - f.NewEchoDeploymentWithReplicas(3) + f.NewEchoDeployment(framework.WithDeploymentReplicas(3)) f.SetNginxConfigMapData(map[string]string{ "worker-processes": "2", "load-balance": "ewma"}, diff --git a/test/e2e/loadbalance/round_robin.go b/test/e2e/loadbalance/round_robin.go index f035005dd2..bc74ba9fba 100644 --- a/test/e2e/loadbalance/round_robin.go +++ b/test/e2e/loadbalance/round_robin.go @@ -32,7 +32,7 @@ var _ = framework.DescribeSetting("[Load Balancer] round-robin", func() { f := framework.NewDefaultFramework("round-robin") ginkgo.BeforeEach(func() { - f.NewEchoDeploymentWithReplicas(3) + f.NewEchoDeployment(framework.WithDeploymentReplicas(3)) f.UpdateNginxConfigMapData("worker-processes", "1") }) diff --git a/test/e2e/lua/dynamic_configuration.go b/test/e2e/lua/dynamic_configuration.go index 0a88fb07bb..b382e52cc5 100644 --- a/test/e2e/lua/dynamic_configuration.go +++ b/test/e2e/lua/dynamic_configuration.go @@ -43,7 +43,7 @@ var _ = framework.IngressNginxDescribe("[Lua] dynamic configuration", func() { f := framework.NewDefaultFramework("dynamic-configuration") ginkgo.BeforeEach(func() { - f.NewEchoDeploymentWithReplicas(1) + f.NewEchoDeployment() ensureIngress(f, "foo.com", framework.EchoService) }) @@ -124,7 +124,10 @@ var _ = framework.IngressNginxDescribe("[Lua] dynamic configuration", func() { ginkgo.It("handles endpoints only changes consistently (down scaling of replicas vs. empty service)", func() { deploymentName := "scalingecho" - f.NewEchoDeploymentWithNameAndReplicas(deploymentName, 0) + f.NewEchoDeployment( + framework.WithDeploymentName(deploymentName), + framework.WithDeploymentReplicas(0), + ) createIngress(f, "scaling.foo.com", deploymentName) resp := f.HTTPTestClient(). diff --git a/test/e2e/settings/default_ssl_certificate.go b/test/e2e/settings/default_ssl_certificate.go index 421a1543bc..eede8ef756 100644 --- a/test/e2e/settings/default_ssl_certificate.go +++ b/test/e2e/settings/default_ssl_certificate.go @@ -38,7 +38,7 @@ var _ = framework.IngressNginxDescribe("[SSL] [Flag] default-ssl-certificate", f port := 80 ginkgo.BeforeEach(func() { - f.NewEchoDeploymentWithReplicas(1) + f.NewEchoDeployment(framework.WithDeploymentReplicas(1)) var err error tlsConfig, err = framework.CreateIngressTLSSecret(f.KubeClientSet, diff --git a/test/e2e/settings/disable_catch_all.go b/test/e2e/settings/disable_catch_all.go index f5d9bfadc4..dce772f9a1 100644 --- a/test/e2e/settings/disable_catch_all.go +++ b/test/e2e/settings/disable_catch_all.go @@ -34,7 +34,7 @@ var _ = framework.IngressNginxDescribe("[Flag] disable-catch-all", func() { f := framework.NewDefaultFramework("disabled-catch-all") ginkgo.BeforeEach(func() { - f.NewEchoDeploymentWithReplicas(1) + f.NewEchoDeployment(framework.WithDeploymentReplicas(1)) err := f.UpdateIngressControllerDeployment(func(deployment *appsv1.Deployment) error { args := deployment.Spec.Template.Spec.Containers[0].Args diff --git a/test/e2e/settings/disable_service_external_name.go b/test/e2e/settings/disable_service_external_name.go index 910a906ca4..d8da89d4ac 100644 --- a/test/e2e/settings/disable_service_external_name.go +++ b/test/e2e/settings/disable_service_external_name.go @@ -35,7 +35,7 @@ var _ = framework.IngressNginxDescribe("[Flag] disable-service-external-name", f f := framework.NewDefaultFramework("disabled-service-external-name") ginkgo.BeforeEach(func() { - f.NewEchoDeploymentWithReplicas(2) + f.NewEchoDeployment(framework.WithDeploymentReplicas(2)) err := f.UpdateIngressControllerDeployment(func(deployment *appsv1.Deployment) error { args := deployment.Spec.Template.Spec.Containers[0].Args diff --git a/test/e2e/settings/ingress_class.go b/test/e2e/settings/ingress_class.go index 09134ccbaa..2372d209b7 100644 --- a/test/e2e/settings/ingress_class.go +++ b/test/e2e/settings/ingress_class.go @@ -45,7 +45,7 @@ var _ = framework.IngressNginxDescribe("[Flag] ingress-class", func() { otherController := "k8s.io/other-class" ginkgo.BeforeEach(func() { - f.NewEchoDeploymentWithReplicas(1) + f.NewEchoDeployment(framework.WithDeploymentReplicas(1)) doOnce.Do(func() { _, err := f.KubeClientSet.NetworkingV1().IngressClasses(). diff --git a/test/e2e/settings/namespace_selector.go b/test/e2e/settings/namespace_selector.go index ea162d594c..7c07a841d4 100644 --- a/test/e2e/settings/namespace_selector.go +++ b/test/e2e/settings/namespace_selector.go @@ -37,7 +37,7 @@ var _ = framework.IngressNginxDescribe("[Flag] watch namespace selector", func() prepareTestIngress := func(baseName string, host string, labels map[string]string) string { ns, err := framework.CreateKubeNamespaceWithLabel(f.BaseName, labels, f.KubeClientSet) assert.Nil(ginkgo.GinkgoT(), err, "creating test namespace") - f.NewEchoDeploymentWithNamespaceAndReplicas(ns, 1) + f.NewEchoDeployment(framework.WithDeploymentNamespace(ns)) ing := framework.NewSingleIngressWithIngressClass(host, "/", host, ns, framework.EchoService, f.IngressClass, 80, nil) f.EnsureIngress(ing) return ns diff --git a/test/e2e/status/update.go b/test/e2e/status/update.go index 23679afdc2..43d61b0e97 100644 --- a/test/e2e/status/update.go +++ b/test/e2e/status/update.go @@ -69,7 +69,7 @@ var _ = framework.IngressNginxDescribe("[Status] status update", func() { }) assert.Nil(ginkgo.GinkgoT(), err, "unexpected error updating ingress controller deployment flags") - f.NewEchoDeploymentWithReplicas(1) + f.NewEchoDeployment() ing := f.EnsureIngress(framework.NewSingleIngress(host, "/", host, f.Namespace, framework.EchoService, 80, nil)) diff --git a/test/e2e/tcpudp/tcp.go b/test/e2e/tcpudp/tcp.go index 9b1885510c..553cb46d39 100644 --- a/test/e2e/tcpudp/tcp.go +++ b/test/e2e/tcpudp/tcp.go @@ -38,7 +38,7 @@ var _ = framework.IngressNginxDescribe("[TCP] tcp-services", func() { f := framework.NewDefaultFramework("tcp") ginkgo.It("should expose a TCP service", func() { - f.NewEchoDeploymentWithReplicas(1) + f.NewEchoDeployment() config, err := f.KubeClientSet. CoreV1().