remove healthcheck and cleanup controller flags (#899)

* remove healthcheck and cleanup controller flags

.circleci/config.yml

@@ -143,7 +143,7 @@ jobs:
                     -kubecontext="kind-dc1" \
                     -secondary-kubecontext="kind-dc2" \
                     -debug-directory="$TEST_RESULTS/debug" \
-                    -consul-k8s-image="gcr.io/nitya-293720/consul-k8s-dev:tproxymetrics2" # TODO: change once feature-tproxy consul-k8s branch is merged
+                    -consul-k8s-image=kschoche/consul-k8s-dev2:2021-4-9 # TODO: change once feature-tproxy consul-k8s branch is merged
               then
                 echo "Tests in ${pkg} failed, aborting early"
                 exit_code=1

CHANGELOG.md

@@ -9,6 +9,8 @@ IMPROVEMENTS:
 
 BREAKING CHANGES:
 * Minimum Kubernetes versions supported is 1.16+. [[GH-883](https://github.com/hashicorp/consul-helm/pull/883)]
+* Connect: `-enable-health-checks-controller`, `-health-checks-reconcile-period`, `-cleanup-controller-reconcile-period` have been removed
+  and are no longer supported as the controllers have been replaced by the endpoints controller. [[GH-892](https://github.com/hashicorp/consul-helm/pull/899)]
 
 BUG FIXES:
 * Add startup probe to connect-inject deployment to give time for certificates to be available.

templates/client-config-configmap.yaml

@@ -19,7 +19,7 @@ data:
       "enable_central_service_config": true
     }
 
-  {{- if (and .Values.connectInject.enabled .Values.connectInject.healthChecks.enabled) }}
+  {{- if .Values.connectInject.enabled }}
   {{/* We set check_update_interval to 0s so that check output is immediately viewable
        in the UI. */}}
   config.json: |-

templates/connect-inject-deployment.yaml

@@ -93,11 +93,6 @@ spec:
                 {{- if .Values.connectInject.logLevel }}
                 -log-level={{ .Values.connectInject.logLevel }} \
                 {{- end }}
-                {{- if .Values.connectInject.healthChecks.enabled }}
-                -enable-health-checks-controller=true \
-                -health-checks-reconcile-period={{ .Values.connectInject.healthChecks.reconcilePeriod }} \
-                {{- end }}
-                -cleanup-controller-reconcile-period={{ .Values.connectInject.cleanupController.reconcilePeriod }} \
                 {{- if (or (and (ne (.Values.connectInject.metrics.defaultEnabled | toString) "-") .Values.connectInject.metrics.defaultEnabled) (and (eq (.Values.connectInject.metrics.defaultEnabled | toString) "-") .Values.global.metrics.enabled)) }}
                 -default-enable-metrics=true \
                 {{- else }}

templates/server-acl-init-job.yaml

@@ -139,9 +139,6 @@ spec:
                 {{- if and .Values.externalServers.enabled .Values.externalServers.k8sAuthMethodHost }}
                 -inject-auth-method-host={{ .Values.externalServers.k8sAuthMethodHost }} \
                 {{- end }}
-                {{- if .Values.connectInject.healthChecks.enabled }}
-                -enable-health-checks \
-                {{- end }}
                 {{- end }}
 
                 {{- if .Values.meshGateway.enabled }}

test/acceptance/tests/connect/connect_inject_test.go

@@ -93,8 +93,8 @@ func TestConnectInject(t *testing.T) {
 	}
 }
 
-// Test the cleanup controller that cleans up force-killed pods.
-func TestConnectInject_CleanupController(t *testing.T) {
+// Test the endpoints controller cleans up force-killed pods.
+func TestConnectInject_CleanupKilledPods(t *testing.T) {
 	cases := []struct {
 		secure      bool
 		autoEncrypt bool

test/unit/client-config-configmap.bats

@@ -61,24 +61,11 @@ load _helpers
   [ "${actual}" = "true" ]
 }
 
-#--------------------------------------------------------------------
-# connectInject.healthChecks
-
-@test "client/ConfigMap: check_update_interval is not set by default" {
-  cd `chart_dir`
-  local actual=$(helm template \
-      -s templates/client-config-configmap.yaml  \
-      . | tee /dev/stderr |
-      yq '.data["config.json"] | length > 0' | tee /dev/stderr)
-  [ "${actual}" = "false" ]
-}
-
-@test "client/ConfigMap: check_update_interval is set when health checks enabled" {
+@test "client/ConfigMap: check_update_interval is set when connect is enabled" {
   cd `chart_dir`
   local actual=$(helm template \
       -s templates/client-config-configmap.yaml  \
       --set 'connectInject.enabled=true' \
-      --set 'connectInject.healthChecks.enabled=true' \
       . | tee /dev/stderr |
       yq '.data["config.json"] | contains("check_update_interval")' | tee /dev/stderr)
   [ "${actual}" = "true" ]

test/unit/connect-inject-deployment.bats

@@ -152,97 +152,6 @@ EOF
   rm -f "$temp_file"
 }
 
-#--------------------------------------------------------------------
-# healthChecks
-
-@test "connectInject/Deployment: health checks enabled by default" {
-  cd `chart_dir`
-  local cmd=$(helm template \
-      -s templates/connect-inject-deployment.yaml \
-      --set 'connectInject.enabled=true' \
-      . | tee /dev/stderr |
-      yq '.spec.template.spec.containers[0].command' | tee /dev/stderr)
-
-  local actual=$(echo "$cmd" |
-    yq 'any(contains("-enable-health-checks-controller"))' | tee /dev/stderr)
-  [ "${actual}" = "true" ]
-  local actual=$(echo "$cmd" |
-    yq 'any(contains("-health-checks-reconcile-period"))' | tee /dev/stderr)
-  [ "${actual}" = "true" ]
-}
-
-@test "connectInject/Deployment: health checks can be disabled" {
-  cd `chart_dir`
-  local cmd=$(helm template \
-      -s templates/connect-inject-deployment.yaml \
-      --set 'connectInject.enabled=true' \
-      --set 'connectInject.healthChecks.enabled=false' \
-      . | tee /dev/stderr |
-      yq '.spec.template.spec.containers[0].command' | tee /dev/stderr)
-
-  local actual=$(echo "$cmd" |
-    yq 'any(contains("-enable-health-checks-controller"))' | tee /dev/stderr)
-  [ "${actual}" = "false" ]
-}
-
-@test "connectInject/Deployment: health checks reconcile period set by default when health checks are enabled" {
-  cd `chart_dir`
-  local cmd=$(helm template \
-      -s templates/connect-inject-deployment.yaml \
-      --set 'connectInject.enabled=true' \
-      --set 'connectInject.healthChecks.enabled=true' \
-      . | tee /dev/stderr |
-      yq '.spec.template.spec.containers[0].command' | tee /dev/stderr)
-
-  local actual=$(echo "$cmd" |
-    yq 'any(contains("-health-checks-reconcile-period=1m"))' | tee /dev/stderr)
-  [ "${actual}" = "true" ]
-}
-
-@test "connectInject/Deployment: health checks reconcile period can be set" {
-  cd `chart_dir`
-  local cmd=$(helm template \
-      -s templates/connect-inject-deployment.yaml \
-      --set 'connectInject.enabled=true' \
-      --set 'connectInject.healthChecks.enabled=true' \
-      --set 'connectInject.healthChecks.reconcilePeriod=10h' \
-      . | tee /dev/stderr |
-      yq '.spec.template.spec.containers[0].command' | tee /dev/stderr)
-
-  local actual=$(echo "$cmd" |
-    yq 'any(contains("-health-checks-reconcile-period=10h"))' | tee /dev/stderr)
-  [ "${actual}" = "true" ]
-}
-
-#--------------------------------------------------------------------
-# cleanup controller
-@test "connectInject/Deployment: cleanup controller reconcile period set by default" {
-  cd `chart_dir`
-  local cmd=$(helm template \
-      -s templates/connect-inject-deployment.yaml \
-      --set 'connectInject.enabled=true' \
-      . | tee /dev/stderr |
-      yq '.spec.template.spec.containers[0].command' | tee /dev/stderr)
-
-  local actual=$(echo "$cmd" |
-    yq 'any(contains("-cleanup-controller-reconcile-period=5m"))' | tee /dev/stderr)
-  [ "${actual}" = "true" ]
-}
-
-@test "connectInject/Deployment: cleanup controller reconcile period can be set" {
-  cd `chart_dir`
-  local cmd=$(helm template \
-      -s templates/connect-inject-deployment.yaml \
-      --set 'connectInject.enabled=true' \
-      --set 'connectInject.cleanupController.reconcilePeriod=10h' \
-      . | tee /dev/stderr |
-      yq '.spec.template.spec.containers[0].command' | tee /dev/stderr)
-
-  local actual=$(echo "$cmd" |
-    yq 'any(contains("-cleanup-controller-reconcile-period=10h"))' | tee /dev/stderr)
-  [ "${actual}" = "true" ]
-}
-
 #--------------------------------------------------------------------
 # metrics
 

test/unit/server-acl-init-job.bats

@@ -1408,42 +1408,6 @@ load _helpers
   [ "${actual}" = "true" ]
 }
 
-@test "serverACLInit/Job: health checks flag enabled with ACLs" {
-  cd `chart_dir`
-  local actual=$(helm template \
-      -s templates/server-acl-init-job.yaml  \
-      --set 'global.acls.manageSystemACLs=true' \
-      --set 'connectInject.healthChecks.enabled=true' \
-      --set 'connectInject.enabled=true' \
-      . | tee /dev/stderr |
-      yq '.spec.template.spec.containers[0].command | any(contains("-enable-health-checks"))' | tee /dev/stderr)
-  [ "${actual}" = "true" ]
-}
-
-@test "serverACLInit/Job: health checks flag not passed with connectInject disabled" {
-  cd `chart_dir`
-  local actual=$(helm template \
-      -s templates/server-acl-init-job.yaml  \
-      --set 'global.acls.manageSystemACLs=true' \
-      --set 'connectInject.enabled=false' \
-      --set 'connectInject.healthChecks.enabled=true' \
-      . | tee /dev/stderr |
-      yq '.spec.template.spec.containers[0].command | any(contains("-enable-health-checks"))' | tee /dev/stderr)
-  [ "${actual}" = "false" ]
-}
-
-@test "serverACLInit/Job: health checks flag not passed with health checks disabled" {
-  cd `chart_dir`
-  local actual=$(helm template \
-      -s templates/server-acl-init-job.yaml  \
-      --set 'global.acls.manageSystemACLs=true' \
-      --set 'connectInject.enabled=true' \
-      --set 'connectInject.healthChecks.enabled=false' \
-      . | tee /dev/stderr |
-      yq '.spec.template.spec.containers[0].command | any(contains("-enable-health-checks"))' | tee /dev/stderr)
-  [ "${actual}" = "false" ]
-}
-
 #--------------------------------------------------------------------
 # controller
 

values.yaml

@@ -1293,18 +1293,6 @@ connectInject:
   # to explicitly opt-out of injection.
   default: false
 
-  # Enables synchronization of Kubernetes health probe status with Consul.
-  # NOTE: It is highly recommended to enable TLS with this feature because it requires
-  # making calls to Consul clients across the cluster. Without TLS enabled, these calls
-  # could leak ACL tokens should the cluster network become compromised.
-  healthChecks:
-    # Enables the Consul Health Check controller which syncs the readiness status of
-    # connect-injected pods with Consul.
-    enabled: true
-    # If `healthChecks.enabled` is set to `true`, `reconcilePeriod` defines how often a full state
-    # reconcile is done after the initial reconcile at startup is completed.
-    reconcilePeriod: "1m"
-
   # Configures metrics for Consul Connect services. All values are overridable
   # via annotations on a per-pod basis.
   metrics:
@@ -1343,17 +1331,6 @@ connectInject:
     # `consul.hashicorp.com/service-metrics-path` annotation.
     defaultPrometheusScrapePath: "/metrics"
 
-  # Cleanup controller cleans up Consul service instances that remain registered
-  # despite their pods no longer running. This could happen if the pod's `preStop`
-  # hook failed to execute for some reason.
-  cleanupController:
-    # How often to do a full reconcile where the controller looks at all pods
-    # and service instances and ensure the state is correct.
-    # The controller reacts to each delete event immediately but if it misses
-    # an event due to being down or a network issue, the reconcile loop will
-    # handle cleaning up any missed deleted pods.
-    reconcilePeriod: "5m"
-
   # Used to pass arguments to the injected envoy sidecar.
   # Valid arguments to pass to envoy can be found here: https://www.envoyproxy.io/docs/envoy/latest/operations/cli
   # e.g "--log-level debug --disable-hot-restart"