From fd6945747c4fd12d32e4fc98f1829f2554434686 Mon Sep 17 00:00:00 2001 From: Julio Chana Date: Fri, 14 Sep 2018 12:14:23 +0200 Subject: [PATCH 1/6] [DEVOPS-824] Add a limit of 58 characters for RF names --- README.md | 1 + api/redisfailover/v1alpha2/validate.go | 9 +++++++++ 2 files changed, 10 insertions(+) diff --git a/README.md b/README.md index a5a6dd4b6..6cc008c42 100644 --- a/README.md +++ b/README.md @@ -50,6 +50,7 @@ This redis-failover will be managed by the operator, resulting in the following * `rfs-`: Sentinel service **NOTE**: `NAME` is the named provided when creating the RedisFailover. +**IMPORTANT**: the name of the redis-failover to be created cannot be longer that 58 characters, due to prepend of redis/sentinel identification. ### Persistance The operator has the ability of add persistance to Redis data. By default an `emptyDir` will be used, so the data is not saved. diff --git a/api/redisfailover/v1alpha2/validate.go b/api/redisfailover/v1alpha2/validate.go index 3d073aa2b..b12ba37fa 100644 --- a/api/redisfailover/v1alpha2/validate.go +++ b/api/redisfailover/v1alpha2/validate.go @@ -2,10 +2,19 @@ package v1alpha2 import ( "errors" + "fmt" +) + +const ( + maxNameLength = 58 ) // Validate set the values by default if not defined and checks if the values given are valid func (r *RedisFailover) Validate() error { + if len(r.Name) > maxNameLength { + return fmt.Errorf("name length can't be higher than %d", maxNameLength) + } + if r.Spec.Redis.Replicas == 0 { r.Spec.Redis.Replicas = defaultRedisNumber } else if r.Spec.Redis.Replicas < defaultRedisNumber { From b7f93b4a7526fd947e0ef45e0a72c65b95bca28a Mon Sep 17 00:00:00 2001 From: Julio Chana Date: Fri, 14 Sep 2018 12:19:20 +0200 Subject: [PATCH 2/6] [DEVOPS-824] Reduce shutdown configmap name to be max of 5 chars --- operator/redisfailover/service/constants.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/operator/redisfailover/service/constants.go b/operator/redisfailover/service/constants.go index 1ce7f320a..5cb650c9f 100644 --- a/operator/redisfailover/service/constants.go +++ b/operator/redisfailover/service/constants.go @@ -25,7 +25,7 @@ const ( sentinelConfigFileName = "sentinel.conf" redisConfigFileName = "redis.conf" redisName = "r" - redisShutdownName = "r-shutdown" + redisShutdownName = "r-s" redisRoleName = "redis" redisGroupName = "mymaster" appLabel = "redis-failover" From 7a7eeda01a3cb6e95e4417a0bdf765fc66b0b062 Mon Sep 17 00:00:00 2001 From: Julio Chana Date: Fri, 14 Sep 2018 12:19:45 +0200 Subject: [PATCH 3/6] [DEVOPS-824] Add set as failed if no heal is done --- operator/redisfailover/checker.go | 2 ++ operator/redisfailover/handler.go | 1 + 2 files changed, 3 insertions(+) diff --git a/operator/redisfailover/checker.go b/operator/redisfailover/checker.go index e5f806fba..35d0d349d 100644 --- a/operator/redisfailover/checker.go +++ b/operator/redisfailover/checker.go @@ -43,6 +43,7 @@ func (r *RedisFailoverHandler) CheckAndHeal(rf *redisfailoverv1alpha2.RedisFailo r.logger.Debugf("Time %.f more than expected. Not even one master, fixing...", minTime.Round(time.Second).Seconds()) // We can consider there's an error if err2 := r.rfHealer.SetRandomMaster(rf); err2 != nil { + r.mClient.SetClusterError(rf.Namespace, rf.Name) return err2 } } else { @@ -86,6 +87,7 @@ func (r *RedisFailoverHandler) CheckAndHeal(rf *redisfailoverv1alpha2.RedisFailo if err := r.rfChecker.CheckSentinelMonitor(sip, master); err != nil { r.logger.Debug("Sentinel is not monitoring the correct master") if err := r.rfHealer.NewSentinelMonitor(sip, master, rf); err != nil { + r.mClient.SetClusterError(rf.Namespace, rf.Name) return err } } diff --git a/operator/redisfailover/handler.go b/operator/redisfailover/handler.go index 57bb06367..085d4e400 100644 --- a/operator/redisfailover/handler.go +++ b/operator/redisfailover/handler.go @@ -63,6 +63,7 @@ func (r *RedisFailoverHandler) Add(_ context.Context, obj runtime.Object) error } if err := rf.Validate(); err != nil { + r.mClient.SetClusterError(rf.Namespace, rf.Name) return err } From f82dce797285ac4f31c030b6af36070bf52c4bb0 Mon Sep 17 00:00:00 2001 From: Julio Chana Date: Fri, 14 Sep 2018 13:39:08 +0200 Subject: [PATCH 4/6] [DEVOPS-824] Change limit to 48 characters for RF names --- README.md | 2 +- api/redisfailover/v1alpha2/validate.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 6cc008c42..8c0a0d15a 100644 --- a/README.md +++ b/README.md @@ -50,7 +50,7 @@ This redis-failover will be managed by the operator, resulting in the following * `rfs-`: Sentinel service **NOTE**: `NAME` is the named provided when creating the RedisFailover. -**IMPORTANT**: the name of the redis-failover to be created cannot be longer that 58 characters, due to prepend of redis/sentinel identification. +**IMPORTANT**: the name of the redis-failover to be created cannot be longer that 48 characters, due to prepend of redis/sentinel identification and statefulset limitation. ### Persistance The operator has the ability of add persistance to Redis data. By default an `emptyDir` will be used, so the data is not saved. diff --git a/api/redisfailover/v1alpha2/validate.go b/api/redisfailover/v1alpha2/validate.go index b12ba37fa..09154b064 100644 --- a/api/redisfailover/v1alpha2/validate.go +++ b/api/redisfailover/v1alpha2/validate.go @@ -6,7 +6,7 @@ import ( ) const ( - maxNameLength = 58 + maxNameLength = 48 ) // Validate set the values by default if not defined and checks if the values given are valid From 11a358449fed58ef8d126d83fa3c9933029c24d0 Mon Sep 17 00:00:00 2001 From: Julio Chana Date: Fri, 14 Sep 2018 13:39:42 +0200 Subject: [PATCH 5/6] [DEVOPS-824] Delete metric if redis-failover no longer exist --- metrics/dummy.go | 1 + metrics/metrics.go | 6 ++++++ metrics/metrics_test.go | 21 +++++++++++++++++++++ operator/redisfailover/handler.go | 5 +++++ 4 files changed, 33 insertions(+) diff --git a/metrics/dummy.go b/metrics/dummy.go index b9f42e637..49480e5e9 100644 --- a/metrics/dummy.go +++ b/metrics/dummy.go @@ -8,3 +8,4 @@ type dummy struct{} func (d *dummy) SetClusterOK(namespace string, name string) {} func (d *dummy) SetClusterError(namespace string, name string) {} +func (d *dummy) DeleteCluster(namespace string, name string) {} diff --git a/metrics/metrics.go b/metrics/metrics.go index 67e808cf4..78e5549a5 100644 --- a/metrics/metrics.go +++ b/metrics/metrics.go @@ -15,6 +15,7 @@ const ( type Instrumenter interface { SetClusterOK(namespace string, name string) SetClusterError(namespace string, name string) + DeleteCluster(namespace string, name string) } // PromMetrics implements the instrumenter so the metrics can be managed by Prometheus. @@ -71,3 +72,8 @@ func (p *PromMetrics) SetClusterOK(namespace string, name string) { func (p *PromMetrics) SetClusterError(namespace string, name string) { p.clusterOK.WithLabelValues(namespace, name).Set(0) } + +// DeleteCluster set the cluster status to Error +func (p *PromMetrics) DeleteCluster(namespace string, name string) { + p.clusterOK.DeleteLabelValues(namespace, name) +} diff --git a/metrics/metrics_test.go b/metrics/metrics_test.go index fb34fa4fa..3c08cc6a6 100644 --- a/metrics/metrics_test.go +++ b/metrics/metrics_test.go @@ -87,6 +87,27 @@ func TestPrometheusMetrics(t *testing.T) { }, expCode: http.StatusOK, }, + { + name: "Deleting a cluster should remove it", + addMetrics: func(pm *metrics.PromMetrics) { + pm.SetClusterOK("testns1", "test") + pm.DeleteCluster("testns1", "test") + }, + expMetrics: []string{}, + expCode: http.StatusOK, + }, + { + name: "Deleting a cluster should remove only the desired one", + addMetrics: func(pm *metrics.PromMetrics) { + pm.SetClusterOK("testns1", "test") + pm.SetClusterOK("testns2", "test") + pm.DeleteCluster("testns1", "test") + }, + expMetrics: []string{ + `my_metrics_controller_cluster_ok{name="test",namespace="testns2"} 1`, + }, + expCode: http.StatusOK, + }, } for _, test := range tests { diff --git a/operator/redisfailover/handler.go b/operator/redisfailover/handler.go index 085d4e400..cf0329b30 100644 --- a/operator/redisfailover/handler.go +++ b/operator/redisfailover/handler.go @@ -3,6 +3,7 @@ package redisfailover import ( "context" "fmt" + "strings" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -83,6 +84,10 @@ func (r *RedisFailoverHandler) Add(_ context.Context, obj runtime.Object) error // Delete handles the deletion of a RF. func (r *RedisFailoverHandler) Delete(_ context.Context, name string) error { + n := strings.Split(name, "/") + if len(n) >= 2 { + r.mClient.DeleteCluster(n[0], n[1]) + } // No need to do anything, it will be handled by the owner reference done // on the creation. r.logger.Debugf("ignoring, kubernetes GCs all using the objects OwnerReference metadata") From b4a14296b90444e7b6c621ecfd2c05424bd6b3f1 Mon Sep 17 00:00:00 2001 From: Julio Chana Date: Fri, 14 Sep 2018 15:28:49 +0200 Subject: [PATCH 6/6] [DEVOPS-824] Bump version to 0.5.3 --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 48505c188..bc962cac3 100644 --- a/Makefile +++ b/Makefile @@ -1,4 +1,4 @@ -VERSION := 0.5.2 +VERSION := 0.5.3 # Name of this service/application SERVICE_NAME := redis-operator