Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

operator: Allow disabling Grafana deployment #1241

Merged
merged 4 commits into from Jul 5, 2021
Merged

operator: Allow disabling Grafana deployment #1241

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e97ecb8
operator: Allow disabling Grafana deployment
bison Jul 5, 2021
ec2d198
tasks: Implement destroy method for Grafana
bison Jun 22, 2021
2fee744
test/e2e: Add test for config option to disable Grafana
bison Jul 5, 2021
91de7fb
Update CHANGELOG.md
bison Jun 23, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
@@ -1,5 +1,9 @@
# Note: This CHANGELOG is only for the monitoring team to track all monitoring related changes. Please see OpenShift release notes for official changes.

## 4.9

- [#1241](https://github.com/openshift/cluster-monitoring-operator/pull/1241) Add config option to disable Grafana deployment.

## 4.8

- [#1087](https://github.com/openshift/cluster-monitoring-operator/pull/1087) Decrease alert severity to "warning" for ThanosQueryHttpRequestQueryErrorRateHigh and ThanosQueryHttpRequestQueryRangeErrorRateHigh alerts.
Expand Down
4 changes: 0 additions & 4 deletions assets/prometheus-k8s/prometheus.yaml
View file
Open in desktop
Expand Up @@ -45,7 +45,6 @@ spec:
- -http-address=
- -email-domain=*
- -upstream=http://localhost:9090
- -htpasswd-file=/etc/proxy/htpasswd/auth
- -openshift-service-account=prometheus-k8s
- '-openshift-sar={"resource": "namespaces", "verb": "get"}'
- '-openshift-delegate-urls={"/": {"resource": "namespaces", "verb": "get"}}'
Expand Down Expand Up @@ -77,8 +76,6 @@ spec:
name: secret-prometheus-k8s-tls
- mountPath: /etc/proxy/secrets
name: secret-prometheus-k8s-proxy
- mountPath: /etc/proxy/htpasswd
name: secret-prometheus-k8s-htpasswd
- args:
- --secure-listen-address=0.0.0.0:9092
- --upstream=http://127.0.0.1:9095
Expand Down Expand Up @@ -199,7 +196,6 @@ spec:
- kube-etcd-client-certs
- prometheus-k8s-tls
- prometheus-k8s-proxy
- prometheus-k8s-htpasswd
- prometheus-k8s-thanos-sidecar-tls
- kube-rbac-proxy
securityContext:
Expand Down
6 changes: 0 additions & 6 deletions assets/thanos-querier/deployment.yaml
View file
Open in desktop
Expand Up @@ -94,7 +94,6 @@ spec:
- -http-address=
- -email-domain=*
- -upstream=http://localhost:9090
- -htpasswd-file=/etc/proxy/htpasswd/auth
- -openshift-service-account=thanos-querier
- '-openshift-sar={"resource": "namespaces", "verb": "get"}'
- '-openshift-delegate-urls={"/": {"resource": "namespaces", "verb": "get"}}'
Expand Down Expand Up @@ -126,8 +125,6 @@ spec:
name: secret-thanos-querier-tls
- mountPath: /etc/proxy/secrets
name: secret-thanos-querier-oauth-cookie
- mountPath: /etc/proxy/htpasswd
name: secret-thanos-querier-oauth-htpasswd
- args:
- --secure-listen-address=0.0.0.0:9092
- --upstream=http://127.0.0.1:9095
Expand Down Expand Up @@ -199,9 +196,6 @@ spec:
- name: secret-thanos-querier-oauth-cookie
secret:
secretName: thanos-querier-oauth-cookie
- name: secret-thanos-querier-oauth-htpasswd
secret:
secretName: thanos-querier-oauth-htpasswd
- name: secret-thanos-querier-kube-rbac-proxy
secret:
secretName: thanos-querier-kube-rbac-proxy
Expand Down
22 changes: 16 additions & 6 deletions jsonnet/prometheus.libsonnet
View file
Open in desktop
Expand Up @@ -272,6 +272,13 @@ function(params)
// These patches inject the oauth proxy as a sidecar and configures it with
// TLS. Additionally as the Alertmanager is protected with TLS, authN and
// authZ it requires some additonal configuration.
//
// Note that Grafana is enabled by default, but may be explicitly disabled
// by the user. We need to inject an htpasswd file for the oauth-proxy when
// it is enabled, so by default the operator also adds a few things at
// runtime: a volume and volume-mount for the secret, and an argument to the
// proxy container pointing to the mounted htpasswd file. If Grafana is
// disabled, these things are not injected.
prometheus+: {
spec+: {
alerting+: {
Expand Down Expand Up @@ -301,10 +308,11 @@ function(params)
runAsUser: 65534,
},
secrets+: [
// NOTE: The following is injected at runtime if Grafana is enabled:
// 'prometheus-k8s-htpasswd'
'kube-etcd-client-certs', //TODO(paulfantom): move it to etcd addon
'prometheus-k8s-tls',
'prometheus-k8s-proxy',
'prometheus-k8s-htpasswd',
'prometheus-k8s-thanos-sidecar-tls',
'kube-rbac-proxy',
],
Expand Down Expand Up @@ -348,12 +356,13 @@ function(params)
},
],
args: [
// NOTE: The following is injected at runtime if Grafana is enabled:
// '-htpasswd-file=/etc/proxy/htpasswd/auth'
'-provider=openshift',
'-https-address=:9091',
'-http-address=',
'-email-domain=*',
'-upstream=http://localhost:9090',
'-htpasswd-file=/etc/proxy/htpasswd/auth',
'-openshift-service-account=prometheus-k8s',
'-openshift-sar={"resource": "namespaces", "verb": "get"}',
'-openshift-delegate-urls={"/": {"resource": "namespaces", "verb": "get"}}',
Expand All @@ -366,6 +375,11 @@ function(params)
],
terminationMessagePolicy: 'FallbackToLogsOnError',
volumeMounts: [
// NOTE: The following is injected at runtime if Grafana is enabled:
// {
// mountPath: '/etc/proxy/htpasswd',
// name: 'secret-prometheus-k8s-htpasswd',
// },
{
mountPath: '/etc/tls/private',
name: 'secret-prometheus-k8s-tls',
Expand All @@ -374,10 +388,6 @@ function(params)
mountPath: '/etc/proxy/secrets',
name: 'secret-prometheus-k8s-proxy',
},
{
mountPath: '/etc/proxy/htpasswd',
name: 'secret-prometheus-k8s-htpasswd',
},
],
},
{
Expand Down
31 changes: 20 additions & 11 deletions jsonnet/thanos-querier.libsonnet
View file
Open in desktop
Expand Up @@ -265,6 +265,12 @@ function(params)
},
},

// Note that Grafana is enabled by default, but may be explicitly disabled
// by the user. We need to inject an htpasswd file for the oauth-proxy when
// it is enabled, so by default the operator also adds a few things at
// runtime: a volume and volume-mount for the secret, and an argument to the
// proxy container pointing to the mounted htpasswd file. If Grafana is
// disabled, these things are not injected.
deployment+: {
spec+: {
strategy+: {
Expand All @@ -291,6 +297,13 @@ function(params)
},
},
volumes+: [
// NOTE: If Grafana is enabled, the following is injected at runtime:
// {
// name: 'secret-thanos-querier-oauth-htpasswd',
// secret: {
// secretName: 'thanos-querier-oauth-htpasswd',
// },
// },
{
name: 'secret-thanos-querier-tls',
secret: {
Expand All @@ -303,12 +316,6 @@ function(params)
secretName: 'thanos-querier-oauth-cookie',
},
},
{
name: 'secret-thanos-querier-oauth-htpasswd',
secret: {
secretName: 'thanos-querier-oauth-htpasswd',
},
},
{
name: 'secret-thanos-querier-kube-rbac-proxy',
secret: {
Expand Down Expand Up @@ -394,12 +401,13 @@ function(params)
{ name: 'NO_PROXY', value: '' },
],
args: [
// NOTE: The following is injected at runtime if Grafana is enabled:
// '-htpasswd-file=/etc/proxy/htpasswd/auth'
'-provider=openshift',
'-https-address=:9091',
'-http-address=',
'-email-domain=*',
'-upstream=http://localhost:9090',
'-htpasswd-file=/etc/proxy/htpasswd/auth',
'-openshift-service-account=thanos-querier',
'-openshift-sar={"resource": "namespaces", "verb": "get"}',
'-openshift-delegate-urls={"/": {"resource": "namespaces", "verb": "get"}}',
Expand All @@ -412,6 +420,11 @@ function(params)
],
terminationMessagePolicy: 'FallbackToLogsOnError',
volumeMounts: [
// NOTE: The following is injected at runtime if Grafana is enabled:
// {
// mountPath: '/etc/proxy/htpasswd',
// name: 'secret-thanos-querier-oauth-htpasswd',
// },
{
mountPath: '/etc/tls/private',
name: 'secret-thanos-querier-tls',
Expand All @@ -420,10 +433,6 @@ function(params)
mountPath: '/etc/proxy/secrets',
name: 'secret-thanos-querier-oauth-cookie',
},
{
mountPath: '/etc/proxy/htpasswd',
name: 'secret-thanos-querier-oauth-htpasswd',
},
],
},
{
Expand Down
10 changes: 10 additions & 0 deletions pkg/client/client.go
View file
Open in desktop
Expand Up @@ -1024,6 +1024,16 @@ func (c *Client) CreateOrUpdateConfigMapList(cml *v1.ConfigMapList) error {
return nil
}

func (c *Client) DeleteConfigMapList(cml *v1.ConfigMapList) error {
for _, cm := range cml.Items {
err := c.DeleteConfigMap(&cm)
if err != nil {
return err
}
}
return nil
}

func (c *Client) CreateOrUpdateConfigMap(cm *v1.ConfigMap) error {
cmClient := c.kclient.CoreV1().ConfigMaps(cm.GetNamespace())
existing, err := cmClient.Get(context.TODO(), cm.GetName(), metav1.GetOptions{})
Expand Down
11 changes: 11 additions & 0 deletions pkg/manifests/config.go
View file
Open in desktop
Expand Up @@ -151,10 +151,21 @@ type ThanosQuerierConfig struct {
}

type GrafanaConfig struct {
Enabled *bool `json:"enabled"`
NodeSelector map[string]string `json:"nodeSelector"`
Tolerations []v1.Toleration `json:"tolerations"`
}

// IsEnabled returns the underlying value of the `Enabled` boolean pointer. It
// defaults to TRUE if the pointer is nil because Grafana should be enabled by
// default.
func (g *GrafanaConfig) IsEnabled() bool {
if g.Enabled == nil {
return true
}
return *g.Enabled
}

type KubeStateMetricsConfig struct {
NodeSelector map[string]string `json:"nodeSelector"`
Tolerations []v1.Toleration `json:"tolerations"`
Expand Down
43 changes: 43 additions & 0 deletions pkg/manifests/config_test.go
View file
Open in desktop
Expand Up @@ -234,3 +234,46 @@ func TestHttpProxyConfig(t *testing.T) {
}
}
}

func TestGrafanaDefaultsToEnabled(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice to convert this into a table driven test to make sure each test case is run independently of each other. Here is one such example: https://github.com/openshift/cluster-monitoring-operator/blob/master/pkg/manifests/manifests_test.go#L60

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK. I also prefer that. Honestly, I just copied the extremely similar test above this and changed it to test Grafana. I can try to find time to make it table driven.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

for _, tt := range []struct {
name string
config string
expectEnabled bool
}{
{
name: "empty config",
config: "",
expectEnabled: true,
},
{
name: "empty grafana config",
config: `{"grafana":{}}`,
expectEnabled: true,
},
{
name: "grafana explicitly enabled",
config: `{"grafana":{"enabled": true}}`,
expectEnabled: true,
},
{
name: "grafana disabled",
config: `{"grafana":{"enabled": false}}`,
expectEnabled: false,
},
} {
t.Run(tt.name, func(t *testing.T) {
c, err := NewConfigFromString(tt.config)
if err != nil {
t.Fatal(err)
}

enabled := c.ClusterMonitoringConfiguration.GrafanaConfig.IsEnabled()

if enabled != tt.expectEnabled {
t.Fatalf("GrafanaConfig.IsEnabled() returned %t, expected %t",
enabled, tt.expectEnabled)
}
})
}
}