From 10f5795753a7a3db1ef9277f33a29c4495ceb41c Mon Sep 17 00:00:00 2001 From: Iryna Shustava Date: Mon, 1 Aug 2022 13:30:14 -0600 Subject: [PATCH 01/29] peering: set peering server config only when peering Helm value is true --- .../templates/server-config-configmap.yaml | 5 ++++ .../test/unit/server-config-configmap.bats | 25 +++++++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/charts/consul/templates/server-config-configmap.yaml b/charts/consul/templates/server-config-configmap.yaml index 113a1df22a..e35311a9c7 100644 --- a/charts/consul/templates/server-config-configmap.yaml +++ b/charts/consul/templates/server-config-configmap.yaml @@ -32,6 +32,11 @@ data: }, "recursors": {{ .Values.global.recursors | toJson }}, "retry_join": ["{{template "consul.fullname" . }}-server.{{ .Release.Namespace }}.svc:{{ .Values.server.ports.serflan.port }}"], + {{- if .Values.global.peering.enabled }} + "peering": { + "enabled": true + }, + {{- end }} "server": true } {{- $vaultConnectCAEnabled := and .Values.global.secretsBackend.vault.connectCA.address .Values.global.secretsBackend.vault.connectCA.rootPKIPath .Values.global.secretsBackend.vault.connectCA.intermediatePKIPath -}} diff --git a/charts/consul/test/unit/server-config-configmap.bats b/charts/consul/test/unit/server-config-configmap.bats index 9b6c4206de..d31cbe774c 100755 --- a/charts/consul/test/unit/server-config-configmap.bats +++ b/charts/consul/test/unit/server-config-configmap.bats @@ -905,3 +905,28 @@ load _helpers [ "${actual}" = null ] } + +#-------------------------------------------------------------------- +# peering + +@test "server/ConfigMap: peering configuration is unspecified by default" { + cd `chart_dir` + local actual=$(helm template \ + -s templates/server-config-configmap.yaml \ + . | tee /dev/stderr | + yq -r '.data["server.json"]' | jq -r .peering | tee /dev/stderr) + + [ "${actual}" = "null" ] +} + +@test "server/ConfigMap: peering configuration is set by if global.peering.enabled is true" { + cd `chart_dir` + local actual=$(helm template \ + -s templates/server-config-configmap.yaml \ + --set 'global.peering.enabled=true' \ + --set 'connectInject.enabled=true' \ + . | tee /dev/stderr | + yq -r '.data["server.json"]' | jq -r .peering.enabled | tee /dev/stderr) + + [ "${actual}" = "true" ] +} From 6c1413da1e0252009753237495da4e06ffc79328 Mon Sep 17 00:00:00 2001 From: Iryna Shustava Date: Mon, 1 Aug 2022 15:21:02 -0600 Subject: [PATCH 02/29] acceptance: use 1.13 image in all acceptance tests --- .circleci/config.yml | 2 ++ acceptance/tests/peering/peering_connect_namespaces_test.go | 2 -- acceptance/tests/peering/peering_connect_test.go | 2 -- 3 files changed, 2 insertions(+), 4 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index b1bcce48c7..2cf2ab4c4b 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -101,6 +101,7 @@ commands: ${ENABLE_ENTERPRISE:+-enable-enterprise} \ -enable-multi-cluster \ -debug-directory="$TEST_RESULTS/debug" \ + -consul-image=docker.mirror.hashicorp.services/hashicorppreview/consul-enterprise:1.13-dev \ -consul-k8s-image=<< parameters.consul-k8s-image >> then echo "Tests in ${pkg} failed, aborting early" @@ -132,6 +133,7 @@ commands: -enable-multi-cluster \ ${ENABLE_ENTERPRISE:+-enable-enterprise} \ -debug-directory="$TEST_RESULTS/debug" \ + -consul-image=docker.mirror.hashicorp.services/hashicorppreview/consul-enterprise:1.13-dev \ -consul-k8s-image=<< parameters.consul-k8s-image >> jobs: diff --git a/acceptance/tests/peering/peering_connect_namespaces_test.go b/acceptance/tests/peering/peering_connect_namespaces_test.go index 78a1c58436..ce5733fdd2 100644 --- a/acceptance/tests/peering/peering_connect_namespaces_test.go +++ b/acceptance/tests/peering/peering_connect_namespaces_test.go @@ -95,8 +95,6 @@ func TestPeering_ConnectNamespaces(t *testing.T) { "global.peering.enabled": "true", "global.enableConsulNamespaces": "true", - "global.image": "thisisnotashwin/consul@sha256:b1d3f59406adf5fb9a3bee4ded058e619d3a186e83b2e2dc14d6da3f28a7073d", - "global.tls.enabled": "true", "global.tls.httpsOnly": strconv.FormatBool(c.ACLsAndAutoEncryptEnabled), "global.tls.enableAutoEncrypt": strconv.FormatBool(c.ACLsAndAutoEncryptEnabled), diff --git a/acceptance/tests/peering/peering_connect_test.go b/acceptance/tests/peering/peering_connect_test.go index d18d6bee70..f5c9722682 100644 --- a/acceptance/tests/peering/peering_connect_test.go +++ b/acceptance/tests/peering/peering_connect_test.go @@ -54,8 +54,6 @@ func TestPeering_Connect(t *testing.T) { commonHelmValues := map[string]string{ "global.peering.enabled": "true", - "global.image": "thisisnotashwin/consul@sha256:b1d3f59406adf5fb9a3bee4ded058e619d3a186e83b2e2dc14d6da3f28a7073d", - "global.tls.enabled": "true", "global.tls.httpsOnly": strconv.FormatBool(c.ACLsAndAutoEncryptEnabled), "global.tls.enableAutoEncrypt": strconv.FormatBool(c.ACLsAndAutoEncryptEnabled), From 51181ddccbcb2a2a57aa8e56cfc502ec9199fa54 Mon Sep 17 00:00:00 2001 From: Iryna Shustava Date: Mon, 1 Aug 2022 18:31:20 -0600 Subject: [PATCH 03/29] try running OSS only tests --- .circleci/config.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 2cf2ab4c4b..9a01d03faa 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -84,7 +84,7 @@ commands: # Enterprise tests can't run on fork PRs because they require # a secret. if [ -z "$CIRCLE_PR_NUMBER" ]; then - ENABLE_ENTERPRISE=true + ENABLE_ENTERPRISE=false fi # We have to run the tests for each package separately so that we can @@ -101,7 +101,7 @@ commands: ${ENABLE_ENTERPRISE:+-enable-enterprise} \ -enable-multi-cluster \ -debug-directory="$TEST_RESULTS/debug" \ - -consul-image=docker.mirror.hashicorp.services/hashicorppreview/consul-enterprise:1.13-dev \ + -consul-image=docker.mirror.hashicorp.services/hashicorppreview/consul:1.13-dev \ -consul-k8s-image=<< parameters.consul-k8s-image >> then echo "Tests in ${pkg} failed, aborting early" From 648c031e4054b8acc8047bfc626d4c3a9df6ad10 Mon Sep 17 00:00:00 2001 From: Iryna Shustava Date: Tue, 2 Aug 2022 10:27:44 -0600 Subject: [PATCH 04/29] Revert "try running OSS only tests" This reverts commit 51181ddccbcb2a2a57aa8e56cfc502ec9199fa54. --- .circleci/config.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 9a01d03faa..2cf2ab4c4b 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -84,7 +84,7 @@ commands: # Enterprise tests can't run on fork PRs because they require # a secret. if [ -z "$CIRCLE_PR_NUMBER" ]; then - ENABLE_ENTERPRISE=false + ENABLE_ENTERPRISE=true fi # We have to run the tests for each package separately so that we can @@ -101,7 +101,7 @@ commands: ${ENABLE_ENTERPRISE:+-enable-enterprise} \ -enable-multi-cluster \ -debug-directory="$TEST_RESULTS/debug" \ - -consul-image=docker.mirror.hashicorp.services/hashicorppreview/consul:1.13-dev \ + -consul-image=docker.mirror.hashicorp.services/hashicorppreview/consul-enterprise:1.13-dev \ -consul-k8s-image=<< parameters.consul-k8s-image >> then echo "Tests in ${pkg} failed, aborting early" From 49cfe8ab095f129f3bb5f6d86b763a629f2a8407 Mon Sep 17 00:00:00 2001 From: Iryna Shustava Date: Tue, 2 Aug 2022 10:28:56 -0600 Subject: [PATCH 05/29] only run peering test --- .circleci/config.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.circleci/config.yml b/.circleci/config.yml index 2cf2ab4c4b..6d24ba07ca 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -100,6 +100,7 @@ commands: << parameters.additional-flags >> \ ${ENABLE_ENTERPRISE:+-enable-enterprise} \ -enable-multi-cluster \ + -run TestPeering_ConnectNamespaces/default_destination_namespace \ -debug-directory="$TEST_RESULTS/debug" \ -consul-image=docker.mirror.hashicorp.services/hashicorppreview/consul-enterprise:1.13-dev \ -consul-k8s-image=<< parameters.consul-k8s-image >> From 9e10072ed0ea0a0f2ef76cee13068fb2c6382144 Mon Sep 17 00:00:00 2001 From: Iryna Shustava Date: Tue, 2 Aug 2022 12:44:08 -0600 Subject: [PATCH 06/29] add timeout and print out peering token --- acceptance/framework/k8s/helpers.go | 10 ++++++++++ acceptance/framework/k8s/kubectl.go | 4 ++-- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/acceptance/framework/k8s/helpers.go b/acceptance/framework/k8s/helpers.go index 3f981336ee..c646fd2ab0 100644 --- a/acceptance/framework/k8s/helpers.go +++ b/acceptance/framework/k8s/helpers.go @@ -2,6 +2,8 @@ package k8s import ( "context" + "encoding/base64" + "encoding/json" "fmt" "strings" "testing" @@ -136,6 +138,14 @@ func CopySecret(t *testing.T, sourceContext, destContext environment.TestContext secret.ResourceVersion = "" require.NoError(r, err) }) + secretData := secret.Data["data"] + + var token map[string]interface{} + // Decode the token to extract the ServerName and PeerID from the token. CA is always NULL. + decodeBytes, err := base64.StdEncoding.DecodeString(string(secretData)) + require.NoError(t, err) + err = json.Unmarshal(decodeBytes, &token) + logger.Log(t, "peering token", token) _, err = destContext.KubernetesClient(t).CoreV1().Secrets(destContext.KubectlOptions(t).Namespace).Create(context.Background(), secret, metav1.CreateOptions{}) require.NoError(t, err) } diff --git a/acceptance/framework/k8s/kubectl.go b/acceptance/framework/k8s/kubectl.go index 318cde217e..7382cb0507 100644 --- a/acceptance/framework/k8s/kubectl.go +++ b/acceptance/framework/k8s/kubectl.go @@ -94,7 +94,7 @@ func KubectlApplyK(t *testing.T, options *k8s.KubectlOptions, kustomizeDir strin // deletes it from the cluster by running 'kubectl delete -f'. // If there's an error deleting the file, fail the test. func KubectlDelete(t *testing.T, options *k8s.KubectlOptions, configPath string) { - _, err := RunKubectlAndGetOutputE(t, options, "delete", "-f", configPath) + _, err := RunKubectlAndGetOutputE(t, options, "delete", "--timeout=60s", "-f", configPath) require.NoError(t, err) } @@ -102,7 +102,7 @@ func KubectlDelete(t *testing.T, options *k8s.KubectlOptions, configPath string) // deletes it from the cluster by running 'kubectl delete -k'. // If there's an error deleting the file, fail the test. func KubectlDeleteK(t *testing.T, options *k8s.KubectlOptions, kustomizeDir string) { - _, err := RunKubectlAndGetOutputE(t, options, "delete", "-k", kustomizeDir) + _, err := RunKubectlAndGetOutputE(t, options, "delete", "--timeout=60s", "-k", kustomizeDir) require.NoError(t, err) } From 26b4f84e2c0c8cfeeaab029cfa706cade6534885 Mon Sep 17 00:00:00 2001 From: Iryna Shustava Date: Tue, 2 Aug 2022 14:18:15 -0600 Subject: [PATCH 07/29] add more debug output --- .circleci/config.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 6d24ba07ca..004fd2f41e 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -102,7 +102,7 @@ commands: -enable-multi-cluster \ -run TestPeering_ConnectNamespaces/default_destination_namespace \ -debug-directory="$TEST_RESULTS/debug" \ - -consul-image=docker.mirror.hashicorp.services/hashicorppreview/consul-enterprise:1.13-dev \ + -consul-image=ishustava/consul-dev@sha256:d51bc7f150a5930ed7c1a0d8b688767faff8996dd845c74d718c67b464f1ceb9 \ -consul-k8s-image=<< parameters.consul-k8s-image >> then echo "Tests in ${pkg} failed, aborting early" From 80e60ee10de11427222528f9419307ca6d203283 Mon Sep 17 00:00:00 2001 From: Iryna Shustava Date: Tue, 2 Aug 2022 14:21:51 -0600 Subject: [PATCH 08/29] Update logging image --- .circleci/config.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 004fd2f41e..8f22cb8c20 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -102,7 +102,7 @@ commands: -enable-multi-cluster \ -run TestPeering_ConnectNamespaces/default_destination_namespace \ -debug-directory="$TEST_RESULTS/debug" \ - -consul-image=ishustava/consul-dev@sha256:d51bc7f150a5930ed7c1a0d8b688767faff8996dd845c74d718c67b464f1ceb9 \ + -consul-image=ishustava/consul-dev@sha256:5af2f804bc47043578e6793d72923760a11f51c3dd3b4ef82ecec221f39e0af2 \ -consul-k8s-image=<< parameters.consul-k8s-image >> then echo "Tests in ${pkg} failed, aborting early" From 295928be193288238b2b8b8d7f6de69e7542bf33 Mon Sep 17 00:00:00 2001 From: Iryna Shustava Date: Tue, 2 Aug 2022 15:32:57 -0600 Subject: [PATCH 09/29] use image with potential deletion fix --- .circleci/config.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 8f22cb8c20..681f05b4fc 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -102,7 +102,7 @@ commands: -enable-multi-cluster \ -run TestPeering_ConnectNamespaces/default_destination_namespace \ -debug-directory="$TEST_RESULTS/debug" \ - -consul-image=ishustava/consul-dev@sha256:5af2f804bc47043578e6793d72923760a11f51c3dd3b4ef82ecec221f39e0af2 \ + -consul-image=ishustava/consul-dev@sha256:02da8d365c11b1aac13b1725e59c1765292df9d40af22a8da5f3880615703e66 \ -consul-k8s-image=<< parameters.consul-k8s-image >> then echo "Tests in ${pkg} failed, aborting early" From 8d0f2515d12ef8e334c008ae8776b4ef43d8c1e1 Mon Sep 17 00:00:00 2001 From: Iryna Shustava Date: Tue, 2 Aug 2022 16:19:56 -0600 Subject: [PATCH 10/29] try another deletion fix image --- .circleci/config.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 681f05b4fc..6dc2be32f8 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -102,7 +102,7 @@ commands: -enable-multi-cluster \ -run TestPeering_ConnectNamespaces/default_destination_namespace \ -debug-directory="$TEST_RESULTS/debug" \ - -consul-image=ishustava/consul-dev@sha256:02da8d365c11b1aac13b1725e59c1765292df9d40af22a8da5f3880615703e66 \ + -consul-image=ishustava/consul-dev@sha256:5dcd562830813e78f0b8a8448edc0dc16c3850b7b6b526431dcc61f891c91202 \ -consul-k8s-image=<< parameters.consul-k8s-image >> then echo "Tests in ${pkg} failed, aborting early" From 8837f963b5856e458d6f3ec7c198a29c7bcef234 Mon Sep 17 00:00:00 2001 From: Iryna Shustava Date: Tue, 2 Aug 2022 18:14:51 -0600 Subject: [PATCH 11/29] attempt to fix peering establish error --- .circleci/config.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 6dc2be32f8..ffc26b50f8 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -102,7 +102,7 @@ commands: -enable-multi-cluster \ -run TestPeering_ConnectNamespaces/default_destination_namespace \ -debug-directory="$TEST_RESULTS/debug" \ - -consul-image=ishustava/consul-dev@sha256:5dcd562830813e78f0b8a8448edc0dc16c3850b7b6b526431dcc61f891c91202 \ + -consul-image=ishustava/consul-dev@sha256:4d844c4680803fd664f1b84ef0891692135d260628df842f8eb671fd861fc79b \ -consul-k8s-image=<< parameters.consul-k8s-image >> then echo "Tests in ${pkg} failed, aborting early" From 4356669abb9832e402c960d57bc2faeeb3ad7879 Mon Sep 17 00:00:00 2001 From: Iryna Shustava Date: Tue, 2 Aug 2022 18:54:24 -0600 Subject: [PATCH 12/29] attempt2 to fix peering establish error --- .circleci/config.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index ffc26b50f8..ca5df5afa3 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -102,7 +102,7 @@ commands: -enable-multi-cluster \ -run TestPeering_ConnectNamespaces/default_destination_namespace \ -debug-directory="$TEST_RESULTS/debug" \ - -consul-image=ishustava/consul-dev@sha256:4d844c4680803fd664f1b84ef0891692135d260628df842f8eb671fd861fc79b \ + -consul-image=ishustava/consul-dev@sha256:6dc2ae18f50218b8948645d47c4cb7a30395a73cdb8137b4df6c0ad8a5a28ad4 \ -consul-k8s-image=<< parameters.consul-k8s-image >> then echo "Tests in ${pkg} failed, aborting early" From 4c8365fdfb78c11ced6480e1a7aa12e38dddbea8 Mon Sep 17 00:00:00 2001 From: Iryna Shustava Date: Tue, 2 Aug 2022 19:19:31 -0600 Subject: [PATCH 13/29] run all tests --- .circleci/config.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index ca5df5afa3..545fafa4b6 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -100,7 +100,6 @@ commands: << parameters.additional-flags >> \ ${ENABLE_ENTERPRISE:+-enable-enterprise} \ -enable-multi-cluster \ - -run TestPeering_ConnectNamespaces/default_destination_namespace \ -debug-directory="$TEST_RESULTS/debug" \ -consul-image=ishustava/consul-dev@sha256:6dc2ae18f50218b8948645d47c4cb7a30395a73cdb8137b4df6c0ad8a5a28ad4 \ -consul-k8s-image=<< parameters.consul-k8s-image >> From 691a790e16399a952006c93457454ddf9ba09826 Mon Sep 17 00:00:00 2001 From: Iryna Shustava Date: Tue, 2 Aug 2022 19:36:34 -0600 Subject: [PATCH 14/29] rebase ent against main --- .circleci/config.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 545fafa4b6..d65f7b16ae 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -101,7 +101,7 @@ commands: ${ENABLE_ENTERPRISE:+-enable-enterprise} \ -enable-multi-cluster \ -debug-directory="$TEST_RESULTS/debug" \ - -consul-image=ishustava/consul-dev@sha256:6dc2ae18f50218b8948645d47c4cb7a30395a73cdb8137b4df6c0ad8a5a28ad4 \ + -consul-image=ishustava/consul-dev@sha256:a26fd1564c6760caf225073baff36e961fb58e3f5235932070d5bf8c5d6830ab \ -consul-k8s-image=<< parameters.consul-k8s-image >> then echo "Tests in ${pkg} failed, aborting early" From ab80703f16ded658209f7d48f817bfcf14ea622e Mon Sep 17 00:00:00 2001 From: Iryna Shustava Date: Wed, 3 Aug 2022 10:14:30 -0600 Subject: [PATCH 15/29] Revert "run all tests" This reverts commit 4c8365fdfb78c11ced6480e1a7aa12e38dddbea8. --- .circleci/config.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.circleci/config.yml b/.circleci/config.yml index d65f7b16ae..a8b6d84298 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -100,6 +100,7 @@ commands: << parameters.additional-flags >> \ ${ENABLE_ENTERPRISE:+-enable-enterprise} \ -enable-multi-cluster \ + -run TestPeering_ConnectNamespaces/default_destination_namespace \ -debug-directory="$TEST_RESULTS/debug" \ -consul-image=ishustava/consul-dev@sha256:a26fd1564c6760caf225073baff36e961fb58e3f5235932070d5bf8c5d6830ab \ -consul-k8s-image=<< parameters.consul-k8s-image >> From 5411390d97d5141c7ecf5211527351e53f797580 Mon Sep 17 00:00:00 2001 From: Iryna Shustava Date: Wed, 3 Aug 2022 10:16:18 -0600 Subject: [PATCH 16/29] try another peering fix --- .circleci/config.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index a8b6d84298..1de6788b99 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -102,7 +102,7 @@ commands: -enable-multi-cluster \ -run TestPeering_ConnectNamespaces/default_destination_namespace \ -debug-directory="$TEST_RESULTS/debug" \ - -consul-image=ishustava/consul-dev@sha256:a26fd1564c6760caf225073baff36e961fb58e3f5235932070d5bf8c5d6830ab \ + -consul-image=ishustava/consul-dev@sha256:c8d4c6e0ba991325166c26083d5d5280eaea7394d9a13dbfe9cf87165252a949 \ -consul-k8s-image=<< parameters.consul-k8s-image >> then echo "Tests in ${pkg} failed, aborting early" From 8307e7ef1808691c4b970dbbe1073e27a0369a62 Mon Sep 17 00:00:00 2001 From: Iryna Shustava Date: Wed, 3 Aug 2022 11:52:40 -0600 Subject: [PATCH 17/29] comment out all except for one test case --- .../peering_connect_namespaces_test.go | 60 +++++++++---------- 1 file changed, 30 insertions(+), 30 deletions(-) diff --git a/acceptance/tests/peering/peering_connect_namespaces_test.go b/acceptance/tests/peering/peering_connect_namespaces_test.go index ce5733fdd2..ba6e4dea15 100644 --- a/acceptance/tests/peering/peering_connect_namespaces_test.go +++ b/acceptance/tests/peering/peering_connect_namespaces_test.go @@ -54,36 +54,36 @@ func TestPeering_ConnectNamespaces(t *testing.T) { false, false, }, - { - "single destination namespace", - staticServerNamespace, - false, - false, - }, - { - "mirror k8s namespaces", - staticServerNamespace, - true, - false, - }, - { - "default destination namespace", - defaultNamespace, - false, - true, - }, - { - "single destination namespace", - staticServerNamespace, - false, - true, - }, - { - "mirror k8s namespaces", - staticServerNamespace, - true, - true, - }, + //{ + // "single destination namespace", + // staticServerNamespace, + // false, + // false, + //}, + //{ + // "mirror k8s namespaces", + // staticServerNamespace, + // true, + // false, + //}, + //{ + // "default destination namespace", + // defaultNamespace, + // false, + // true, + //}, + //{ + // "single destination namespace", + // staticServerNamespace, + // false, + // true, + //}, + //{ + // "mirror k8s namespaces", + // staticServerNamespace, + // true, + // true, + //}, } for _, c := range cases { From 966b8a61ce6a626bafe3a3f335400e1987586ebe Mon Sep 17 00:00:00 2001 From: Iryna Shustava Date: Wed, 3 Aug 2022 17:56:32 -0500 Subject: [PATCH 18/29] add logging to the controller --- .../peering_acceptor_controller.go | 21 +++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/control-plane/connect-inject/peering_acceptor_controller.go b/control-plane/connect-inject/peering_acceptor_controller.go index da977d8e8a..7d795158dd 100644 --- a/control-plane/connect-inject/peering_acceptor_controller.go +++ b/control-plane/connect-inject/peering_acceptor_controller.go @@ -103,9 +103,11 @@ func (r *PeeringAcceptorController) Reconcile(ctx context.Context, req ctrl.Requ // existingStatusSecret will be nil if it doesn't exist, and have the contents of the secret if it does exist. var existingStatusSecret *corev1.Secret if statusSecretSet { + r.Log.Info("secret status is set; retrieving secret") existingStatusSecret, err = r.getExistingSecret(ctx, acceptor.SecretRef().Name, acceptor.Namespace) if err != nil { - r.updateStatusError(ctx, acceptor, KubernetesError, err) + r.Log.Error(err, "error retrieving existing secret", "name", acceptor.SecretRef().Name) + r.updateStatusError(ctx, acceptor, KubernetesError, err) // todo: why do set update status error here? return ctrl.Result{}, err } } @@ -157,17 +159,21 @@ func (r *PeeringAcceptorController) Reconcile(ctx context.Context, req ctrl.Requ // TODO(peering): Verify that the existing peering in Consul is an acceptor peer. If it is a dialing peer, an error should be thrown. + r.Log.Info("peering exists in Consul") // If the peering does exist in Consul, figure out whether to generate and store a new token by comparing the secret // in the status to the resource version of the secret. If no secret is specified in the status, shouldGenerate will // be set to true. var shouldGenerate bool var nameChanged bool if statusSecretSet { - shouldGenerate, nameChanged, err = shouldGenerateToken(acceptor, existingStatusSecret) + r.Log.Info("status secret set; determining if we need to generate token again") + shouldGenerate, nameChanged, err = r.shouldGenerateToken(acceptor, existingStatusSecret) if err != nil { + r.Log.Error(err, "error determining if we should generate token again") r.updateStatusError(ctx, acceptor, InternalError, err) return ctrl.Result{}, err } + r.Log.Info("finished determining if we should generate token", "shouldGenerate", shouldGenerate, "nameChanged", nameChanged) } else { shouldGenerate = true } @@ -205,15 +211,18 @@ func (r *PeeringAcceptorController) Reconcile(ctx context.Context, req ctrl.Requ // shouldGenerateToken returns whether a token should be generated, and whether the name of the secret has changed. It // compares the spec secret's name/key/backend and resource version with the name/key/backend and resource version of the status secret's. -func shouldGenerateToken(acceptor *consulv1alpha1.PeeringAcceptor, existingStatusSecret *corev1.Secret) (shouldGenerate bool, nameChanged bool, err error) { +func (r *PeeringAcceptorController) shouldGenerateToken(acceptor *consulv1alpha1.PeeringAcceptor, existingStatusSecret *corev1.Secret) (shouldGenerate bool, nameChanged bool, err error) { if acceptor.SecretRef() == nil { + r.Log.Info("shouldGenerateToken; secretRef is nil") return false, false, errors.New("shouldGenerateToken was called with an empty fields in the existing status") } // Compare the existing name, key, and backend. if acceptor.SecretRef().Name != acceptor.Secret().Name { + r.Log.Info("shouldGenerateToken; names don't match", "secret-ref-name", acceptor.SecretRef().Name, "spec name", acceptor.Secret().Name) return true, true, nil } if acceptor.SecretRef().Key != acceptor.Secret().Key { + r.Log.Info("shouldGenerateToken; keys don't match", "secret-ref-key", acceptor.SecretRef().Key, "spec key", acceptor.Secret().Key) return true, false, nil } // TODO(peering): remove this when validation webhook exists. @@ -225,18 +234,22 @@ func shouldGenerateToken(acceptor *consulv1alpha1.PeeringAcceptor, existingStatu if err != nil { return false, false, err } + r.Log.Info("shouldGenerateToken; checking peering version annotation", "version", peeringVersion) if acceptor.Status.LatestPeeringVersion == nil || *acceptor.Status.LatestPeeringVersion < peeringVersion { + r.Log.Info("shouldGenerateToken; should regenerate is true because either the latest version is nil or lower than peering version", "latest-version", acceptor.Status.LatestPeeringVersion) return true, false, nil } } // Compare the existing secret resource version. // Get the secret specified by the status, make sure it matches the status' secret.ResourceVersion. if existingStatusSecret != nil { + r.Log.Info("shouldGenerateToken; comparing resource versions of exsiting secret with the one in secret ref") if existingStatusSecret.ResourceVersion != acceptor.SecretRef().ResourceVersion { + r.Log.Info("shouldGenerateToken; should generate is true because versions don't match", "existing-status-secret", existingStatusSecret.ResourceVersion, "secret-ref-version", acceptor.SecretRef().ResourceVersion) return true, false, nil } - } else { + r.Log.Info("shouldGenerateToken, should generate is true because existing status secret is nil") return true, false, nil } return false, false, nil From 6c6be1f833fcff7111d97600647bb0a79c602605 Mon Sep 17 00:00:00 2001 From: Iryna Shustava Date: Wed, 3 Aug 2022 18:20:10 -0500 Subject: [PATCH 19/29] add more logging --- control-plane/connect-inject/peering_acceptor_controller.go | 1 + 1 file changed, 1 insertion(+) diff --git a/control-plane/connect-inject/peering_acceptor_controller.go b/control-plane/connect-inject/peering_acceptor_controller.go index 7d795158dd..b2965e8ab6 100644 --- a/control-plane/connect-inject/peering_acceptor_controller.go +++ b/control-plane/connect-inject/peering_acceptor_controller.go @@ -206,6 +206,7 @@ func (r *PeeringAcceptorController) Reconcile(ctx context.Context, req ctrl.Requ return ctrl.Result{}, err } + r.Log.Info("finished reconcile") return ctrl.Result{}, nil } From 972ca58e161bcac76c8819745245c633f34f0695 Mon Sep 17 00:00:00 2001 From: Iryna Shustava Date: Sat, 6 Aug 2022 11:48:11 -0600 Subject: [PATCH 20/29] don't regenerate token if secret version changed and get the object before updating --- .../peering_acceptor_controller.go | 42 +++++++++++++------ 1 file changed, 30 insertions(+), 12 deletions(-) diff --git a/control-plane/connect-inject/peering_acceptor_controller.go b/control-plane/connect-inject/peering_acceptor_controller.go index b2965e8ab6..3a9bfd8b03 100644 --- a/control-plane/connect-inject/peering_acceptor_controller.go +++ b/control-plane/connect-inject/peering_acceptor_controller.go @@ -4,6 +4,7 @@ import ( "context" "errors" "strconv" + "sync" "time" "github.com/go-logr/logr" @@ -32,6 +33,8 @@ type PeeringAcceptorController struct { Log logr.Logger Scheme *runtime.Scheme context.Context + + mutex sync.RWMutex } const ( @@ -150,7 +153,7 @@ func (r *PeeringAcceptorController) Reconcile(ctx context.Context, req ctrl.Requ } } // Store the state in the status. - err := r.updateStatus(ctx, acceptor, secretResourceVersion) + err := r.updateStatus(ctx, req.NamespacedName, secretResourceVersion) return ctrl.Result{}, err } else if err != nil { r.Log.Error(err, "failed to get Peering from Consul", "name", req.Name) @@ -202,7 +205,7 @@ func (r *PeeringAcceptorController) Reconcile(ctx context.Context, req ctrl.Requ } // Store the state in the status. - err := r.updateStatus(ctx, acceptor, secretResourceVersion) + err := r.updateStatus(ctx, req.NamespacedName, secretResourceVersion) return ctrl.Result{}, err } @@ -245,19 +248,33 @@ func (r *PeeringAcceptorController) shouldGenerateToken(acceptor *consulv1alpha1 // Get the secret specified by the status, make sure it matches the status' secret.ResourceVersion. if existingStatusSecret != nil { r.Log.Info("shouldGenerateToken; comparing resource versions of exsiting secret with the one in secret ref") - if existingStatusSecret.ResourceVersion != acceptor.SecretRef().ResourceVersion { - r.Log.Info("shouldGenerateToken; should generate is true because versions don't match", "existing-status-secret", existingStatusSecret.ResourceVersion, "secret-ref-version", acceptor.SecretRef().ResourceVersion) - return true, false, nil - } - } else { - r.Log.Info("shouldGenerateToken, should generate is true because existing status secret is nil") - return true, false, nil + // general question of whether we should regenerate it at all + // there should be three cases: + // 1. if version(existing secret from status) > the version in CR, should we just update the status in the CR? why do we regenerate the token in this case (which we do at the end) + // 2. if version(existing secret from) < the version in CR, that should be impossible? + // 3. + //if existingStatusSecret.ResourceVersion != acceptor.SecretRef().ResourceVersion { + // r.Log.Info("shouldGenerateToken; should generate is true because versions don't match", "existing-status-secret", existingStatusSecret.ResourceVersion, "secret-ref-version", acceptor.SecretRef().ResourceVersion) + // return true, false, nil + //} + return false, false, nil + } - return false, false, nil + + r.Log.Info("shouldGenerateToken, should generate is true because existing status secret is nil") + return true, false, nil } // updateStatus updates the peeringAcceptor's secret in the status. -func (r *PeeringAcceptorController) updateStatus(ctx context.Context, acceptor *consulv1alpha1.PeeringAcceptor, secretResourceVersion string) error { +func (r *PeeringAcceptorController) updateStatus(ctx context.Context, acceptorObjKey types.NamespacedName, secretResourceVersion string) error { + //r.mutex.Lock() + //defer r.mutex.Unlock() + // Get the latest resource before we update it. + var acceptor *consulv1alpha1.PeeringAcceptor + err := r.Client.Get(ctx, acceptorObjKey, acceptor) + if err != nil { + return err + } acceptor.Status.SecretRef = &consulv1alpha1.SecretRefStatus{ Secret: *acceptor.Secret(), ResourceVersion: secretResourceVersion, @@ -274,7 +291,8 @@ func (r *PeeringAcceptorController) updateStatus(ctx context.Context, acceptor * acceptor.Status.LatestPeeringVersion = pointerToUint64(peeringVersion) } } - err := r.Status().Update(ctx, acceptor) + // todo: does it need a read-write lock + err = r.Status().Update(ctx, acceptor) if err != nil { r.Log.Error(err, "failed to update PeeringAcceptor status", "name", acceptor.Name, "namespace", acceptor.Namespace) } From eca23be1075d20efd89b85d851d8cead3dc6e9ee Mon Sep 17 00:00:00 2001 From: Iryna Shustava Date: Sat, 6 Aug 2022 13:13:17 -0600 Subject: [PATCH 21/29] more descriptive error message --- control-plane/connect-inject/peering_acceptor_controller.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/control-plane/connect-inject/peering_acceptor_controller.go b/control-plane/connect-inject/peering_acceptor_controller.go index 3a9bfd8b03..911f84d88a 100644 --- a/control-plane/connect-inject/peering_acceptor_controller.go +++ b/control-plane/connect-inject/peering_acceptor_controller.go @@ -3,6 +3,7 @@ package connectinject import ( "context" "errors" + "fmt" "strconv" "sync" "time" @@ -270,10 +271,10 @@ func (r *PeeringAcceptorController) updateStatus(ctx context.Context, acceptorOb //r.mutex.Lock() //defer r.mutex.Unlock() // Get the latest resource before we update it. - var acceptor *consulv1alpha1.PeeringAcceptor + acceptor := &consulv1alpha1.PeeringAcceptor{} err := r.Client.Get(ctx, acceptorObjKey, acceptor) if err != nil { - return err + return fmt.Errorf("error fetching acceptor resource before status update: %w", err) } acceptor.Status.SecretRef = &consulv1alpha1.SecretRefStatus{ Secret: *acceptor.Secret(), From fe2742e91054f18f1c0873cbd9395c43daa7935c Mon Sep 17 00:00:00 2001 From: Iryna Shustava Date: Sat, 6 Aug 2022 15:13:28 -0600 Subject: [PATCH 22/29] add more logs --- control-plane/connect-inject/peering_acceptor_controller.go | 1 + 1 file changed, 1 insertion(+) diff --git a/control-plane/connect-inject/peering_acceptor_controller.go b/control-plane/connect-inject/peering_acceptor_controller.go index 911f84d88a..cfa3eca1f1 100644 --- a/control-plane/connect-inject/peering_acceptor_controller.go +++ b/control-plane/connect-inject/peering_acceptor_controller.go @@ -376,6 +376,7 @@ func (r *PeeringAcceptorController) SetupWithManager(mgr ctrl.Manager) error { // generateToken is a helper function that calls the Consul api to generate a token for the peer. func (r *PeeringAcceptorController) generateToken(ctx context.Context, peerName string) (*api.PeeringGenerateTokenResponse, error) { + r.Log.Info("calling /peering/token to generate token") req := api.PeeringGenerateTokenRequest{ PeerName: peerName, } From e760abc92a0a6cc4ec758e1d96b7d978ea4c00d8 Mon Sep 17 00:00:00 2001 From: Iryna Shustava Date: Sat, 6 Aug 2022 15:55:33 -0600 Subject: [PATCH 23/29] add more logs --- control-plane/connect-inject/peering_acceptor_controller.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/control-plane/connect-inject/peering_acceptor_controller.go b/control-plane/connect-inject/peering_acceptor_controller.go index cfa3eca1f1..a1407fd303 100644 --- a/control-plane/connect-inject/peering_acceptor_controller.go +++ b/control-plane/connect-inject/peering_acceptor_controller.go @@ -102,6 +102,7 @@ func (r *PeeringAcceptorController) Reconcile(ctx context.Context, req ctrl.Requ } } + // todo: we should check that the secret in the spec exists and just update status rather than regenerating a new token altogether statusSecretSet := acceptor.SecretRef() != nil // existingStatusSecret will be nil if it doesn't exist, and have the contents of the secret if it does exist. @@ -179,6 +180,7 @@ func (r *PeeringAcceptorController) Reconcile(ctx context.Context, req ctrl.Requ } r.Log.Info("finished determining if we should generate token", "shouldGenerate", shouldGenerate, "nameChanged", nameChanged) } else { + r.Log.Info("status is not set; generating a new token") shouldGenerate = true } From 1accb3ab3169bf82b8af53e904f41dac178021ff Mon Sep 17 00:00:00 2001 From: Iryna Shustava Date: Sat, 6 Aug 2022 16:08:01 -0600 Subject: [PATCH 24/29] run all peering tests --- .circleci/config.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 1de6788b99..b2699d1d0c 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -100,7 +100,7 @@ commands: << parameters.additional-flags >> \ ${ENABLE_ENTERPRISE:+-enable-enterprise} \ -enable-multi-cluster \ - -run TestPeering_ConnectNamespaces/default_destination_namespace \ + -run TestPeering_* \ -debug-directory="$TEST_RESULTS/debug" \ -consul-image=ishustava/consul-dev@sha256:c8d4c6e0ba991325166c26083d5d5280eaea7394d9a13dbfe9cf87165252a949 \ -consul-k8s-image=<< parameters.consul-k8s-image >> From 1811557d1e3f5338b274b4281eff67e794527867 Mon Sep 17 00:00:00 2001 From: Iryna Shustava Date: Sat, 6 Aug 2022 16:19:19 -0600 Subject: [PATCH 25/29] add more jobs --- .circleci/config.yml | 42 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/.circleci/config.yml b/.circleci/config.yml index b2699d1d0c..8ebdc6d795 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -1000,10 +1000,52 @@ workflows: - build-distros-linux # Run acceptance tests using the docker image built for the control plane - acceptance: + name: acceptance1 context: consul-ci requires: - dev-upload-docker - acceptance-tproxy: + name: acceptance2 + context: consul-ci + requires: + - dev-upload-docker + - acceptance: + name: acceptance3 + context: consul-ci + requires: + - dev-upload-docker + - acceptance-tproxy: + name: acceptance4 + context: consul-ci + requires: + - dev-upload-docker + - acceptance: + name: acceptance5 + context: consul-ci + requires: + - dev-upload-docker + - acceptance-tproxy: + name: acceptance6 + context: consul-ci + requires: + - dev-upload-docker + - acceptance: + name: acceptance7 + context: consul-ci + requires: + - dev-upload-docker + - acceptance-tproxy: + name: acceptance8 + context: consul-ci + requires: + - dev-upload-docker + - acceptance: + name: acceptance9 + context: consul-ci + requires: + - dev-upload-docker + - acceptance-tproxy: + name: acceptance10 context: consul-ci requires: - dev-upload-docker From fccd495fa6181ae567c162edddbc2e27ca91d506 Mon Sep 17 00:00:00 2001 From: Iryna Shustava Date: Sat, 6 Aug 2022 16:39:15 -0600 Subject: [PATCH 26/29] don't generate token if secret in the spec already exists --- .../peering_acceptor_controller.go | 33 ++++++++++++++----- 1 file changed, 25 insertions(+), 8 deletions(-) diff --git a/control-plane/connect-inject/peering_acceptor_controller.go b/control-plane/connect-inject/peering_acceptor_controller.go index a1407fd303..a56c6fd878 100644 --- a/control-plane/connect-inject/peering_acceptor_controller.go +++ b/control-plane/connect-inject/peering_acceptor_controller.go @@ -105,16 +105,33 @@ func (r *PeeringAcceptorController) Reconcile(ctx context.Context, req ctrl.Requ // todo: we should check that the secret in the spec exists and just update status rather than regenerating a new token altogether statusSecretSet := acceptor.SecretRef() != nil - // existingStatusSecret will be nil if it doesn't exist, and have the contents of the secret if it does exist. - var existingStatusSecret *corev1.Secret + // existingSecret will be nil if it doesn't exist, and have the contents of the secret if it does exist. + var existingSecret *corev1.Secret if statusSecretSet { r.Log.Info("secret status is set; retrieving secret") - existingStatusSecret, err = r.getExistingSecret(ctx, acceptor.SecretRef().Name, acceptor.Namespace) + existingSecret, err = r.getExistingSecret(ctx, acceptor.SecretRef().Name, acceptor.Namespace) if err != nil { r.Log.Error(err, "error retrieving existing secret", "name", acceptor.SecretRef().Name) r.updateStatusError(ctx, acceptor, KubernetesError, err) // todo: why do set update status error here? return ctrl.Result{}, err } + } else { + // If status is not set, check if the secret from the spec already exists and update the status. + existingSecret, err = r.getExistingSecret(ctx, acceptor.Secret().Name, acceptor.Namespace) + if err != nil { + r.Log.Error(err, "error retrieving existing secret", "name", acceptor.Secret().Name) + r.updateStatusError(ctx, acceptor, KubernetesError, err) + return ctrl.Result{}, err + } + if existingSecret != nil { + r.Log.Info("secret in the spec exists; updating status from existing secret") + err = r.updateStatus(ctx, req.NamespacedName, existingSecret.ResourceVersion) + if err != nil { + r.Log.Error(err, "error updating status", "name", acceptor.Secret().Name) + r.updateStatusError(ctx, acceptor, KubernetesError, err) + return ctrl.Result{}, err + } + } } var secretResourceVersion string @@ -132,9 +149,9 @@ func (r *PeeringAcceptorController) Reconcile(ctx context.Context, req ctrl.Requ r.Log.Info("peering doesn't exist in Consul; creating new peering", "name", acceptor.Name) if statusSecretSet { - if existingStatusSecret != nil { + if existingSecret != nil { r.Log.Info("stale secret in status; deleting stale secret", "name", acceptor.Name) - err := r.Client.Delete(ctx, existingStatusSecret) + err := r.Client.Delete(ctx, existingSecret) if err != nil { r.updateStatusError(ctx, acceptor, KubernetesError, err) return ctrl.Result{}, err @@ -172,7 +189,7 @@ func (r *PeeringAcceptorController) Reconcile(ctx context.Context, req ctrl.Requ var nameChanged bool if statusSecretSet { r.Log.Info("status secret set; determining if we need to generate token again") - shouldGenerate, nameChanged, err = r.shouldGenerateToken(acceptor, existingStatusSecret) + shouldGenerate, nameChanged, err = r.shouldGenerateToken(acceptor, existingSecret) if err != nil { r.Log.Error(err, "error determining if we should generate token again") r.updateStatusError(ctx, acceptor, InternalError, err) @@ -198,8 +215,8 @@ func (r *PeeringAcceptorController) Reconcile(ctx context.Context, req ctrl.Requ } // Delete the existing secret if the name changed. This needs to come before updating the status if we do generate a new token. if nameChanged { - if existingStatusSecret != nil { - err := r.Client.Delete(ctx, existingStatusSecret) + if existingSecret != nil { + err := r.Client.Delete(ctx, existingSecret) if err != nil { r.updateStatusError(ctx, acceptor, ConsulAgentError, err) return ctrl.Result{}, err From 568108aef845f0a0f6a8a1e2f0c55acfa3e2719d Mon Sep 17 00:00:00 2001 From: Iryna Shustava Date: Sat, 6 Aug 2022 16:52:25 -0600 Subject: [PATCH 27/29] allow secret to exist from the spec w/o status being updated --- .../peering_acceptor_controller.go | 23 +++++-------------- 1 file changed, 6 insertions(+), 17 deletions(-) diff --git a/control-plane/connect-inject/peering_acceptor_controller.go b/control-plane/connect-inject/peering_acceptor_controller.go index a56c6fd878..d8695c8933 100644 --- a/control-plane/connect-inject/peering_acceptor_controller.go +++ b/control-plane/connect-inject/peering_acceptor_controller.go @@ -123,15 +123,6 @@ func (r *PeeringAcceptorController) Reconcile(ctx context.Context, req ctrl.Requ r.updateStatusError(ctx, acceptor, KubernetesError, err) return ctrl.Result{}, err } - if existingSecret != nil { - r.Log.Info("secret in the spec exists; updating status from existing secret") - err = r.updateStatus(ctx, req.NamespacedName, existingSecret.ResourceVersion) - if err != nil { - r.Log.Error(err, "error updating status", "name", acceptor.Secret().Name) - r.updateStatusError(ctx, acceptor, KubernetesError, err) - return ctrl.Result{}, err - } - } } var secretResourceVersion string @@ -148,14 +139,12 @@ func (r *PeeringAcceptorController) Reconcile(ctx context.Context, req ctrl.Requ if peering == nil { r.Log.Info("peering doesn't exist in Consul; creating new peering", "name", acceptor.Name) - if statusSecretSet { - if existingSecret != nil { - r.Log.Info("stale secret in status; deleting stale secret", "name", acceptor.Name) - err := r.Client.Delete(ctx, existingSecret) - if err != nil { - r.updateStatusError(ctx, acceptor, KubernetesError, err) - return ctrl.Result{}, err - } + if existingSecret != nil { + r.Log.Info("secret exists without a peering in Consul; deleting stale secret", "name", acceptor.Name) + err := r.Client.Delete(ctx, existingSecret) + if err != nil { + r.updateStatusError(ctx, acceptor, KubernetesError, err) + return ctrl.Result{}, err } } // Generate and store the peering token. From 7940d18309113a2691c0c372e6a413af4e7e966d Mon Sep 17 00:00:00 2001 From: Iryna Shustava Date: Sat, 6 Aug 2022 18:06:21 -0600 Subject: [PATCH 28/29] don't generate token twice --- .../connect-inject/peering_acceptor_controller.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/control-plane/connect-inject/peering_acceptor_controller.go b/control-plane/connect-inject/peering_acceptor_controller.go index d8695c8933..76d2f3ad05 100644 --- a/control-plane/connect-inject/peering_acceptor_controller.go +++ b/control-plane/connect-inject/peering_acceptor_controller.go @@ -35,7 +35,7 @@ type PeeringAcceptorController struct { Scheme *runtime.Scheme context.Context - mutex sync.RWMutex + mutex sync.Mutex } const ( @@ -176,8 +176,8 @@ func (r *PeeringAcceptorController) Reconcile(ctx context.Context, req ctrl.Requ // be set to true. var shouldGenerate bool var nameChanged bool - if statusSecretSet { - r.Log.Info("status secret set; determining if we need to generate token again") + if existingSecret != nil { + r.Log.Info("found existing secret; determining if we need to generate token again") shouldGenerate, nameChanged, err = r.shouldGenerateToken(acceptor, existingSecret) if err != nil { r.Log.Error(err, "error determining if we should generate token again") @@ -186,7 +186,7 @@ func (r *PeeringAcceptorController) Reconcile(ctx context.Context, req ctrl.Requ } r.Log.Info("finished determining if we should generate token", "shouldGenerate", shouldGenerate, "nameChanged", nameChanged) } else { - r.Log.Info("status is not set; generating a new token") + r.Log.Info("existing secret is nil; generating a new token") shouldGenerate = true } From 23917216b6723d1baf81ff5dd9ff4b9b6e8a1588 Mon Sep 17 00:00:00 2001 From: Iryna Shustava Date: Mon, 8 Aug 2022 16:18:22 -0600 Subject: [PATCH 29/29] use latest consul 1.13 dev image --- .circleci/config.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 8ebdc6d795..9b1d4e951a 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -102,7 +102,7 @@ commands: -enable-multi-cluster \ -run TestPeering_* \ -debug-directory="$TEST_RESULTS/debug" \ - -consul-image=ishustava/consul-dev@sha256:c8d4c6e0ba991325166c26083d5d5280eaea7394d9a13dbfe9cf87165252a949 \ + -consul-image=docker.mirror.hashicorp.services/hashicorppreview/consul-enterprise:1.13-dev \ -consul-k8s-image=<< parameters.consul-k8s-image >> then echo "Tests in ${pkg} failed, aborting early"