From b6dadfc37b83c47fcdbbc5600fc39619631b3cce Mon Sep 17 00:00:00 2001 From: Derek Menteer <105233703+hashi-derek@users.noreply.github.com> Date: Mon, 11 Sep 2023 08:17:15 -0500 Subject: [PATCH] [NET-5399] Improve token fetching performance for endpoints controller. (#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. --- .changelog/2910.txt | 3 +++ acceptance/go.mod | 4 ++-- acceptance/go.sum | 8 ++++---- .../tests/connect/connect_inject_test.go | 18 ++++++++++++++++++ .../endpoints/endpoints_controller.go | 15 ++++++++++++--- control-plane/go.mod | 4 ++-- control-plane/go.sum | 8 ++++---- 7 files changed, 45 insertions(+), 15 deletions(-) create mode 100644 .changelog/2910.txt diff --git a/.changelog/2910.txt b/.changelog/2910.txt new file mode 100644 index 0000000000..8a1e40f6c2 --- /dev/null +++ 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 index 2da38c70b7..20dd19ba34 100644 --- 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 index 0f33cfadc8..44d0fdd5f1 100644 --- 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 index ff8b5bf2df..39235db36e 100644 --- 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 index fb44a2a5ba..29d9f5b497 100644 --- 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 index 2dcd78848c..0083fedc27 100644 --- 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 index dec3ba9eb4..5e24ac60b4 100644 --- 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=