fix successThreshold, validate server protocol, deprecate scheme (#792)

* fix: success parameter in probes; restructure probes generation Signed-off-by: Igor Beliakov <demtis.register@gmail.com> * chore: rewrite probe scheme logic Signed-off-by: Igor Beliakov <demtis.register@gmail.com> * fix: remove probe scheme duplication Signed-off-by: Igor Beliakov <demtis.register@gmail.com> * chore: improve code readability Signed-off-by: Igor Beliakov <demtis.register@gmail.com> * chore: refactor probes Signed-off-by: Igor Beliakov <demtis.register@gmail.com> * chore: add tests for probes Signed-off-by: Igor Beliakov <demtis.register@gmail.com> * deprecate scheme in probes Signed-off-by: Igor Beliakov <demtis.register@gmail.com> * validate server protocol through crds Signed-off-by: Igor Beliakov <demtis.register@gmail.com> * update latest crds Signed-off-by: Igor Beliakov <demtis.register@gmail.com> * update bundle crds Signed-off-by: Igor Beliakov <demtis.register@gmail.com> * Rename test Signed-off-by: Igor Beliakov <demtis.register@gmail.com> * Bring scheme back to avoid any incompatibility in v4 * Update CRDs Signed-off-by: Igor Beliakov <demtis.register@gmail.com> * update tests to make them use ProbeHandler Signed-off-by: Igor Beliakov <demtis.register@gmail.com> Co-authored-by: Hubert Stefanski <35736504+HubertStefanski@users.noreply.github.com>
grafana · Jul 20, 2022 · 1ff83ec · 1ff83ec
1 parent 4b1c75b
commit 1ff83ec
Show file tree

Hide file tree

Showing 9 changed files with 299 additions and 112 deletions.
diff --git a/api/integreatly/v1alpha1/grafana_types.go b/api/integreatly/v1alpha1/grafana_types.go
@@ -45,20 +45,22 @@ type GrafanaSpec struct {
 }
 
 type ReadinessProbeSpec struct {
-	InitialDelaySeconds *int32       `json:"initialDelaySeconds,omitempty"`
-	TimeOutSeconds      *int32       `json:"timeoutSeconds,omitempty"`
-	PeriodSeconds       *int32       `json:"periodSeconds,omitempty"`
-	SuccessThreshold    *int32       `json:"successThreshold,omitempty"`
-	FailureThreshold    *int32       `json:"failureThreshold,omitempty"`
-	Scheme              v1.URIScheme `json:"scheme,omitempty"`
+	InitialDelaySeconds *int32 `json:"initialDelaySeconds,omitempty"`
+	TimeOutSeconds      *int32 `json:"timeoutSeconds,omitempty"`
+	PeriodSeconds       *int32 `json:"periodSeconds,omitempty"`
+	SuccessThreshold    *int32 `json:"successThreshold,omitempty"`
+	FailureThreshold    *int32 `json:"failureThreshold,omitempty"`
+	// URIScheme identifies the scheme used for connection to a host for Get actions. Deprecated in favor of config.server.protocol.
+	Scheme v1.URIScheme `json:"scheme,omitempty"`
 }
 type LivenessProbeSpec struct {
-	InitialDelaySeconds *int32       `json:"initialDelaySeconds,omitempty"`
-	TimeOutSeconds      *int32       `json:"timeoutSeconds,omitempty"`
-	PeriodSeconds       *int32       `json:"periodSeconds,omitempty"`
-	SuccessThreshold    *int32       `json:"successThreshold,omitempty"`
-	FailureThreshold    *int32       `json:"failureThreshold,omitempty"`
-	Scheme              v1.URIScheme `json:"scheme,omitempty"`
+	InitialDelaySeconds *int32 `json:"initialDelaySeconds,omitempty"`
+	TimeOutSeconds      *int32 `json:"timeoutSeconds,omitempty"`
+	PeriodSeconds       *int32 `json:"periodSeconds,omitempty"`
+	SuccessThreshold    *int32 `json:"successThreshold,omitempty"`
+	FailureThreshold    *int32 `json:"failureThreshold,omitempty"`
+	// URIScheme identifies the scheme used for connection to a host for Get actions. Deprecated in favor of config.server.protocol.
+	Scheme v1.URIScheme `json:"scheme,omitempty"`
 }
 
 type JsonnetConfig struct {
@@ -199,6 +201,7 @@ type GrafanaConfigPaths struct {
 type GrafanaConfigServer struct {
 	HttpAddr string `json:"http_addr,omitempty" ini:"http_addr,omitempty"`
 	HttpPort string `json:"http_port,omitempty" ini:"http_port,omitempty"`
+	// +kubebuilder:validation:Enum=http;https
 	Protocol string `json:"protocol,omitempty" ini:"protocol,omitempty"`
 	Socket   string `json:"socket,omitempty" ini:"socket,omitempty"`
 	Domain   string `json:"domain,omitempty" ini:"domain,omitempty"`
@@ -693,3 +696,10 @@ func (cr *Grafana) GetPreferServiceValue() bool {
 	}
 	return false
 }
+
+func (cr *Grafana) GetScheme() v1.URIScheme {
+	if cr.Spec.Config.Server != nil && cr.Spec.Config.Server.Protocol == "https" {
+		return v1.URISchemeHTTPS
+	}
+	return v1.URISchemeHTTP
+}
diff --git a/api/integreatly/v1alpha1/grafana_types_test.go b/api/integreatly/v1alpha1/grafana_types_test.go
@@ -0,0 +1,66 @@
+package v1alpha1
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	v1 "k8s.io/api/core/v1"
+)
+
+func TestGrafana_GetScheme(t *testing.T) {
+	tests := []struct {
+		name string
+		cr   *Grafana
+		want v1.URIScheme
+	}{
+		{
+			name: "Nil server spec",
+			cr:   &Grafana{},
+			want: v1.URISchemeHTTP,
+		},
+		{
+			name: "Empty server spec",
+			cr: &Grafana{
+				Spec: GrafanaSpec{
+					Config: GrafanaConfig{
+						Server: &GrafanaConfigServer{},
+					},
+				},
+			},
+			want: v1.URISchemeHTTP,
+		},
+		{
+			name: "HTTP in server spec",
+			cr: &Grafana{
+				Spec: GrafanaSpec{
+					Config: GrafanaConfig{
+						Server: &GrafanaConfigServer{
+							Protocol: "http",
+						},
+					},
+				},
+			},
+			want: v1.URISchemeHTTP,
+		},
+		{
+			name: "HTTPS in server spec",
+			cr: &Grafana{
+				Spec: GrafanaSpec{
+					Config: GrafanaConfig{
+						Server: &GrafanaConfigServer{
+							Protocol: "https",
+						},
+					},
+				},
+			},
+			want: v1.URISchemeHTTPS,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := tt.cr.GetScheme()
+			assert.Equal(t, tt.want, got)
+		})
+	}
+}
diff --git a/bundle/manifests/integreatly.org_grafanas.yaml b/bundle/manifests/integreatly.org_grafanas.yaml
@@ -655,6 +655,9 @@ spec:
                         type: string
                       protocol:
                         type: string
+                        enum:
+                         - http
+                         - https
                       root_url:
                         type: string
                       router_logging:
@@ -4808,7 +4811,7 @@ spec:
                     type: integer
                   scheme:
                     description: URIScheme identifies the scheme used for connection
-                      to a host for Get actions
+                      to a host for Get actions. Deprecated in favor of config.server.protocol.
                     type: string
                   successThreshold:
                     format: int32
@@ -4830,7 +4833,7 @@ spec:
                     type: integer
                   scheme:
                     description: URIScheme identifies the scheme used for connection
-                      to a host for Get actions
+                      to a host for Get actions. Deprecated in favor of config.server.protocol.
                     type: string
                   successThreshold:
                     format: int32

diff --git a/config/crd/bases/integreatly.org_grafanas.yaml b/config/crd/bases/integreatly.org_grafanas.yaml
@@ -656,6 +656,9 @@ spec:
                       http_port:
                         type: string
                       protocol:
+                        enum:
+                        - http
+                        - https
                         type: string
                       root_url:
                         type: string
@@ -5295,7 +5298,7 @@ spec:
                     type: integer
                   scheme:
                     description: URIScheme identifies the scheme used for connection
-                      to a host for Get actions
+                      to a host for Get actions. Deprecated in favor of config.server.protocol.
                     type: string
                   successThreshold:
                     format: int32
@@ -5317,7 +5320,7 @@ spec:
                     type: integer
                   scheme:
                     description: URIScheme identifies the scheme used for connection
-                      to a host for Get actions
+                      to a host for Get actions. Deprecated in favor of config.server.protocol.
                     type: string
                   successThreshold:
                     format: int32

diff --git a/controllers/model/grafanaDeployment.go b/controllers/model/grafanaDeployment.go
@@ -16,14 +16,24 @@ import (
 )
 
 const (
-	InitMemoryRequest = "128Mi"
-	InitCpuRequest    = "250m"
-	InitMemoryLimit   = "512Mi"
-	InitCpuLimit      = "1000m"
-	MemoryRequest     = "256Mi"
-	CpuRequest        = "100m"
-	MemoryLimit       = "1024Mi"
-	CpuLimit          = "500m"
+	InitMemoryRequest                       = "128Mi"
+	InitCpuRequest                          = "250m"
+	InitMemoryLimit                         = "512Mi"
+	InitCpuLimit                            = "1000m"
+	MemoryRequest                           = "256Mi"
+	CpuRequest                              = "100m"
+	MemoryLimit                             = "1024Mi"
+	CpuLimit                                = "500m"
+	LivenessProbeFailureThreshold     int32 = 10
+	LivenessProbeInitialDelaySeconds  int32 = 60
+	LivenessProbePeriodSeconds        int32 = 10
+	LivenessProbeSuccessThreshold     int32 = 1
+	LivenessProbeTimeoutSeconds       int32 = 30
+	ReadinessProbeFailureThreshold    int32 = 1
+	ReadinessProbeInitialDelaySeconds int32 = 5
+	ReadinessProbePeriodSeconds       int32 = 10
+	ReadinessProbeSuccessThreshold    int32 = 1
+	ReadinessProbeTimeoutSeconds      int32 = 3
 )
 
 func getSkipCreateAdminAccount(cr *v1alpha1.Grafana) bool {
@@ -448,99 +458,47 @@ func getVolumeMounts(cr *v1alpha1.Grafana) []v13.VolumeMount {
 	return mounts
 }
 
-func getLivenessProbe(cr *v1alpha1.Grafana, delay, timeout, failure int32) *v13.Probe {
-	var period int32 = 10
-	var success int32 = 1
-	scheme := v13.URISchemeHTTP
-	if cr.Spec.Config.Server != nil && cr.Spec.Config.Server.Protocol == "https" {
-		scheme = v13.URISchemeHTTPS
-	}
-
-	if cr.Spec.LivenessProbeSpec != nil && cr.Spec.LivenessProbeSpec.InitialDelaySeconds != nil {
-		delay = *cr.Spec.LivenessProbeSpec.InitialDelaySeconds
-	}
-
-	if cr.Spec.LivenessProbeSpec != nil && cr.Spec.LivenessProbeSpec.TimeOutSeconds != nil {
-		timeout = *cr.Spec.LivenessProbeSpec.TimeOutSeconds
-	}
-
-	if cr.Spec.LivenessProbeSpec != nil && cr.Spec.LivenessProbeSpec.FailureThreshold != nil {
-		failure = *cr.Spec.LivenessProbeSpec.FailureThreshold
-	}
-
-	if cr.Spec.LivenessProbeSpec != nil && cr.Spec.LivenessProbeSpec.PeriodSeconds != nil {
-		period = *cr.Spec.LivenessProbeSpec.PeriodSeconds
-	}
-
-	if cr.Spec.LivenessProbeSpec != nil && cr.Spec.LivenessProbeSpec.SuccessThreshold != nil {
-		period = *cr.Spec.LivenessProbeSpec.SuccessThreshold
-	}
-
-	if cr.Spec.LivenessProbeSpec != nil && cr.Spec.LivenessProbeSpec.Scheme != "" {
-		scheme = cr.Spec.LivenessProbeSpec.Scheme
+func getLivenessProbe(cr *v1alpha1.Grafana) *v13.Probe {
+	spec := &v1alpha1.LivenessProbeSpec{}
+	if cr.Spec.LivenessProbeSpec != nil {
+		spec = cr.Spec.LivenessProbeSpec
 	}
 
 	return &v13.Probe{
 		ProbeHandler: v13.ProbeHandler{
 			HTTPGet: &v13.HTTPGetAction{
 				Path:   constants.GrafanaHealthEndpoint,
 				Port:   intstr.FromInt(GetGrafanaPort(cr)),
-				Scheme: scheme,
+				Scheme: cr.GetScheme(),
 			},
 		},
-		InitialDelaySeconds: delay,
-		TimeoutSeconds:      timeout,
-		PeriodSeconds:       period,
-		SuccessThreshold:    success,
-		FailureThreshold:    failure,
+		InitialDelaySeconds: getDefaultInt32(spec.InitialDelaySeconds, LivenessProbeInitialDelaySeconds),
+		TimeoutSeconds:      getDefaultInt32(spec.TimeOutSeconds, LivenessProbeTimeoutSeconds),
+		PeriodSeconds:       getDefaultInt32(spec.PeriodSeconds, LivenessProbePeriodSeconds),
+		SuccessThreshold:    getDefaultInt32(spec.SuccessThreshold, LivenessProbeSuccessThreshold),
+		FailureThreshold:    getDefaultInt32(spec.FailureThreshold, LivenessProbeFailureThreshold),
 	}
 }
 
-func getReadinessProbe(cr *v1alpha1.Grafana, delay, timeout, failure int32) *v13.Probe {
-	var period int32 = 10
-	var success int32 = 1
-	scheme := v13.URISchemeHTTP
-	if cr.Spec.Config.Server != nil && cr.Spec.Config.Server.Protocol == "https" {
-		scheme = v13.URISchemeHTTPS
-	}
-
-	if cr.Spec.ReadinessProbeSpec != nil && cr.Spec.ReadinessProbeSpec.InitialDelaySeconds != nil {
-		delay = *cr.Spec.ReadinessProbeSpec.InitialDelaySeconds
-	}
-
-	if cr.Spec.ReadinessProbeSpec != nil && cr.Spec.ReadinessProbeSpec.TimeOutSeconds != nil {
-		timeout = *cr.Spec.ReadinessProbeSpec.TimeOutSeconds
-	}
-
-	if cr.Spec.ReadinessProbeSpec != nil && cr.Spec.ReadinessProbeSpec.FailureThreshold != nil {
-		failure = *cr.Spec.ReadinessProbeSpec.FailureThreshold
-	}
-
-	if cr.Spec.ReadinessProbeSpec != nil && cr.Spec.ReadinessProbeSpec.PeriodSeconds != nil {
-		period = *cr.Spec.ReadinessProbeSpec.PeriodSeconds
-	}
-
-	if cr.Spec.ReadinessProbeSpec != nil && cr.Spec.ReadinessProbeSpec.SuccessThreshold != nil {
-		period = *cr.Spec.ReadinessProbeSpec.SuccessThreshold
-	}
-
-	if cr.Spec.ReadinessProbeSpec != nil && cr.Spec.ReadinessProbeSpec.Scheme != "" {
-		scheme = cr.Spec.ReadinessProbeSpec.Scheme
+func getReadinessProbe(cr *v1alpha1.Grafana) *v13.Probe {
+	spec := &v1alpha1.ReadinessProbeSpec{}
+	if cr.Spec.ReadinessProbeSpec != nil {
+		spec = cr.Spec.ReadinessProbeSpec
 	}
 
 	return &v13.Probe{
 		ProbeHandler: v13.ProbeHandler{
 			HTTPGet: &v13.HTTPGetAction{
 				Path:   constants.GrafanaHealthEndpoint,
 				Port:   intstr.FromInt(GetGrafanaPort(cr)),
-				Scheme: scheme,
+				Scheme: cr.GetScheme(),
 			},
 		},
-		InitialDelaySeconds: delay,
-		TimeoutSeconds:      timeout,
-		PeriodSeconds:       period,
-		SuccessThreshold:    success,
-		FailureThreshold:    failure,
+		InitialDelaySeconds: getDefaultInt32(spec.InitialDelaySeconds, ReadinessProbeInitialDelaySeconds),
+		TimeoutSeconds:      getDefaultInt32(spec.TimeOutSeconds, ReadinessProbeTimeoutSeconds),
+		PeriodSeconds:       getDefaultInt32(spec.PeriodSeconds, ReadinessProbePeriodSeconds),
+		SuccessThreshold:    getDefaultInt32(spec.SuccessThreshold, ReadinessProbeSuccessThreshold),
+		FailureThreshold:    getDefaultInt32(spec.FailureThreshold, ReadinessProbeFailureThreshold),
 	}
 }
 
@@ -610,8 +568,8 @@ func getContainers(cr *v1alpha1.Grafana, configHash, dsHash, credentialsHash str
 		EnvFrom:                  getEnvFrom(cr),
 		Resources:                getResources(cr),
 		VolumeMounts:             getVolumeMounts(cr),
-		LivenessProbe:            getLivenessProbe(cr, 60, 30, 10),
-		ReadinessProbe:           getReadinessProbe(cr, 5, 3, 1),
+		LivenessProbe:            getLivenessProbe(cr),
+		ReadinessProbe:           getReadinessProbe(cr),
 		TerminationMessagePath:   "/dev/termination-log",
 		TerminationMessagePolicy: "File",
 		ImagePullPolicy:          "IfNotPresent",