From 896970c8a3c35dbf5e34ad5b52e0c5fa0576c2c3 Mon Sep 17 00:00:00 2001 From: Thomas Eckert Date: Wed, 14 Jun 2023 16:54:28 -0400 Subject: [PATCH 01/11] Fix cache and service deletion issue --- control-plane/api-gateway/binding/binder.go | 17 +++++++++++++++++ control-plane/api-gateway/cache/gateway.go | 17 ----------------- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/control-plane/api-gateway/binding/binder.go b/control-plane/api-gateway/binding/binder.go index 28a26985a8..96741be981 100644 --- a/control-plane/api-gateway/binding/binder.go +++ b/control-plane/api-gateway/binding/binder.go @@ -110,6 +110,23 @@ func (b *Binder) Snapshot() *Snapshot { } } + for _, service := range b.config.ConsulGatewayServices { + podExists := false + for _, pod := range registrationPods { + if service.ServiceID == pod.Name { + podExists = true + } + } + + if !podExists { + snapshot.Consul.Deregistrations = append(snapshot.Consul.Deregistrations, api.CatalogDeregistration{ + Node: service.Node, + ServiceID: service.ServiceID, + Namespace: service.Namespace, + }) + } + } + gatewayClassConfig := b.config.GatewayClassConfig isGatewayDeleted := b.isGatewayDeleted() diff --git a/control-plane/api-gateway/cache/gateway.go b/control-plane/api-gateway/cache/gateway.go index d1de8dd7cc..0d79542eec 100644 --- a/control-plane/api-gateway/cache/gateway.go +++ b/control-plane/api-gateway/cache/gateway.go @@ -14,7 +14,6 @@ import ( "github.com/hashicorp/consul-k8s/control-plane/consul" "github.com/hashicorp/consul/api" "k8s.io/apimachinery/pkg/types" - "sigs.k8s.io/controller-runtime/pkg/event" ) type GatewayCache struct { @@ -22,8 +21,6 @@ type GatewayCache struct { serverMgr consul.ServerConnectionManager logger logr.Logger - events chan event.GenericEvent - data map[api.ResourceReference][]api.CatalogService dataMutex sync.RWMutex @@ -38,7 +35,6 @@ func NewGatewayCache(ctx context.Context, config Config) *GatewayCache { config: config, serverMgr: config.ConsulServerConnMgr, logger: config.Logger, - events: make(chan event.GenericEvent), data: make(map[api.ResourceReference][]api.CatalogService), subscribedGateways: make(map[api.ResourceReference]context.CancelFunc), ctx: ctx, @@ -140,18 +136,5 @@ func (r *GatewayCache) subscribeToGateway(ctx context.Context, ref api.ResourceR r.dataMutex.Lock() r.data[common.NormalizeMeta(ref)] = derefed r.dataMutex.Unlock() - - event := event.GenericEvent{ - Object: newConfigEntryObject(resource), - } - - select { - case <-ctx.Done(): - r.dataMutex.Lock() - delete(r.data, ref) - r.dataMutex.Unlock() - return - case r.events <- event: - } } } From a7dfa1ca06c1b226f6427d3bb4b904c29819e3e0 Mon Sep 17 00:00:00 2001 From: Nathan Coleman Date: Wed, 14 Jun 2023 17:00:38 -0400 Subject: [PATCH 02/11] Add comments --- control-plane/api-gateway/binding/binder.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/control-plane/api-gateway/binding/binder.go b/control-plane/api-gateway/binding/binder.go index 96741be981..a319b7f76e 100644 --- a/control-plane/api-gateway/binding/binder.go +++ b/control-plane/api-gateway/binding/binder.go @@ -110,6 +110,8 @@ func (b *Binder) Snapshot() *Snapshot { } } +// Loop through the list of Consul services and deregister any instances +// that have had their corresponding Pod deleted for _, service := range b.config.ConsulGatewayServices { podExists := false for _, pod := range registrationPods { From 6edf8123449d5775145b6fbba09c1d22346f88bd Mon Sep 17 00:00:00 2001 From: Sarah Alsmiller Date: Wed, 14 Jun 2023 16:00:59 -0500 Subject: [PATCH 03/11] add in acceptance test --- acceptance/go.mod | 3 +- acceptance/go.sum | 2 + .../api_gateway_gatewayclassconfig_test.go | 208 ++++++++++++++++++ 3 files changed, 212 insertions(+), 1 deletion(-) create mode 100644 acceptance/tests/api-gateway/api_gateway_gatewayclassconfig_test.go diff --git a/acceptance/go.mod b/acceptance/go.mod index ddce3e4e5c..59cbbab79f 100644 --- a/acceptance/go.mod +++ b/acceptance/go.mod @@ -16,6 +16,7 @@ require ( k8s.io/api v0.26.3 k8s.io/apimachinery v0.26.3 k8s.io/client-go v0.26.3 + k8s.io/utils v0.0.0-20230209194617-a36077c30491 sigs.k8s.io/controller-runtime v0.14.6 sigs.k8s.io/gateway-api v0.7.0 ) @@ -30,6 +31,7 @@ require ( github.com/cespare/xxhash/v2 v2.1.2 // indirect github.com/cpuguy83/go-md2man/v2 v2.0.0 // indirect github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect + github.com/deckarep/golang-set v1.7.1 // indirect github.com/emicklei/go-restful/v3 v3.9.0 // indirect github.com/evanphx/json-patch v5.6.0+incompatible // indirect github.com/evanphx/json-patch/v5 v5.6.0 // indirect @@ -120,7 +122,6 @@ require ( k8s.io/component-base v0.26.3 // indirect k8s.io/klog/v2 v2.100.1 // indirect k8s.io/kube-openapi v0.0.0-20230501164219-8b0f38b5fd1f // indirect - k8s.io/utils v0.0.0-20230209194617-a36077c30491 // indirect sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd // indirect sigs.k8s.io/structured-merge-diff/v4 v4.2.3 // indirect sigs.k8s.io/yaml v1.3.0 // indirect diff --git a/acceptance/go.sum b/acceptance/go.sum index bd7421de60..578a95b1dd 100644 --- a/acceptance/go.sum +++ b/acceptance/go.sum @@ -144,6 +144,8 @@ github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSs github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc h1:U9qPSI2PIWSS1VwoXQT9A3Wy9MM3WgvqSxFWenqJduM= github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/deckarep/golang-set v1.7.1 h1:SCQV0S6gTtp6itiFrTqI+pfmJ4LN85S1YzhDf9rTHJQ= +github.com/deckarep/golang-set v1.7.1/go.mod h1:93vsz/8Wt4joVM7c2AVqh+YRMiUSc14yDtF28KmMOgQ= github.com/dgrijalva/jwt-go v3.2.0+incompatible/go.mod h1:E3ru+11k8xSBh+hMPgOLZmtrrCbhqsmaPHjLKYnJCaQ= github.com/dimchansky/utfbom v1.1.0/go.mod h1:rO41eb7gLfo8SF1jd9F8HplJm1Fewwi4mQvIirEdv+8= github.com/dnaeon/go-vcr v1.0.1/go.mod h1:aBB1+wY4s93YsC3HHjMBMrwTj2R9FHDzUr9KyGc8n1E= diff --git a/acceptance/tests/api-gateway/api_gateway_gatewayclassconfig_test.go b/acceptance/tests/api-gateway/api_gateway_gatewayclassconfig_test.go new file mode 100644 index 0000000000..6100ed6c2e --- /dev/null +++ b/acceptance/tests/api-gateway/api_gateway_gatewayclassconfig_test.go @@ -0,0 +1,208 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: MPL-2.0 + +package apigateway + +import ( + "context" + "github.com/hashicorp/consul-k8s/acceptance/framework/consul" + "github.com/hashicorp/consul-k8s/acceptance/framework/helpers" + "github.com/hashicorp/consul-k8s/acceptance/framework/logger" + "github.com/hashicorp/consul-k8s/control-plane/api-gateway/common" + "github.com/hashicorp/consul-k8s/control-plane/api/v1alpha1" + "github.com/hashicorp/consul/api" + "github.com/hashicorp/consul/sdk/testutil/retry" + "github.com/stretchr/testify/require" + appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "k8s.io/utils/pointer" + "sigs.k8s.io/controller-runtime/pkg/client" + gwv1beta1 "sigs.k8s.io/gateway-api/apis/v1beta1" + "testing" +) + +// GatewayClassConfig tests the creation of a gatewayclassconfig object and makes sure that its configuration +// is properly applied to any child gateway objects, namely that the number of gateway instances match the defined +// minInstances,maxInstances and defaultInstances parameters, and that changing the parent gateway does not affect +// the child gateways. +func TestAPIGateway_GatewayClassConfig(t *testing.T) { + ctx := suite.Environment().DefaultContext(t) + cfg := suite.Config() + helmValues := map[string]string{ + "global.logLevel": "trace", + "connectInject.enabled": "true", + } + releaseName := helpers.RandomName() + consulCluster := consul.NewHelmCluster(t, helmValues, ctx, cfg, releaseName) + consulCluster.Create(t) + // Override the default proxy config settings for this test. + consulClient, _ := consulCluster.SetupConsulClient(t, false) + _, _, err := consulClient.ConfigEntries().Set(&api.ProxyConfigEntry{ + Kind: api.ProxyDefaults, + Name: api.ProxyConfigGlobal, + Config: map[string]interface{}{ + "protocol": "http", + }, + }, nil) + require.NoError(t, err) + + k8sClient := ctx.ControllerRuntimeClient(t) + namespace := "gateway-namespace" + + //create clean namespace + err = k8sClient.Create(context.Background(), &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: namespace, + }, + }) + require.NoError(t, err) + helpers.Cleanup(t, cfg.NoCleanupOnFailure, func() { + logger.Log(t, "deleting gateway namesapce") + k8sClient.Delete(context.Background(), &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: namespace, + }, + }) + }) + + defaultInstances := pointer.Int32(2) + maxInstances := pointer.Int32(8) + minInstances := pointer.Int32(1) + // create a GatewayClassConfig with configuration set + gatewayClassConfigName := "gateway-class-config" + gatewayClassConfig := &v1alpha1.GatewayClassConfig{ + ObjectMeta: metav1.ObjectMeta{ + Name: gatewayClassConfigName, + }, + Spec: v1alpha1.GatewayClassConfigSpec{ + DeploymentSpec: v1alpha1.DeploymentSpec{ + DefaultInstances: defaultInstances, + MaxInstances: maxInstances, + MinInstances: minInstances, + }, + }, + } + logger.Log(t, "creating gateway class config") + err = k8sClient.Create(context.Background(), gatewayClassConfig) + require.NoError(t, err) + helpers.Cleanup(t, cfg.NoCleanupOnFailure, func() { + logger.Log(t, "deleting all gateway class configs") + k8sClient.DeleteAllOf(context.Background(), &v1alpha1.GatewayClassConfig{}) + }) + + gatewayParametersRef := &gwv1beta1.ParametersReference{ + Group: gwv1beta1.Group(v1alpha1.ConsulHashicorpGroup), + Kind: gwv1beta1.Kind(v1alpha1.GatewayClassConfigKind), + Name: gatewayClassConfigName, + } + + // create gateway class referencing gateway-class-config + gatewayClassName := "gateway-class" + logger.Log(t, "creating controlled gateway class") + createGatewayClass(t, k8sClient, gatewayClassName, gatewayClassControllerName, gatewayParametersRef) + helpers.Cleanup(t, cfg.NoCleanupOnFailure, func() { + logger.Log(t, "deleting all gateway classes") + k8sClient.DeleteAllOf(context.Background(), &gwv1beta1.GatewayClass{}) + }) + + // Create a certificate to reference in listeners + certificateInfo := generateCertificate(t, nil, "certificate.consul.local") + certificateName := "certificate" + certificate := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: certificateName, + Namespace: namespace, + Labels: map[string]string{ + "test-certificate": "true", + }, + }, + Type: corev1.SecretTypeTLS, + Data: map[string][]byte{ + corev1.TLSCertKey: certificateInfo.CertPEM, + corev1.TLSPrivateKeyKey: certificateInfo.PrivateKeyPEM, + }, + } + logger.Log(t, "creating certificate") + err = k8sClient.Create(context.Background(), certificate) + require.NoError(t, err) + helpers.Cleanup(t, cfg.NoCleanupOnFailure, func() { + k8sClient.Delete(context.Background(), certificate) + }) + + // Create gateway referencing gateway class config + gatewayName := "gateway" + logger.Log(t, "creating controlled gateway") + gateway := createGateway(t, k8sClient, gatewayName, namespace, gatewayClassName, certificateName) + // make sure it exists + logger.Log(t, "checking that gateway one is synchronized to Consul") + checkConsulExists(t, consulClient, api.APIGateway, gatewayName) + + helpers.Cleanup(t, cfg.NoCleanupOnFailure, func() { + logger.Log(t, "deleting all gateways") + k8sClient.DeleteAllOf(context.Background(), &gwv1beta1.Gateway{}, client.InNamespace(namespace)) + }) + + // Scenario: Gateway deployment should match the default instances defined on the gateway class config + logger.Log(t, "checking that gateway instances match defined gateway class config") + checkNumberOfInstances(t, k8sClient, consulClient, gateway.Name, gateway.Namespace, defaultInstances, gateway) + + //Scenario: Updating the GatewayClassConfig should not affect gateways that have already been created + logger.Log(t, "updating gatewayclassconfig values") + err = k8sClient.Get(context.Background(), types.NamespacedName{Name: gatewayClassConfigName, Namespace: namespace}, gatewayClassConfig) + require.NoError(t, err) + gatewayClassConfig.Spec.DeploymentSpec.DefaultInstances = pointer.Int32(8) + gatewayClassConfig.Spec.DeploymentSpec.MinInstances = pointer.Int32(5) + err = k8sClient.Update(context.Background(), gatewayClassConfig) + require.NoError(t, err) + checkNumberOfInstances(t, k8sClient, consulClient, gateway.Name, gateway.Namespace, defaultInstances, gateway) + + //Scenario: gateways should be able to scale independently and not get overridden by the controller unless it's above the max + scale(t, k8sClient, gateway.Name, gateway.Namespace, maxInstances) + checkNumberOfInstances(t, k8sClient, consulClient, gateway.Name, gateway.Namespace, maxInstances, gateway) + scale(t, k8sClient, gateway.Name, gateway.Namespace, pointer.Int32(100)) + checkNumberOfInstances(t, k8sClient, consulClient, gateway.Name, gateway.Namespace, maxInstances, gateway) + scale(t, k8sClient, gateway.Name, gateway.Namespace, pointer.Int32(0)) + checkNumberOfInstances(t, k8sClient, consulClient, gateway.Name, gateway.Namespace, minInstances, gateway) + +} + +func scale(t *testing.T, client client.Client, name, namespace string, scaleTo *int32) { + t.Helper() + + retryCheck(t, 30, func(r *retry.R) { + var deployment appsv1.Deployment + err := client.Get(context.Background(), types.NamespacedName{Name: name, Namespace: namespace}, &deployment) + require.NoError(r, err) + + deployment.Spec.Replicas = scaleTo + err = client.Update(context.Background(), &deployment) + require.NoError(r, err) + + //time.Sleep(time.Minute * 100) + }) +} + +func checkNumberOfInstances(t *testing.T, k8client client.Client, consulClient *api.Client, name, namespace string, wantNumber *int32, gateway *gwv1beta1.Gateway) { + t.Helper() + + retryCheck(t, 30, func(r *retry.R) { + //first check to make sure the number of replicas has been set properly + var deployment appsv1.Deployment + err := k8client.Get(context.Background(), types.NamespacedName{Name: name, Namespace: namespace}, &deployment) + require.NoError(r, err) + require.EqualValues(r, *wantNumber, *deployment.Spec.Replicas) + + //then check to make sure the number of gateway pods matches the replicas generated + podList := corev1.PodList{} + labels := common.LabelsForGateway(gateway) + err = k8client.List(context.Background(), &podList, client.InNamespace(namespace), client.MatchingLabels(labels)) + require.NoError(r, err) + require.EqualValues(r, *wantNumber, len(podList.Items)) + + service, _, err := consulClient.Catalog().Service(name, "", nil) + require.NoError(r, err) + require.EqualValues(r, *wantNumber, len(service)) + }) +} From ef87e895c5c62e564e59d40d00f28021dc61fbde Mon Sep 17 00:00:00 2001 From: Nathan Coleman Date: Wed, 14 Jun 2023 17:01:29 -0400 Subject: [PATCH 04/11] Fix indentation --- control-plane/api-gateway/binding/binder.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/control-plane/api-gateway/binding/binder.go b/control-plane/api-gateway/binding/binder.go index a319b7f76e..3306f670f1 100644 --- a/control-plane/api-gateway/binding/binder.go +++ b/control-plane/api-gateway/binding/binder.go @@ -110,8 +110,8 @@ func (b *Binder) Snapshot() *Snapshot { } } -// Loop through the list of Consul services and deregister any instances -// that have had their corresponding Pod deleted + // Loop through the list of Consul services and deregister any instances + // that have had their corresponding Pod deleted for _, service := range b.config.ConsulGatewayServices { podExists := false for _, pod := range registrationPods { From b088e35e8a44a17a5802ec34021917fda98a86ee Mon Sep 17 00:00:00 2001 From: Nathan Coleman Date: Thu, 15 Jun 2023 10:16:36 -0400 Subject: [PATCH 05/11] Fix unit test for deleting gateway w/ consul services --- control-plane/api-gateway/binding/binder_test.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/control-plane/api-gateway/binding/binder_test.go b/control-plane/api-gateway/binding/binder_test.go index a3af702dab..bbd869ed14 100644 --- a/control-plane/api-gateway/binding/binder_test.go +++ b/control-plane/api-gateway/binding/binder_test.go @@ -809,6 +809,11 @@ func TestBinder_Registrations(t *testing.T) { {Node: "test", ServiceID: "pod2", Namespace: "namespace1"}, {Node: "test", ServiceID: "pod3", Namespace: "namespace1"}, }, + Pods: []corev1.Pod{ + {ObjectMeta: metav1.ObjectMeta{Name: "pod1"}}, + {ObjectMeta: metav1.ObjectMeta{Name: "pod2"}}, + {ObjectMeta: metav1.ObjectMeta{Name: "pod3"}}, + }, }), expectedDeregistrations: []api.CatalogDeregistration{ {Node: "test", ServiceID: "pod1", Namespace: "namespace1"}, From 1dd8c811bcd28d567eaf1b574e001a59dd58a584 Mon Sep 17 00:00:00 2001 From: Nathan Coleman Date: Thu, 15 Jun 2023 10:28:21 -0400 Subject: [PATCH 06/11] Remove redundant service deregistration code --- control-plane/api-gateway/binding/binder.go | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/control-plane/api-gateway/binding/binder.go b/control-plane/api-gateway/binding/binder.go index 3306f670f1..28a26985a8 100644 --- a/control-plane/api-gateway/binding/binder.go +++ b/control-plane/api-gateway/binding/binder.go @@ -110,25 +110,6 @@ func (b *Binder) Snapshot() *Snapshot { } } - // Loop through the list of Consul services and deregister any instances - // that have had their corresponding Pod deleted - for _, service := range b.config.ConsulGatewayServices { - podExists := false - for _, pod := range registrationPods { - if service.ServiceID == pod.Name { - podExists = true - } - } - - if !podExists { - snapshot.Consul.Deregistrations = append(snapshot.Consul.Deregistrations, api.CatalogDeregistration{ - Node: service.Node, - ServiceID: service.ServiceID, - Namespace: service.Namespace, - }) - } - } - gatewayClassConfig := b.config.GatewayClassConfig isGatewayDeleted := b.isGatewayDeleted() From 7513cbc3fac3e6cffb3f18af535cca28d689f195 Mon Sep 17 00:00:00 2001 From: Nathan Coleman Date: Thu, 15 Jun 2023 10:29:18 -0400 Subject: [PATCH 07/11] Exit loop early once registration is found for service --- control-plane/api-gateway/binding/binder.go | 1 + 1 file changed, 1 insertion(+) diff --git a/control-plane/api-gateway/binding/binder.go b/control-plane/api-gateway/binding/binder.go index 28a26985a8..7fbf18d412 100644 --- a/control-plane/api-gateway/binding/binder.go +++ b/control-plane/api-gateway/binding/binder.go @@ -197,6 +197,7 @@ func (b *Binder) Snapshot() *Snapshot { for _, registration := range registrations { if service.ServiceID == registration.Service.ID { found = true + break } } if !found { From 39d93a1bbfd1ba0e9fa19612c6f61bd5c53d7687 Mon Sep 17 00:00:00 2001 From: Nathan Coleman Date: Thu, 15 Jun 2023 10:32:25 -0400 Subject: [PATCH 08/11] Fix import blocking --- .../api_gateway_gatewayclassconfig_test.go | 14 ++++++++------ control-plane/api-gateway/binding/binder_test.go | 2 +- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/acceptance/tests/api-gateway/api_gateway_gatewayclassconfig_test.go b/acceptance/tests/api-gateway/api_gateway_gatewayclassconfig_test.go index 6100ed6c2e..987892be01 100644 --- a/acceptance/tests/api-gateway/api_gateway_gatewayclassconfig_test.go +++ b/acceptance/tests/api-gateway/api_gateway_gatewayclassconfig_test.go @@ -5,11 +5,8 @@ package apigateway import ( "context" - "github.com/hashicorp/consul-k8s/acceptance/framework/consul" - "github.com/hashicorp/consul-k8s/acceptance/framework/helpers" - "github.com/hashicorp/consul-k8s/acceptance/framework/logger" - "github.com/hashicorp/consul-k8s/control-plane/api-gateway/common" - "github.com/hashicorp/consul-k8s/control-plane/api/v1alpha1" + "testing" + "github.com/hashicorp/consul/api" "github.com/hashicorp/consul/sdk/testutil/retry" "github.com/stretchr/testify/require" @@ -20,7 +17,12 @@ import ( "k8s.io/utils/pointer" "sigs.k8s.io/controller-runtime/pkg/client" gwv1beta1 "sigs.k8s.io/gateway-api/apis/v1beta1" - "testing" + + "github.com/hashicorp/consul-k8s/acceptance/framework/consul" + "github.com/hashicorp/consul-k8s/acceptance/framework/helpers" + "github.com/hashicorp/consul-k8s/acceptance/framework/logger" + "github.com/hashicorp/consul-k8s/control-plane/api-gateway/common" + "github.com/hashicorp/consul-k8s/control-plane/api/v1alpha1" ) // GatewayClassConfig tests the creation of a gatewayclassconfig object and makes sure that its configuration diff --git a/control-plane/api-gateway/binding/binder_test.go b/control-plane/api-gateway/binding/binder_test.go index bbd869ed14..adc6f7dbbd 100644 --- a/control-plane/api-gateway/binding/binder_test.go +++ b/control-plane/api-gateway/binding/binder_test.go @@ -15,6 +15,7 @@ import ( logrtest "github.com/go-logr/logr/testing" "github.com/google/go-cmp/cmp" + "github.com/hashicorp/consul/api" "github.com/stretchr/testify/require" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -25,7 +26,6 @@ import ( "github.com/hashicorp/consul-k8s/control-plane/api-gateway/common" "github.com/hashicorp/consul-k8s/control-plane/api/v1alpha1" - "github.com/hashicorp/consul/api" ) func init() { From 8681705d023651e1e3911120b65d89292b5130a9 Mon Sep 17 00:00:00 2001 From: Nathan Coleman Date: Thu, 15 Jun 2023 10:38:37 -0400 Subject: [PATCH 09/11] Set status on pods added to test --- .../api-gateway/binding/binder_test.go | 24 ++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/control-plane/api-gateway/binding/binder_test.go b/control-plane/api-gateway/binding/binder_test.go index adc6f7dbbd..bb30b98f52 100644 --- a/control-plane/api-gateway/binding/binder_test.go +++ b/control-plane/api-gateway/binding/binder_test.go @@ -810,9 +810,27 @@ func TestBinder_Registrations(t *testing.T) { {Node: "test", ServiceID: "pod3", Namespace: "namespace1"}, }, Pods: []corev1.Pod{ - {ObjectMeta: metav1.ObjectMeta{Name: "pod1"}}, - {ObjectMeta: metav1.ObjectMeta{Name: "pod2"}}, - {ObjectMeta: metav1.ObjectMeta{Name: "pod3"}}, + { + ObjectMeta: metav1.ObjectMeta{Name: "pod1"}, + Status: corev1.PodStatus{ + Phase: corev1.PodRunning, + Conditions: []corev1.PodCondition{{Type: corev1.PodReady, Status: corev1.ConditionTrue}}, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{Name: "pod2"}, + Status: corev1.PodStatus{ + Phase: corev1.PodRunning, + Conditions: []corev1.PodCondition{{Type: corev1.PodReady, Status: corev1.ConditionTrue}}, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{Name: "pod3"}, + Status: corev1.PodStatus{ + Phase: corev1.PodRunning, + Conditions: []corev1.PodCondition{{Type: corev1.PodReady, Status: corev1.ConditionTrue}}, + }, + }, }, }), expectedDeregistrations: []api.CatalogDeregistration{ From 3178b1bf7e67eaedf1c1bbbfed286c81c6c4b8ed Mon Sep 17 00:00:00 2001 From: Nathan Coleman Date: Thu, 15 Jun 2023 10:44:39 -0400 Subject: [PATCH 10/11] Apply suggestions from code review --- .../api-gateway/api_gateway_gatewayclassconfig_test.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/acceptance/tests/api-gateway/api_gateway_gatewayclassconfig_test.go b/acceptance/tests/api-gateway/api_gateway_gatewayclassconfig_test.go index 987892be01..fcb0367e4a 100644 --- a/acceptance/tests/api-gateway/api_gateway_gatewayclassconfig_test.go +++ b/acceptance/tests/api-gateway/api_gateway_gatewayclassconfig_test.go @@ -181,8 +181,6 @@ func scale(t *testing.T, client client.Client, name, namespace string, scaleTo * deployment.Spec.Replicas = scaleTo err = client.Update(context.Background(), &deployment) require.NoError(r, err) - - //time.Sleep(time.Minute * 100) }) } @@ -203,8 +201,8 @@ func checkNumberOfInstances(t *testing.T, k8client client.Client, consulClient * require.NoError(r, err) require.EqualValues(r, *wantNumber, len(podList.Items)) - service, _, err := consulClient.Catalog().Service(name, "", nil) + services, _, err := consulClient.Catalog().Service(name, "", nil) require.NoError(r, err) - require.EqualValues(r, *wantNumber, len(service)) + require.EqualValues(r, *wantNumber, len(services)) }) } From 979c906456ce53d3efbb7a515abd2a6ca244b695 Mon Sep 17 00:00:00 2001 From: Thomas Eckert Date: Thu, 15 Jun 2023 11:30:27 -0400 Subject: [PATCH 11/11] Reduce count of test gateways to 10 from 100 --- .../tests/api-gateway/api_gateway_gatewayclassconfig_test.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/acceptance/tests/api-gateway/api_gateway_gatewayclassconfig_test.go b/acceptance/tests/api-gateway/api_gateway_gatewayclassconfig_test.go index 6100ed6c2e..146fa094aa 100644 --- a/acceptance/tests/api-gateway/api_gateway_gatewayclassconfig_test.go +++ b/acceptance/tests/api-gateway/api_gateway_gatewayclassconfig_test.go @@ -5,6 +5,8 @@ package apigateway import ( "context" + "testing" + "github.com/hashicorp/consul-k8s/acceptance/framework/consul" "github.com/hashicorp/consul-k8s/acceptance/framework/helpers" "github.com/hashicorp/consul-k8s/acceptance/framework/logger" @@ -20,7 +22,6 @@ import ( "k8s.io/utils/pointer" "sigs.k8s.io/controller-runtime/pkg/client" gwv1beta1 "sigs.k8s.io/gateway-api/apis/v1beta1" - "testing" ) // GatewayClassConfig tests the creation of a gatewayclassconfig object and makes sure that its configuration @@ -161,7 +162,7 @@ func TestAPIGateway_GatewayClassConfig(t *testing.T) { //Scenario: gateways should be able to scale independently and not get overridden by the controller unless it's above the max scale(t, k8sClient, gateway.Name, gateway.Namespace, maxInstances) checkNumberOfInstances(t, k8sClient, consulClient, gateway.Name, gateway.Namespace, maxInstances, gateway) - scale(t, k8sClient, gateway.Name, gateway.Namespace, pointer.Int32(100)) + scale(t, k8sClient, gateway.Name, gateway.Namespace, pointer.Int32(10)) checkNumberOfInstances(t, k8sClient, consulClient, gateway.Name, gateway.Namespace, maxInstances, gateway) scale(t, k8sClient, gateway.Name, gateway.Namespace, pointer.Int32(0)) checkNumberOfInstances(t, k8sClient, consulClient, gateway.Name, gateway.Namespace, minInstances, gateway)