diff --git a/internal/clients/manager.go b/internal/clients/manager.go index d9e9f8bb3b..c009e681c7 100644 --- a/internal/clients/manager.go +++ b/internal/clients/manager.go @@ -228,13 +228,17 @@ func (c *AdminAPIClientsManager) gatewayClientsReconciliationLoop() { // It will adjust lists of gateway clients and notify subscribers about the change if readyGatewayClients list has // changed. func (c *AdminAPIClientsManager) onDiscoveredAdminAPIsNotification(discoveredAdminAPIs []adminapi.DiscoveredAdminAPI) { - c.lock.Lock() - defer c.lock.Unlock() - c.logger.Debug("received notification about Admin API addresses change") - if clientsChanged := c.adjustGatewayClients(discoveredAdminAPIs); clientsChanged { - // Notify subscribers that the clients list has been updated. - c.logger.Debug("notifying subscribers about gateway clients change") + + changed := func() bool { + c.lock.Lock() + defer c.lock.Unlock() + clientsChanged := c.adjustGatewayClients(discoveredAdminAPIs) + readinessChanged := c.reconcileGatewayClientsReadiness() + return clientsChanged || readinessChanged + }() + + if changed { c.notifyGatewayClientsSubscribers() } } @@ -242,13 +246,15 @@ func (c *AdminAPIClientsManager) onDiscoveredAdminAPIsNotification(discoveredAdm // onReadinessReconciliationTick is called on every readinessReconciliationTicker tick. It will reconcile readiness // of all gateway clients and notify subscribers about the change if readyGatewayClients list has changed. func (c *AdminAPIClientsManager) onReadinessReconciliationTick() { - c.lock.Lock() - defer c.lock.Unlock() - c.logger.Debug("reconciling readiness of gateway clients") - if clientsChanged := c.reconcileGatewayClientsReadiness(); clientsChanged { - // Notify subscribers that the clients list has been updated. - c.logger.Debug("notifying subscribers about gateway clients change") + + changed := func() bool { + c.lock.Lock() + defer c.lock.Unlock() + return c.reconcileGatewayClientsReadiness() + }() + + if changed { c.notifyGatewayClientsSubscribers() } } @@ -258,7 +264,7 @@ func (c *AdminAPIClientsManager) onReadinessReconciliationTick() { // of the discovered Admin APIs and creates only those clients that we don't have. // It returns true if the gatewayClients slice has been changed, false otherwise. func (c *AdminAPIClientsManager) adjustGatewayClients(discoveredAdminAPIs []adminapi.DiscoveredAdminAPI) (changed bool) { - // Short circuit, + // Short circuit. if len(discoveredAdminAPIs) == 0 { // If we have no clients and the provided list is empty, it means we're in sync. No change was made. if len(c.readyGatewayClients) == 0 && len(c.pendingGatewayClients) == 0 { @@ -299,8 +305,7 @@ func (c *AdminAPIClientsManager) adjustGatewayClients(discoveredAdminAPIs []admi } } - readinessChanged := c.reconcileGatewayClientsReadiness() - return changed || readinessChanged + return changed } // reconcileGatewayClientsReadiness reconciles the readiness of the gateway clients. It ensures that the clients on the @@ -313,6 +318,11 @@ func (c *AdminAPIClientsManager) reconcileGatewayClientsReadiness() bool { // It's to ensure that the readiness is not reconciled too often when we receive a lot of notifications. defer c.readinessReconciliationTicker.Reset(DefaultReadinessReconciliationInterval) + // Short circuit. + if len(c.readyGatewayClients) == 0 && len(c.pendingGatewayClients) == 0 { + return false + } + readinessCheckResult := c.readinessChecker.CheckReadiness( c.ctx, lo.MapToSlice(c.readyGatewayClients, func(_ string, cl *adminapi.Client) AlreadyCreatedClient { return cl }), @@ -333,6 +343,7 @@ func (c *AdminAPIClientsManager) reconcileGatewayClientsReadiness() bool { // notifyGatewayClientsSubscribers sends notifications to all subscribers that have called SubscribeToGatewayClientsChanges. func (c *AdminAPIClientsManager) notifyGatewayClientsSubscribers() { + c.logger.Debug("notifying subscribers about gateway clients change") for _, sub := range c.gatewayClientsChangesSubscribers { select { case <-c.ctx.Done(): diff --git a/test/e2e/helpers_test.go b/test/e2e/helpers_test.go index b1d2efc195..4e9e191c87 100644 --- a/test/e2e/helpers_test.go +++ b/test/e2e/helpers_test.go @@ -294,7 +294,7 @@ func waitForDeploymentRollout(ctx context.Context, t *testing.T, env environment } if err := allUpdatedReplicasRolledOutAndReady(deployment); err != nil { - t.Logf("controller deployment not ready: %s", err) + t.Logf("%s/%s deployment not ready: %s", namespace, name, err) return false } @@ -900,3 +900,28 @@ func scaleDeployment(ctx context.Context, t *testing.T, env environments.Environ return deployment.Status.ReadyReplicas == replicas }, time.Minute*3, time.Second, "deployment %s did not scale to %d replicas", deployment.Name, replicas) } + +func (d Deployments) Restart(ctx context.Context, t *testing.T, env environments.Environment) { + t.Helper() + + err := env.Cluster().Client().CoreV1().Pods(d.ControllerNN.Namespace).DeleteCollection(ctx, metav1.DeleteOptions{}, metav1.ListOptions{ + LabelSelector: metav1.FormatLabelSelector(&metav1.LabelSelector{ + MatchLabels: map[string]string{ + "app": d.ControllerNN.Name, + }, + }), + }) + require.NoError(t, err, "failed to delete controller pods") + + err = env.Cluster().Client().CoreV1().Pods(d.ControllerNN.Namespace).DeleteCollection(ctx, metav1.DeleteOptions{}, metav1.ListOptions{ + LabelSelector: metav1.FormatLabelSelector(&metav1.LabelSelector{ + MatchLabels: map[string]string{ + "app": d.ProxyNN.Name, + }, + }), + }) + require.NoError(t, err, "failed to delete proxy pods") + + waitForDeploymentRollout(ctx, t, env, d.ControllerNN.Namespace, d.ControllerNN.Name) + waitForDeploymentRollout(ctx, t, env, d.ProxyNN.Namespace, d.ProxyNN.Name) +} diff --git a/test/e2e/kuma_test.go b/test/e2e/kuma_test.go index 336393d12b..a04d035ca2 100644 --- a/test/e2e/kuma_test.go +++ b/test/e2e/kuma_test.go @@ -28,13 +28,8 @@ func TestDeployAllInOneDBLESSKuma(t *testing.T) { require.NoError(t, kuma.EnableMeshForNamespace(ctx, env.Cluster(), "kong")) require.NoError(t, kuma.EnableMeshForNamespace(ctx, env.Cluster(), "default")) - // scale to force a restart of pods and trigger mesh injection (we can't annotate the Kong namespace in advance, - // it gets clobbered by deployKong()). is there a "rollout restart" in client-go? who knows! - scaleDeployment(ctx, t, env, deployments.ProxyNN, 0) - scaleDeployment(ctx, t, env, deployments.ControllerNN, 0) - - scaleDeployment(ctx, t, env, deployments.ProxyNN, 2) - scaleDeployment(ctx, t, env, deployments.ControllerNN, 2) + // Restart Kong pods to trigger mesh injection. + deployments.Restart(ctx, t, env) t.Log("running ingress tests to verify all-in-one deployed ingress controller and proxy are functional") deployIngressWithEchoBackends(ctx, t, env, numberOfEchoBackends) diff --git a/test/e2e/upgrade_test.go b/test/e2e/upgrade_test.go index 35495956ef..d96d69441a 100644 --- a/test/e2e/upgrade_test.go +++ b/test/e2e/upgrade_test.go @@ -110,8 +110,9 @@ func testManifestsUpgrade( t.Logf("deploying target version of kong manifests: %s", testParams.toManifestPath) deployments := ManifestDeploy{ - Path: testParams.toManifestPath, - SkipTestPatches: true, + Path: testParams.toManifestPath, + // Do not skip test patches - we want to verify that upgrade works with an image override in target manifest. + SkipTestPatches: false, }.Run(ctx, t, env) if featureGates := testParams.controllerFeatureGates; featureGates != "" {