From e6bbae502e4d57a00ca4c14ca133b72d4790e864 Mon Sep 17 00:00:00 2001 From: Adam Harrison-Fuller Date: Thu, 24 Oct 2019 09:47:24 +0100 Subject: [PATCH 01/10] Add label Blacklisting --- api/redisfailover/v1/types.go | 1 + api/redisfailover/v1/zz_generated.deepcopy.go | 19 +++++++++++++++++++ operator/redisfailover/handler.go | 11 ++++++++++- 3 files changed, 30 insertions(+), 1 deletion(-) diff --git a/api/redisfailover/v1/types.go b/api/redisfailover/v1/types.go index ed12ee4d4..a46c96b30 100644 --- a/api/redisfailover/v1/types.go +++ b/api/redisfailover/v1/types.go @@ -20,6 +20,7 @@ type RedisFailoverSpec struct { Redis RedisSettings `json:"redis,omitempty"` Sentinel SentinelSettings `json:"sentinel,omitempty"` Auth AuthSettings `json:"auth,omitempty"` + LabelBlacklist []string `json:"labelBlacklist,omitempty"` } // RedisSettings defines the specification of the redis cluster diff --git a/api/redisfailover/v1/zz_generated.deepcopy.go b/api/redisfailover/v1/zz_generated.deepcopy.go index 9e464f819..eba11df61 100644 --- a/api/redisfailover/v1/zz_generated.deepcopy.go +++ b/api/redisfailover/v1/zz_generated.deepcopy.go @@ -123,6 +123,11 @@ func (in *RedisFailoverSpec) DeepCopyInto(out *RedisFailoverSpec) { in.Redis.DeepCopyInto(&out.Redis) in.Sentinel.DeepCopyInto(&out.Sentinel) out.Auth = in.Auth + if in.LabelBlacklist != nil { + in, out := &in.LabelBlacklist, &out.LabelBlacklist + *out = make([]string, len(*in)) + copy(*out, *in) + } return } @@ -177,6 +182,13 @@ func (in *RedisSettings) DeepCopyInto(out *RedisSettings) { (*in)[i].DeepCopyInto(&(*out)[i]) } } + if in.PodAnnotations != nil { + in, out := &in.PodAnnotations, &out.PodAnnotations + *out = make(map[string]string, len(*in)) + for key, val := range *in { + (*out)[key] = val + } + } return } @@ -263,6 +275,13 @@ func (in *SentinelSettings) DeepCopyInto(out *SentinelSettings) { (*in)[i].DeepCopyInto(&(*out)[i]) } } + if in.PodAnnotations != nil { + in, out := &in.PodAnnotations, &out.PodAnnotations + *out = make(map[string]string, len(*in)) + for key, val := range *in { + (*out)[key] = val + } + } return } diff --git a/operator/redisfailover/handler.go b/operator/redisfailover/handler.go index 10d0855df..2b30cca4a 100644 --- a/operator/redisfailover/handler.go +++ b/operator/redisfailover/handler.go @@ -102,7 +102,16 @@ func (r *RedisFailoverHandler) getLabels(rf *redisfailoverv1.RedisFailover) map[ dynLabels := map[string]string{ rfLabelNameKey: rf.Name, } - return util.MergeLabels(defaultLabels, dynLabels, rf.Labels) + + filteredCustomLabels := rf.Labels + for _, label := range rf.Spec.LabelBlacklist { + if _, ok := filteredCustomLabels[label]; ok { + delete(filteredCustomLabels, label); + r.logger.Debugf("Removing %s from labels as it is blacklisted", label) + } + } + + return util.MergeLabels(defaultLabels, dynLabels, filteredCustomLabels) } func (w *RedisFailoverHandler) createOwnerReferences(rf *redisfailoverv1.RedisFailover) []metav1.OwnerReference { From f0838038037009b8962cf17430531084de4f5d1e Mon Sep 17 00:00:00 2001 From: Adam Harrison-Fuller Date: Thu, 24 Oct 2019 13:09:03 +0100 Subject: [PATCH 02/10] Make it a regexp compatible whitelist --- api/redisfailover/v1/types.go | 2 +- api/redisfailover/v1/zz_generated.deepcopy.go | 4 ++-- operator/redisfailover/handler.go | 24 +++++++++++++++---- 3 files changed, 22 insertions(+), 8 deletions(-) diff --git a/api/redisfailover/v1/types.go b/api/redisfailover/v1/types.go index a46c96b30..019f5cc01 100644 --- a/api/redisfailover/v1/types.go +++ b/api/redisfailover/v1/types.go @@ -20,7 +20,7 @@ type RedisFailoverSpec struct { Redis RedisSettings `json:"redis,omitempty"` Sentinel SentinelSettings `json:"sentinel,omitempty"` Auth AuthSettings `json:"auth,omitempty"` - LabelBlacklist []string `json:"labelBlacklist,omitempty"` + LabelWhitelist []string `json:"labelWhitelist,omitempty"` } // RedisSettings defines the specification of the redis cluster diff --git a/api/redisfailover/v1/zz_generated.deepcopy.go b/api/redisfailover/v1/zz_generated.deepcopy.go index eba11df61..9c82a8313 100644 --- a/api/redisfailover/v1/zz_generated.deepcopy.go +++ b/api/redisfailover/v1/zz_generated.deepcopy.go @@ -123,8 +123,8 @@ func (in *RedisFailoverSpec) DeepCopyInto(out *RedisFailoverSpec) { in.Redis.DeepCopyInto(&out.Redis) in.Sentinel.DeepCopyInto(&out.Sentinel) out.Auth = in.Auth - if in.LabelBlacklist != nil { - in, out := &in.LabelBlacklist, &out.LabelBlacklist + if in.LabelWhitelist != nil { + in, out := &in.LabelWhitelist, &out.LabelWhitelist *out = make([]string, len(*in)) copy(*out, *in) } diff --git a/operator/redisfailover/handler.go b/operator/redisfailover/handler.go index 2b30cca4a..cafdfc64c 100644 --- a/operator/redisfailover/handler.go +++ b/operator/redisfailover/handler.go @@ -3,6 +3,7 @@ package redisfailover import ( "context" "fmt" + "regexp" "strings" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -103,14 +104,27 @@ func (r *RedisFailoverHandler) getLabels(rf *redisfailoverv1.RedisFailover) map[ rfLabelNameKey: rf.Name, } - filteredCustomLabels := rf.Labels - for _, label := range rf.Spec.LabelBlacklist { - if _, ok := filteredCustomLabels[label]; ok { - delete(filteredCustomLabels, label); - r.logger.Debugf("Removing %s from labels as it is blacklisted", label) + // Filter the labels based on the whitelist + filteredCustomLabels := make(map[string]string) + if len(rf.Spec.LabelWhitelist) != 0 { + for labelKey, labelValue := range rf.Labels { + for _, regex := range rf.Spec.LabelWhitelist { + compiledRegexp, err := regexp.Compile(regex) + if err != nil { + r.logger.Fatalf("Unable to compile regex: %s", regex) + } + match := compiledRegexp.MatchString(labelKey) + if match { + filteredCustomLabels[labelKey]=labelValue + } + } } + } else { + // If no whitelist is specified then dont filter label. + filteredCustomLabels = rf.Labels } + r.logger.Infof("%#v", filteredCustomLabels) return util.MergeLabels(defaultLabels, dynLabels, filteredCustomLabels) } From 0aafb1ad7b133d7df0c0826bdc51c4f61b6dd765 Mon Sep 17 00:00:00 2001 From: Adam Harrison-Fuller Date: Thu, 24 Oct 2019 13:12:12 +0100 Subject: [PATCH 03/10] Change to MustCompile to it panics when a invalid regex is specified, better to fail and make it obvious --- operator/redisfailover/handler.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/operator/redisfailover/handler.go b/operator/redisfailover/handler.go index cafdfc64c..896fa7041 100644 --- a/operator/redisfailover/handler.go +++ b/operator/redisfailover/handler.go @@ -109,10 +109,7 @@ func (r *RedisFailoverHandler) getLabels(rf *redisfailoverv1.RedisFailover) map[ if len(rf.Spec.LabelWhitelist) != 0 { for labelKey, labelValue := range rf.Labels { for _, regex := range rf.Spec.LabelWhitelist { - compiledRegexp, err := regexp.Compile(regex) - if err != nil { - r.logger.Fatalf("Unable to compile regex: %s", regex) - } + compiledRegexp := regexp.MustCompile(regex) match := compiledRegexp.MatchString(labelKey) if match { filteredCustomLabels[labelKey]=labelValue From 51188f1ff0d00c6c9ad066cdc2586b33cc295506 Mon Sep 17 00:00:00 2001 From: Adam Harrison-Fuller Date: Thu, 24 Oct 2019 13:16:09 +0100 Subject: [PATCH 04/10] Remove logging --- operator/redisfailover/handler.go | 1 - 1 file changed, 1 deletion(-) diff --git a/operator/redisfailover/handler.go b/operator/redisfailover/handler.go index 896fa7041..380fe6a6d 100644 --- a/operator/redisfailover/handler.go +++ b/operator/redisfailover/handler.go @@ -121,7 +121,6 @@ func (r *RedisFailoverHandler) getLabels(rf *redisfailoverv1.RedisFailover) map[ filteredCustomLabels = rf.Labels } - r.logger.Infof("%#v", filteredCustomLabels) return util.MergeLabels(defaultLabels, dynLabels, filteredCustomLabels) } From b72871c017e53c54c76acb4203d70c885338264a Mon Sep 17 00:00:00 2001 From: Adam Harrison-Fuller Date: Thu, 24 Oct 2019 13:52:46 +0100 Subject: [PATCH 05/10] Combining assignment and if statement --- operator/redisfailover/handler.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/operator/redisfailover/handler.go b/operator/redisfailover/handler.go index 380fe6a6d..49783fffb 100644 --- a/operator/redisfailover/handler.go +++ b/operator/redisfailover/handler.go @@ -110,8 +110,7 @@ func (r *RedisFailoverHandler) getLabels(rf *redisfailoverv1.RedisFailover) map[ for labelKey, labelValue := range rf.Labels { for _, regex := range rf.Spec.LabelWhitelist { compiledRegexp := regexp.MustCompile(regex) - match := compiledRegexp.MatchString(labelKey) - if match { + if match := compiledRegexp.MatchString(labelKey); match { filteredCustomLabels[labelKey]=labelValue } } From 847b5abab169ccf467fe226db76def5b2b96ed86 Mon Sep 17 00:00:00 2001 From: Adam Harrison-Fuller Date: Thu, 24 Oct 2019 15:28:27 +0100 Subject: [PATCH 06/10] Dont die on failure to compile regex, add additional check --- operator/redisfailover/handler.go | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/operator/redisfailover/handler.go b/operator/redisfailover/handler.go index 49783fffb..e5a7798ff 100644 --- a/operator/redisfailover/handler.go +++ b/operator/redisfailover/handler.go @@ -106,12 +106,16 @@ func (r *RedisFailoverHandler) getLabels(rf *redisfailoverv1.RedisFailover) map[ // Filter the labels based on the whitelist filteredCustomLabels := make(map[string]string) - if len(rf.Spec.LabelWhitelist) != 0 { - for labelKey, labelValue := range rf.Labels { - for _, regex := range rf.Spec.LabelWhitelist { - compiledRegexp := regexp.MustCompile(regex) + if rf.Spec.LabelWhitelist != nil && len(rf.Spec.LabelWhitelist) != 0 { + for _, regex := range rf.Spec.LabelWhitelist { + compiledRegexp, err := regexp.Compile(regex) + if err != nil { + r.logger.Errorf("Unable to compile label whitelist regex '%s', ignoring it.", regex) + continue + } + for labelKey, labelValue := range rf.Labels { if match := compiledRegexp.MatchString(labelKey); match { - filteredCustomLabels[labelKey]=labelValue + filteredCustomLabels[labelKey] = labelValue } } } @@ -119,7 +123,6 @@ func (r *RedisFailoverHandler) getLabels(rf *redisfailoverv1.RedisFailover) map[ // If no whitelist is specified then dont filter label. filteredCustomLabels = rf.Labels } - return util.MergeLabels(defaultLabels, dynLabels, filteredCustomLabels) } From 057317136ed46c969246a175245e1ec310320945 Mon Sep 17 00:00:00 2001 From: Adam Harrison-Fuller Date: Thu, 24 Oct 2019 17:02:09 +0100 Subject: [PATCH 07/10] Improve documentation --- README.md | 16 ++++++++++ .../control-label-propagation.yaml | 29 +++++++++++++++++++ operator/redisfailover/handler.go | 2 +- 3 files changed, 46 insertions(+), 1 deletion(-) create mode 100644 example/redisfailover/control-label-propagation.yaml diff --git a/README.md b/README.md index e4477f13b..2753e7e2c 100644 --- a/README.md +++ b/README.md @@ -117,6 +117,22 @@ By default, no pod annotations will be applied to Redis nor Sentinel pods. In order to apply custom pod Annotations, you can provide the `podAnnotations` option inside redis/sentinel spec. An example can be found in the [custom annotations example file](example/redisfailover/custom-annotations.yaml). +### Control of label propagation. +By default the operator will propagate all labels on the CRD down to the resources that it creates. This can be problematic if the +labels on the CRD are not fully under your own control (for example: being deployed by a gitops operator) +as a change to a labels value can fail on immutable resources such as PodDisruptionBudgets. To control what labels the operator propagates +to resource is creates you can modify the labelWhitelist option in the spec. + +By default specifying no whitelist or an empty whitelist will cause all labels to still be copied as not to break backwards compatibility. + +Items in the array should be regular expressions, see [here](example/redisfailover/control-label-propagation.yaml) as an example of how they can be used and +[here](https://github.com/google/re2/wiki/Syntax) for a syntax reference. + +The whitelist can also be used as a form of blacklist by specifying a regular expression that will not match any label. + + + + ## Connection to the created Redis Failovers In order to connect to the redis-failover and use it, a [Sentinel-ready](https://redis.io/topics/sentinel-clients) library has to be used. This will connect through the Sentinel service to the Redis node working as a master. diff --git a/example/redisfailover/control-label-propagation.yaml b/example/redisfailover/control-label-propagation.yaml new file mode 100644 index 000000000..a9f67b12e --- /dev/null +++ b/example/redisfailover/control-label-propagation.yaml @@ -0,0 +1,29 @@ +apiVersion: databases.spotahome.com/v1 +kind: RedisFailover +metadata: + name: redisfailover2 + labels: + # These two labels will be propagated. + app.example.com/label1: value + app.example.com/label2: value + # This one wont be, as there is a non-empty whitelist and the regexp doesnt match it. + anotherlabel: value +spec: + sentinel: + replicas: 3 + resources: + requests: + cpu: 100m + limits: + memory: 100Mi + redis: + replicas: 3 + resources: + requests: + cpu: 100m + memory: 100Mi + limits: + cpu: 400m + memory: 500Mi + labelWhitelist: + - ^app.example.com.* diff --git a/operator/redisfailover/handler.go b/operator/redisfailover/handler.go index e5a7798ff..91d64ee27 100644 --- a/operator/redisfailover/handler.go +++ b/operator/redisfailover/handler.go @@ -120,7 +120,7 @@ func (r *RedisFailoverHandler) getLabels(rf *redisfailoverv1.RedisFailover) map[ } } } else { - // If no whitelist is specified then dont filter label. + // If no whitelist is specified then don't filter the labels. filteredCustomLabels = rf.Labels } return util.MergeLabels(defaultLabels, dynLabels, filteredCustomLabels) From 98b68608ef1eecc15650c5c1e4793b6d17068205 Mon Sep 17 00:00:00 2001 From: Adam Harrison-Fuller Date: Thu, 24 Oct 2019 17:23:33 +0100 Subject: [PATCH 08/10] README clarifications --- README.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/README.md b/README.md index 2753e7e2c..cc690b7e7 100644 --- a/README.md +++ b/README.md @@ -130,6 +130,14 @@ Items in the array should be regular expressions, see [here](example/redisfailov The whitelist can also be used as a form of blacklist by specifying a regular expression that will not match any label. +NOTE: The operator will always add the labels it requires for operation to resources. These are the following: +``` +app.kubernetes.io/component +app.kubernetes.io/managed-by +app.kubernetes.io/name +app.kubernetes.io/part-of +redisfailovers.databases.spotahome.com/name +``` From b071f4881c856949c738497b20a008e3220dc132 Mon Sep 17 00:00:00 2001 From: Adam Harrison-Fuller Date: Thu, 24 Oct 2019 17:36:33 +0100 Subject: [PATCH 09/10] Remove whitespace --- README.md | 2 -- 1 file changed, 2 deletions(-) diff --git a/README.md b/README.md index cc690b7e7..a5624c22e 100644 --- a/README.md +++ b/README.md @@ -139,8 +139,6 @@ app.kubernetes.io/part-of redisfailovers.databases.spotahome.com/name ``` - - ## Connection to the created Redis Failovers In order to connect to the redis-failover and use it, a [Sentinel-ready](https://redis.io/topics/sentinel-clients) library has to be used. This will connect through the Sentinel service to the Redis node working as a master. From 20c24815fe9a040614e063c1a0547d9b613fbbb3 Mon Sep 17 00:00:00 2001 From: Adam Harrison-Fuller Date: Tue, 3 Dec 2019 14:23:07 +0000 Subject: [PATCH 10/10] Regenerate generated code --- api/redisfailover/v1/zz_generated.deepcopy.go | 24 +++++++++++ mocks/service/redis/Client.go | 42 +++++++++---------- 2 files changed, 45 insertions(+), 21 deletions(-) diff --git a/api/redisfailover/v1/zz_generated.deepcopy.go b/api/redisfailover/v1/zz_generated.deepcopy.go index 9c82a8313..71a148637 100644 --- a/api/redisfailover/v1/zz_generated.deepcopy.go +++ b/api/redisfailover/v1/zz_generated.deepcopy.go @@ -175,6 +175,11 @@ func (in *RedisSettings) DeepCopyInto(out *RedisSettings) { (*in).DeepCopyInto(*out) } } + if in.ImagePullSecrets != nil { + in, out := &in.ImagePullSecrets, &out.ImagePullSecrets + *out = make([]core_v1.LocalObjectReference, len(*in)) + copy(*out, *in) + } if in.Tolerations != nil { in, out := &in.Tolerations, &out.Tolerations *out = make([]core_v1.Toleration, len(*in)) @@ -182,6 +187,13 @@ func (in *RedisSettings) DeepCopyInto(out *RedisSettings) { (*in)[i].DeepCopyInto(&(*out)[i]) } } + if in.NodeSelector != nil { + in, out := &in.NodeSelector, &out.NodeSelector + *out = make(map[string]string, len(*in)) + for key, val := range *in { + (*out)[key] = val + } + } if in.PodAnnotations != nil { in, out := &in.PodAnnotations, &out.PodAnnotations *out = make(map[string]string, len(*in)) @@ -268,6 +280,11 @@ func (in *SentinelSettings) DeepCopyInto(out *SentinelSettings) { (*in).DeepCopyInto(*out) } } + if in.ImagePullSecrets != nil { + in, out := &in.ImagePullSecrets, &out.ImagePullSecrets + *out = make([]core_v1.LocalObjectReference, len(*in)) + copy(*out, *in) + } if in.Tolerations != nil { in, out := &in.Tolerations, &out.Tolerations *out = make([]core_v1.Toleration, len(*in)) @@ -275,6 +292,13 @@ func (in *SentinelSettings) DeepCopyInto(out *SentinelSettings) { (*in)[i].DeepCopyInto(&(*out)[i]) } } + if in.NodeSelector != nil { + in, out := &in.NodeSelector, &out.NodeSelector + *out = make(map[string]string, len(*in)) + for key, val := range *in { + (*out)[key] = val + } + } if in.PodAnnotations != nil { in, out := &in.PodAnnotations, &out.PodAnnotations *out = make(map[string]string, len(*in)) diff --git a/mocks/service/redis/Client.go b/mocks/service/redis/Client.go index 582761e7e..9b26c5f9c 100644 --- a/mocks/service/redis/Client.go +++ b/mocks/service/redis/Client.go @@ -114,27 +114,6 @@ func (_m *Client) IsMaster(ip string, password string) (bool, error) { return r0, r1 } -// SlaveIsReady provides a mock function with given fields: ip, password -func (_m *Client) SlaveIsReady(ip string, password string) (bool, error) { - ret := _m.Called(ip, password) - - var r0 bool - if rf, ok := ret.Get(0).(func(string, string) bool); ok { - r0 = rf(ip, password) - } else { - r0 = ret.Get(0).(bool) - } - - var r1 error - if rf, ok := ret.Get(1).(func(string, string) error); ok { - r1 = rf(ip, password) - } else { - r1 = ret.Error(1) - } - - return r0, r1 -} - // MakeMaster provides a mock function with given fields: ip, password func (_m *Client) MakeMaster(ip string, password string) error { ret := _m.Called(ip, password) @@ -218,3 +197,24 @@ func (_m *Client) SetCustomSentinelConfig(ip string, configs []string) error { return r0 } + +// SlaveIsReady provides a mock function with given fields: ip, password +func (_m *Client) SlaveIsReady(ip string, password string) (bool, error) { + ret := _m.Called(ip, password) + + var r0 bool + if rf, ok := ret.Get(0).(func(string, string) bool); ok { + r0 = rf(ip, password) + } else { + r0 = ret.Get(0).(bool) + } + + var r1 error + if rf, ok := ret.Get(1).(func(string, string) error); ok { + r1 = rf(ip, password) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +}