Add externalServers.skipServerWatch to helm for working with load bal…

…ancers (#1686) - Add helm value of externalServers.skipServerWatch - Add the environment variable CONSUL_SKIP_SERVER_WATCH to helm _helpers.tpl so that the environment variable will get automatically picked up by connection manager when talking to consul servers - Even though not directly needed there, add the check that externalServers.enabled is also set when externalServers.skipServerWatch=true in connect-inject-deployment.yaml - Bats tests for connect-inject-deployment to make sure that it fails the condition above and that the CONSUL_SKIP_SERVER_WATCH is set in the environment. - Read the env variable in flags/consul.go so that it can be passed to connection manager and down into discover.Config (used by partition-init, catalog-sync, server-acl-init, inject-connect, etc) - Specifically, set the flag in the webhook so that it is passed onto consul-dataplane as a -server-watch-disabled=true flag
hashicorp · Nov 9, 2022 · bf28f85 · bf28f85
1 parent 78b24f0
commit bf28f85
Show file tree

Hide file tree

Showing 16 changed files with 150 additions and 0 deletions.
diff --git a/charts/consul/templates/_helpers.tpl b/charts/consul/templates/_helpers.tpl
@@ -355,6 +355,10 @@ Consul server environment variables for consul-k8s commands.
   value: {{ .Values.externalServers.tlsServerName }}
 {{- end }}
 {{- end }}
+{{- if and .Values.externalServers.enabled .Values.externalServers.skipServerWatch }}
+- name: CONSUL_SKIP_SERVER_WATCH
+  value: "true" 
+{{- end }}
 {{- end -}}
 
 {{/*

diff --git a/charts/consul/templates/connect-inject-deployment.yaml b/charts/consul/templates/connect-inject-deployment.yaml
@@ -8,6 +8,7 @@
 {{- $serverEnabled := (or (and (ne (.Values.server.enabled | toString) "-") .Values.server.enabled) (and (eq (.Values.server.enabled | toString) "-") .Values.global.enabled)) -}}
 {{- $serverExposeServiceEnabled := (or (and (ne (.Values.server.exposeService.enabled | toString) "-") .Values.server.exposeService.enabled) (and (eq (.Values.server.exposeService.enabled | toString) "-") .Values.global.adminPartitions.enabled)) -}}
 {{- if and .Values.externalServers.enabled (not .Values.externalServers.hosts) }}{{ fail "externalServers.hosts must be set if externalServers.enabled is true" }}{{ end -}}
+{{- if and .Values.externalServers.skipServerWatch (not .Values.externalServers.enabled) }}{{ fail "externalServers.enabled must be set if externalServers.skipServerWatch is true" }}{{ end -}}
 {{ template "consul.validateRequiredCloudSecretsExist" . }}
 {{ template "consul.validateCloudSecretKeys" . }}
 # The deployment for running the Connect sidecar injector

diff --git a/charts/consul/templates/ingress-gateways-deployment.yaml b/charts/consul/templates/ingress-gateways-deployment.yaml
@@ -294,6 +294,9 @@ spec:
             {{- if (and $root.Values.global.metrics.enabled $root.Values.global.metrics.enableGatewayMetrics) }}
             -telemetry-prom-scrape-path="/metrics"
             {{- end }}
+            {{- if and $root.Values.externalServers.enabled $root.Values.externalServers.skipServerWatch }}
+            -server-watch-disabled=true
+            {{- end }}
         livenessProbe:
           tcpSocket:
             port: 21000

diff --git a/charts/consul/templates/mesh-gateway-deployment.yaml b/charts/consul/templates/mesh-gateway-deployment.yaml
@@ -244,6 +244,9 @@ spec:
             {{- if (and .Values.global.metrics.enabled .Values.global.metrics.enableGatewayMetrics) }}
             -telemetry-prom-scrape-path="/metrics"
             {{- end }}
+            {{- if and .Values.externalServers.enabled .Values.externalServers.skipServerWatch }}
+            -server-watch-disabled=true
+            {{- end }}
         livenessProbe:
           tcpSocket:
             port: {{ .Values.meshGateway.containerPort }}

diff --git a/charts/consul/templates/terminating-gateways-deployment.yaml b/charts/consul/templates/terminating-gateways-deployment.yaml
@@ -285,6 +285,9 @@ spec:
                 {{- if (and $root.Values.global.metrics.enabled $root.Values.global.metrics.enableGatewayMetrics) }}
                 -telemetry-prom-scrape-path="/metrics"
                 {{- end }}
+                {{- if and $root.Values.externalServers.enabled $root.Values.externalServers.skipServerWatch }}
+                -server-watch-disabled=true
+                {{- end }}
           livenessProbe:
             tcpSocket:
               port: 8443

diff --git a/charts/consul/test/unit/connect-inject-deployment.bats b/charts/consul/test/unit/connect-inject-deployment.bats
@@ -2033,6 +2033,35 @@ reservedNameTest() {
   [ "${actual}" = "" ]
 }
 
+@test "connectInject/Deployment: fails if externalServers.skipServerWatch is not provided when externalServers.enabled is true" {
+  cd `chart_dir`
+  run helm template \
+      -s templates/connect-inject-deployment.yaml \
+      --set 'connectInject.enabled=true' \
+      --set 'server.enabled=false' \
+      --set 'externalServers.skipServerWatch=true' \
+       .
+  [ "$status" -eq 1 ]
+  [[ "$output" =~ "externalServers.enabled must be set if externalServers.skipServerWatch is true" ]]
+}
+
+@test "connectInject/Deployment: configures the sidecar-injector env to skip server watch" {
+  cd `chart_dir`
+  local env=$(helm template \
+      -s templates/connect-inject-deployment.yaml  \
+      --set 'connectInject.enabled=true' \
+      --set 'server.enabled=false' \
+      --set 'externalServers.enabled=true' \
+      --set 'externalServers.hosts[0]=consul' \
+      --set 'externalServers.skipServerWatch=true' \
+      . | tee /dev/stderr |
+       yq '.spec.template.spec.containers[0].env[]' | tee /dev/stderr)\
+
+  local actual=$(echo "$env" |
+    jq -r '. | select( .name == "CONSUL_SKIP_SERVER_WATCH").value' | tee /dev/stderr)
+  [ "${actual}" = "true" ]
+}
+
 #--------------------------------------------------------------------
 # global.cloud
 

diff --git a/charts/consul/test/unit/ingress-gateways-deployment.bats b/charts/consul/test/unit/ingress-gateways-deployment.bats
@@ -312,6 +312,27 @@ load _helpers
   [ "${actual}" = "null" ]
 }
 
+#--------------------------------------------------------------------
+# externalServers.skipServerWatch
+
+@test "ingressGateways/Deployment: sets server-watch-disabled flag when externalServers.enabled and externalServers.skipServerWatch is true" {
+  cd `chart_dir`
+  local object=$(helm template \
+      -s templates/ingress-gateways-deployment.yaml  \
+      --set 'ingressGateways.enabled=true' \
+      --set 'connectInject.enabled=true' \
+      --set 'global.tls.enabled=false' \
+      --set 'server.enabled=false' \
+      --set 'externalServers.enabled=true' \
+      --set 'externalServers.hosts[0]=consul' \
+      --set 'externalServers.skipServerWatch=true' \
+      . | tee /dev/stderr |
+      yq -s -r '.[0].spec.template.spec.containers[0].command[2]' | tee /dev/stderr)
+
+  local actual=$(echo $object | yq -r '. | contains("-server-watch-disabled")' | tee /dev/stderr)
+  [ "${actual}" = "true" ]
+}
+
 #--------------------------------------------------------------------
 # replicas
 

diff --git a/charts/consul/test/unit/mesh-gateway-deployment.bats b/charts/consul/test/unit/mesh-gateway-deployment.bats
@@ -140,6 +140,27 @@ key2: value2' \
   [ "${actual}" = "null" ]
 }
 
+#--------------------------------------------------------------------
+# externalServers.skipServerWatch
+
+@test "meshGateway/Deployment: sets server-watch-disabled flag when externalServers.enabled and externalServers.skipServerWatch is true" {
+  cd `chart_dir`
+  local object=$(helm template \
+      -s templates/ingress-gateways-deployment.yaml  \
+      --set 'ingressGateways.enabled=true' \
+      --set 'connectInject.enabled=true' \
+      --set 'global.tls.enabled=false' \
+      --set 'server.enabled=false' \
+      --set 'externalServers.enabled=true' \
+      --set 'externalServers.hosts[0]=consul' \
+      --set 'externalServers.skipServerWatch=true' \
+      . | tee /dev/stderr |
+      yq -s -r '.[0].spec.template.spec.containers[0].command[2]' | tee /dev/stderr)
+
+  local actual=$(echo $object | yq -r '. | contains("-server-watch-disabled")' | tee /dev/stderr)
+  [ "${actual}" = "true" ]
+}
+
 #--------------------------------------------------------------------
 # replicas
 

diff --git a/charts/consul/test/unit/terminating-gateways-deployment.bats b/charts/consul/test/unit/terminating-gateways-deployment.bats
@@ -369,6 +369,27 @@ load _helpers
   [ "${actual}" = "null" ]
 }
 
+#--------------------------------------------------------------------
+# externalServers.skipServerWatch
+
+@test "terminatingGateways/Deployment: sets server-watch-disabled flag when externalServers.enabled and externalServers.skipServerWatch is true" {
+  cd `chart_dir`
+  local object=$(helm template \
+      -s templates/ingress-gateways-deployment.yaml  \
+      --set 'ingressGateways.enabled=true' \
+      --set 'connectInject.enabled=true' \
+      --set 'global.tls.enabled=false' \
+      --set 'server.enabled=false' \
+      --set 'externalServers.enabled=true' \
+      --set 'externalServers.hosts[0]=consul' \
+      --set 'externalServers.skipServerWatch=true' \
+      . | tee /dev/stderr |
+      yq -s -r '.[0].spec.template.spec.containers[0].command[2]' | tee /dev/stderr)
+
+  local actual=$(echo $object | yq -r '. | contains("-server-watch-disabled")' | tee /dev/stderr)
+  [ "${actual}" = "true" ]
+}
+
 #--------------------------------------------------------------------
 # replicas
 

diff --git a/charts/consul/values.yaml b/charts/consul/values.yaml
@@ -1183,6 +1183,10 @@ externalServers:
   # @type: string
   k8sAuthMethodHost: null
 
+  # If true, setting this prevents the consul-dataplane and consul-k8s components from watching the Consul servers for changes. This is
+  # useful for situations where Consul servers are behind a load balancer. 
+  skipServerWatch: false
+
 # Values that configure running a Consul client on Kubernetes nodes.
 client:
   # If true, the chart will install all

diff --git a/control-plane/connect-inject/consul_dataplane_sidecar.go b/control-plane/connect-inject/consul_dataplane_sidecar.go
@@ -155,6 +155,10 @@ func (w *MeshWebhook) getContainerSidecarCommand(namespace corev1.Namespace, mpi
 		"-envoy-concurrency=" + strconv.Itoa(envoyConcurrency),
 	}
 
+	if w.SkipServerWatch {
+		cmd = append(cmd, "-server-watch-disabled=true")
+	}
+
 	if w.AuthMethod != "" {
 		cmd = append(cmd,
 			"-credential-type=login",

diff --git a/control-plane/connect-inject/consul_dataplane_sidecar_test.go b/control-plane/connect-inject/consul_dataplane_sidecar_test.go
@@ -118,6 +118,12 @@ func TestHandlerConsulDataplaneSidecar(t *testing.T) {
 			},
 			additionalExpCmdArgs: " -tls-disabled",
 		},
+		"skip server watch enabled": {
+			webhookSetupFunc: func(w *MeshWebhook) {
+				w.SkipServerWatch = true
+			},
+			additionalExpCmdArgs: " -server-watch-disabled=true -tls-disabled",
+		},
 	}
 
 	for name, c := range cases {

diff --git a/control-plane/connect-inject/mesh_webhook.go b/control-plane/connect-inject/mesh_webhook.go
@@ -167,6 +167,10 @@ type MeshWebhook struct {
 	// those containers to be created otherwise.
 	EnableOpenShift bool
 
+	// SkipServerWatch prevents consul-dataplane from consuming the server update stream. This is useful
+	// for situations where Consul servers are behind a load balancer.
+	SkipServerWatch bool
+
 	// ReleaseNamespace is the Kubernetes namespace where this webhook is running.
 	ReleaseNamespace string
 

diff --git a/control-plane/subcommand/flags/consul.go b/control-plane/subcommand/flags/consul.go
@@ -38,6 +38,8 @@ const (
 	LoginNamespaceEnvVar       = "CONSUL_LOGIN_NAMESPACE"
 	LoginMetaEnvVar            = "CONSUL_LOGIN_META"
 
+	SkipServerWatchEnvVar = "CONSUL_SKIP_SERVER_WATCH"
+
 	APITimeoutEnvVar = "CONSUL_API_TIMEOUT"
 )
 
@@ -52,6 +54,8 @@ type ConsulFlags struct {
 	Partition  string
 	Datacenter string
 
+	SkipServerWatch bool
+
 	ConsulTLSFlags
 	ConsulACLFlags
 }
@@ -87,6 +91,7 @@ func (f *ConsulFlags) Flags() *flag.FlagSet {
 	grpcPort, _ := strconv.Atoi(os.Getenv(GRPCPortEnvVar))
 	httpPort, _ := strconv.Atoi(os.Getenv(HTTPPortEnvVar))
 	useTLS, _ := strconv.ParseBool(os.Getenv(UseTLSEnvVar))
+	skipServerWatch, _ := strconv.ParseBool(os.Getenv(SkipServerWatchEnvVar))
 	consulLoginMetaFromEnv := os.Getenv(LoginMetaEnvVar)
 	if consulLoginMetaFromEnv != "" {
 		// Parse meta from env var.
@@ -168,6 +173,8 @@ func (f *ConsulFlags) Flags() *flag.FlagSet {
 			"may be specified multiple times to set multiple meta fields.")
 	fs.DurationVar(&f.APITimeout, "api-timeout", defaultAPITimeout,
 		"The time in seconds that the consul API client will wait for a response from the API before cancelling the request.")
+	fs.BoolVar(&f.SkipServerWatch, "skip-server-watch", skipServerWatch, "If true, skip watching server upstream."+
+		"This can also be specified via the CONSUL_SKIP_SERVER_WATCH environment variable.")
 	return fs
 }
 
@@ -223,6 +230,10 @@ func (f *ConsulFlags) ConsulServerConnMgrConfig() (discovery.Config, error) {
 		cfg.Credentials.Static.Token = string(token)
 	}
 
+	if f.SkipServerWatch {
+		cfg.ServerWatchDisabled = true
+	}
+
 	return cfg, nil
 }
 

diff --git a/control-plane/subcommand/flags/consul_test.go b/control-plane/subcommand/flags/consul_test.go
@@ -39,6 +39,7 @@ func TestConsulFlags_Flags(t *testing.T) {
 				LoginPartitionEnvVar:       "other-test-partition",
 				LoginNamespaceEnvVar:       "other-test-ns",
 				LoginMetaEnvVar:            "key1=value1,key2=value2",
+				SkipServerWatchEnvVar:      "true",
 			},
 			expFlags: &ConsulFlags{
 				Addresses:  "consul.address",
@@ -66,6 +67,7 @@ func TestConsulFlags_Flags(t *testing.T) {
 						Meta:            map[string]string{"key1": "value1", "key2": "value2"},
 					},
 				},
+				SkipServerWatch: true,
 			},
 		},
 		"defaults": {
@@ -211,6 +213,18 @@ func TestConsulFlags_ConsulServerConnMgrConfig(t *testing.T) {
 				},
 			},
 		},
+		"skip server watch to server watch disabled": {
+			flags: ConsulFlags{
+				Addresses:       "consul.address",
+				GRPCPort:        8502,
+				SkipServerWatch: true,
+			},
+			expConfig: discovery.Config{
+				Addresses:           "consul.address",
+				GRPCPort:            8502,
+				ServerWatchDisabled: true,
+			},
+		},
 	}
 
 	for name, c := range cases {

diff --git a/control-plane/subcommand/inject-connect/command.go b/control-plane/subcommand/inject-connect/command.go
@@ -499,6 +499,7 @@ func (c *Command) Run(args []string) int {
 			ConsulCACert:                 string(caCertPem),
 			TLSEnabled:                   c.consul.UseTLS,
 			ConsulAddress:                c.consul.Addresses,
+			SkipServerWatch:              c.consul.SkipServerWatch,
 			ConsulTLSServerName:          c.consul.TLSServerName,
 			DefaultProxyCPURequest:       sidecarProxyCPURequest,
 			DefaultProxyCPULimit:         sidecarProxyCPULimit,