[NET-5399] Improve token fetching performance for endpoints controlle…

…r. (#2920) Improve token fetching performance for endpoints controller. Prior to this change, the endpoints controller would list all ACL tokens in a namespace when a service instance is being deleted. This commit improves the performance by querying only the necessary subset of tokens by service-identity / service-name.
hashicorp · Sep 11, 2023 · b6dadfc · b6dadfc
1 parent 1372b85
commit b6dadfc
Show file tree

Hide file tree

Showing 7 changed files with 45 additions and 15 deletions.
diff --git a/.changelog/2910.txt b/.changelog/2910.txt
@@ -0,0 +1,3 @@
+```release-note:improvement
+control-plane: Improve performance for pod deletions by reducing the number of fetched tokens.
+```
diff --git a/acceptance/go.mod b/acceptance/go.mod
@@ -5,8 +5,8 @@ go 1.20
 require (
 	github.com/gruntwork-io/terratest v0.31.2
 	github.com/hashicorp/consul-k8s/control-plane v0.0.0-20230724205934-5b57e6340dff
-	github.com/hashicorp/consul/api v1.22.0-rc1
-	github.com/hashicorp/consul/sdk v0.14.0-rc1
+	github.com/hashicorp/consul/api v1.22.0-rc1.0.20230906181627-0b2e31bbfa2c
+	github.com/hashicorp/consul/sdk v0.14.1
 	github.com/hashicorp/go-uuid v1.0.3
 	github.com/hashicorp/go-version v1.6.0
 	github.com/hashicorp/serf v0.10.1

diff --git a/acceptance/go.sum b/acceptance/go.sum
@@ -337,10 +337,10 @@ github.com/gruntwork-io/terratest v0.31.2 h1:xvYHA80MUq5kx670dM18HInewOrrQrAN+Xb
 github.com/gruntwork-io/terratest v0.31.2/go.mod h1:EEgJie28gX/4AD71IFqgMj6e99KP5mi81hEtzmDjxTo=
 github.com/hashicorp/consul-k8s/control-plane v0.0.0-20230724205934-5b57e6340dff h1:E5o8N01LGheJfgXAbFgjXd37DgnT7MmfeUnmXFMgSuc=
 github.com/hashicorp/consul-k8s/control-plane v0.0.0-20230724205934-5b57e6340dff/go.mod h1:stDdIOMKKlo8hZMViCPPNiLCNuYea2eQofHzOPZUz/o=
-github.com/hashicorp/consul/api v1.22.0-rc1 h1:ePmGqndeMgaI38KUbSA/CqTzeEAIogXyWnfNJzglo70=
-github.com/hashicorp/consul/api v1.22.0-rc1/go.mod h1:wtduXtbAqSGtBdi3tyA5SSAYGAG51rBejV9SEUBciMY=
-github.com/hashicorp/consul/sdk v0.14.0-rc1 h1:PuETOfN0uxl28i0Pq6rK7TBCrIl7psMbL0YTSje4KvM=
-github.com/hashicorp/consul/sdk v0.14.0-rc1/go.mod h1:gHYeuDa0+0qRAD6Wwr6yznMBvBwHKoxSBoW5l73+saE=
+github.com/hashicorp/consul/api v1.22.0-rc1.0.20230906181627-0b2e31bbfa2c h1:BTZmWmB9yo9lAoYTErVPvaBinPL/OvI8uIrUIC6ung8=
+github.com/hashicorp/consul/api v1.22.0-rc1.0.20230906181627-0b2e31bbfa2c/go.mod h1:NZJGRFYruc/80wYowkPFCp1LbGmJC9L8izrwfyVx/Wg=
+github.com/hashicorp/consul/sdk v0.14.1 h1:ZiwE2bKb+zro68sWzZ1SgHF3kRMBZ94TwOCFRF4ylPs=
+github.com/hashicorp/consul/sdk v0.14.1/go.mod h1:vFt03juSzocLRFo59NkeQHHmQa6+g7oU0pfzdI1mUhg=
 github.com/hashicorp/errwrap v1.0.0/go.mod h1:YH+1FKiLXxHSkmPseP+kNlulaMuP3n2brvKWEqk/Jc4=
 github.com/hashicorp/errwrap v1.1.0 h1:OxrOeh75EUXMY8TBjag2fzXGZ40LB6IKw45YeGUDY2I=
 github.com/hashicorp/errwrap v1.1.0/go.mod h1:YH+1FKiLXxHSkmPseP+kNlulaMuP3n2brvKWEqk/Jc4=

diff --git a/acceptance/tests/connect/connect_inject_test.go b/acceptance/tests/connect/connect_inject_test.go
@@ -134,6 +134,24 @@ func TestConnectInject_CleanupKilledPods(t *testing.T) {
 			require.Len(t, pods.Items, 1)
 			podName := pods.Items[0].Name
 
+			// Ensure the token exists
+			if secure {
+				retry.Run(t, func(r *retry.R) {
+					tokens, _, err := consulClient.ACL().TokenListFiltered(
+						api.ACLTokenFilterOptions{ServiceName: "static-client"}, nil)
+					require.NoError(r, err)
+					// Ensure that the tokens exist. Note that we must iterate over the tokens and scan for the name,
+					// because older versions of Consul do not support the filtered query param and will return
+					// the full list of tokens instead.
+					count := 0
+					for _, t := range tokens {
+						if len(t.ServiceIdentities) > 0 && t.ServiceIdentities[0].ServiceName == "static-client" {
+							count++
+						}
+					}
+					require.Greater(r, count, 0)
+				})
+			}
 			logger.Logf(t, "force killing the static-client pod %q", podName)
 			var gracePeriod int64 = 0
 			err = ctx.KubernetesClient(t).CoreV1().Pods(ns).Delete(context.Background(), podName, metav1.DeleteOptions{GracePeriodSeconds: &gracePeriod})

diff --git a/control-plane/connect-inject/controllers/endpoints/endpoints_controller.go b/control-plane/connect-inject/controllers/endpoints/endpoints_controller.go
@@ -965,9 +965,18 @@ func (r *Controller) deleteACLTokensForServiceInstance(apiClient *api.Client, sv
 		return nil
 	}
 
-	tokens, _, err := apiClient.ACL().TokenList(&api.QueryOptions{
-		Namespace: svc.Namespace,
-	})
+	// Note that while the `TokenListFiltered` query below should only return a subset
+	// of tokens from the Consul servers, it will return an unfiltered list on older
+	// versions of Consul (because they do not yet support the query parameter).
+	// To be safe, we still need to iterate over tokens and assert the service name
+	// matches as well.
+	tokens, _, err := apiClient.ACL().TokenListFiltered(
+		api.ACLTokenFilterOptions{
+			ServiceName: svc.Service,
+		},
+		&api.QueryOptions{
+			Namespace: svc.Namespace,
+		})
 	if err != nil {
 		return fmt.Errorf("failed to get a list of tokens from Consul: %s", err)
 	}

diff --git a/control-plane/go.mod b/control-plane/go.mod
@@ -10,8 +10,8 @@ require (
 	github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510
 	github.com/hashicorp/consul-k8s/control-plane/cni v0.0.0-20230511143918-bd16ab83383d
 	github.com/hashicorp/consul-server-connection-manager v0.1.3
-	github.com/hashicorp/consul/api v1.22.0-rc1
-	github.com/hashicorp/consul/sdk v0.14.0-rc1
+	github.com/hashicorp/consul/api v1.22.0-rc1.0.20230906181627-0b2e31bbfa2c
+	github.com/hashicorp/consul/sdk v0.14.1
 	github.com/hashicorp/go-bexpr v0.1.11
 	github.com/hashicorp/go-discover v0.0.0-20230519164032-214571b6a530
 	github.com/hashicorp/go-hclog v1.5.0

diff --git a/control-plane/go.sum b/control-plane/go.sum
@@ -264,12 +264,12 @@ github.com/hashicorp/consul-k8s/control-plane/cni v0.0.0-20230511143918-bd16ab83
 github.com/hashicorp/consul-k8s/control-plane/cni v0.0.0-20230511143918-bd16ab83383d/go.mod h1:IHIHMzkoMwlv6rLsgwcoFBVYupR7/1pKEOHBMjD4L0k=
 github.com/hashicorp/consul-server-connection-manager v0.1.3 h1:fxsZ15XBNNWhV26yBVdCcnxHwSRgf9wqHGS2ZVCQIhc=
 github.com/hashicorp/consul-server-connection-manager v0.1.3/go.mod h1:Md2IGKaFJ4ek9GUA0pW1S2R60wpquMOUs27GiD9kZd0=
-github.com/hashicorp/consul/api v1.22.0-rc1 h1:ePmGqndeMgaI38KUbSA/CqTzeEAIogXyWnfNJzglo70=
-github.com/hashicorp/consul/api v1.22.0-rc1/go.mod h1:wtduXtbAqSGtBdi3tyA5SSAYGAG51rBejV9SEUBciMY=
+github.com/hashicorp/consul/api v1.22.0-rc1.0.20230906181627-0b2e31bbfa2c h1:BTZmWmB9yo9lAoYTErVPvaBinPL/OvI8uIrUIC6ung8=
+github.com/hashicorp/consul/api v1.22.0-rc1.0.20230906181627-0b2e31bbfa2c/go.mod h1:NZJGRFYruc/80wYowkPFCp1LbGmJC9L8izrwfyVx/Wg=
 github.com/hashicorp/consul/proto-public v0.1.0 h1:O0LSmCqydZi363hsqc6n2v5sMz3usQMXZF6ziK3SzXU=
 github.com/hashicorp/consul/proto-public v0.1.0/go.mod h1:vs2KkuWwtjkIgA5ezp4YKPzQp4GitV+q/+PvksrA92k=
-github.com/hashicorp/consul/sdk v0.14.0-rc1 h1:PuETOfN0uxl28i0Pq6rK7TBCrIl7psMbL0YTSje4KvM=
-github.com/hashicorp/consul/sdk v0.14.0-rc1/go.mod h1:gHYeuDa0+0qRAD6Wwr6yznMBvBwHKoxSBoW5l73+saE=
+github.com/hashicorp/consul/sdk v0.14.1 h1:ZiwE2bKb+zro68sWzZ1SgHF3kRMBZ94TwOCFRF4ylPs=
+github.com/hashicorp/consul/sdk v0.14.1/go.mod h1:vFt03juSzocLRFo59NkeQHHmQa6+g7oU0pfzdI1mUhg=
 github.com/hashicorp/errwrap v1.0.0/go.mod h1:YH+1FKiLXxHSkmPseP+kNlulaMuP3n2brvKWEqk/Jc4=
 github.com/hashicorp/errwrap v1.1.0 h1:OxrOeh75EUXMY8TBjag2fzXGZ40LB6IKw45YeGUDY2I=
 github.com/hashicorp/errwrap v1.1.0/go.mod h1:YH+1FKiLXxHSkmPseP+kNlulaMuP3n2brvKWEqk/Jc4=