Skip to content

Commit

Permalink
Fix logic for enabling ui metrics (#841)
Browse files Browse the repository at this point in the history
* Fix logic for enabling ui metrics

Previously ui metrics would be enabled when
`global.metrics.enabled=false` and `ui.metrics.enabled=-`. Now that
would cause UI metrics to be disabled which makes sense since "-" means
inherit from `global.metrics.enabled`.

The logic is now to enable ui metrics when:

```
ui.enabled &&
  (
    (global.metrics.enabled == true && ui.metrics.enabled == "-") ||
    (ui.metrics.enabled == true)
  )
```
  • Loading branch information
lkysow authored Nov 10, 2021
1 parent 4f783fe commit 0ad49a9
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 8 deletions.
12 changes: 8 additions & 4 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,10 +1,17 @@
## UNRELEASED

BREAKING CHANGES:
* Previously [UI metrics](https://www.consul.io/docs/connect/observability/ui-visualization) would be enabled when
`global.metrics=false` and `ui.metrics.enabled=-`. If you are no longer seeing UI metrics,
set `global.metrics=true` or `ui.metrics.enabled=true`. [[GH-841](https://github.com/hashicorp/consul-k8s/pull/841)]

IMPROVEMENTS:
* Control Plane
* TLS: Support PKCS1 and PKCS8 private keys for Consul certificate authority. [[GH-843](https://github.com/hashicorp/consul-k8s/pull/843)]
* CLI
* Delete jobs, cluster roles, and cluster role bindings on `uninstall`. [[GH-820](https://github.com/hashicorp/consul-k8s/pull/820)]
* Helm Chart
* Add `component` labels to all resources. [[GH-840](https://github.com/hashicorp/consul-k8s/pull/840)]

BUG FIXES:
* Control Plane
Expand All @@ -18,10 +25,7 @@ BUG FIXES:
* **(Consul Enterprise only)** Error on Helm install if a reserved name is used for the admin partition name or a
Consul destination namespace for connect or catalog sync. [[GH-846](https://github.com/hashicorp/consul-k8s/pull/846)]
* Truncate Persistent Volume Claim names when namespace names are too long. [[GH-799](https://github.com/hashicorp/consul-k8s/pull/799)]

IMPROVEMENTS:
* Helm Chart
* Add `component` labels to all resources. [[GH-840](https://github.com/hashicorp/consul-k8s/pull/840)]
* Fix issue where UI metrics would be enabled when `global.metrics=false` and `ui.metrics.enabled=-`. [[GH-841](https://github.com/hashicorp/consul-k8s/pull/841)]

## 0.36.0 (November 02, 2021)

Expand Down
2 changes: 1 addition & 1 deletion charts/consul/templates/server-config-configmap.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ data:
}
}
{{- end }}
{{- if (and .Values.ui.enabled (or .Values.ui.metrics.enabled (and .Values.global.metrics.enabled (eq (.Values.ui.metrics.enabled | toString) "-")))) }}
{{- if (and .Values.ui.enabled (or (eq "true" (.Values.ui.metrics.enabled | toString) ) (and .Values.global.metrics.enabled (eq "-" (.Values.ui.metrics.enabled | toString))))) }}
ui-config.json: |-
{
"ui_config": {
Expand Down
22 changes: 22 additions & 0 deletions charts/consul/test/unit/server-config-configmap.bats
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,28 @@ load _helpers
[ "${actual}" = "true" ]
}

@test "server/ConfigMap: does not create ui config when .ui.enabled=false and .ui.metrics.enabled=true" {
cd `chart_dir`
local actual=$(helm template \
-s templates/server-config-configmap.yaml \
--set 'ui.enabled=false' \
--set 'ui.metrics.enabled=false' \
. | tee /dev/stderr |
yq -r '.data["ui-config.json"] | length > 0' | tee /dev/stderr)
[ "${actual}" = "false" ]
}

@test "server/ConfigMap: does not create ui config when .ui.enabled=true and .global.metrics.enabled=false" {
cd `chart_dir`
local actual=$(helm template \
-s templates/server-config-configmap.yaml \
--set 'ui.enabled=true' \
--set 'global.metrics.enabled=false' \
. | tee /dev/stderr |
yq -r '.data["ui-config.json"] | length > 0' | tee /dev/stderr)
[ "${actual}" = "false" ]
}

@test "server/ConfigMap: does not create ui config when .ui.enabled=true and .ui.metrics.enabled=false" {
cd `chart_dir`
local actual=$(helm template \
Expand Down
6 changes: 3 additions & 3 deletions charts/consul/test/unit/server-statefulset.bats
Original file line number Diff line number Diff line change
Expand Up @@ -686,7 +686,7 @@ load _helpers
-s templates/server-statefulset.yaml \
. | tee /dev/stderr |
yq -r '.spec.template.metadata.annotations."consul.hashicorp.com/config-checksum"' | tee /dev/stderr)
[ "${actual}" = b56ac97f873dd8c41c779964e24a458c58fc41404fa2642d3b12b24cd2091e43 ]
[ "${actual}" = 2c5397272acdc6fe5b079bf25c846c5a17f474603c794c64e7226ce0690625f7 ]
}

@test "server/StatefulSet: adds config-checksum annotation when extraConfig is provided" {
Expand All @@ -696,7 +696,7 @@ load _helpers
--set 'server.extraConfig="{\"hello\": \"world\"}"' \
. | tee /dev/stderr |
yq -r '.spec.template.metadata.annotations."consul.hashicorp.com/config-checksum"' | tee /dev/stderr)
[ "${actual}" = d637bd5c2a3738c2885d483fd003d59badc164883897e6032ac2dbe543bb6539 ]
[ "${actual}" = b0d22cb051216505edc0e61b57f9eacc0d7e15b24719d815842df88f06f1abe0 ]
}

@test "server/StatefulSet: adds config-checksum annotation when config is updated" {
Expand All @@ -706,7 +706,7 @@ load _helpers
--set 'global.acls.manageSystemACLs=true' \
. | tee /dev/stderr |
yq -r '.spec.template.metadata.annotations."consul.hashicorp.com/config-checksum"' | tee /dev/stderr)
[ "${actual}" = daf925cbf6af12cc87f5d7791370e28dddbe9ca78d8ad9fc963e558c60a333e7 ]
[ "${actual}" = 7772975be982e25cc8df101375374e2ba672a55737f8f1580011e0d88d8752a8 ]
}

#--------------------------------------------------------------------
Expand Down

0 comments on commit 0ad49a9

Please sign in to comment.