From c48c1b96b9abd6599969a632a2da67332659f10c Mon Sep 17 00:00:00 2001 From: Julio Chana Date: Tue, 2 Jan 2018 17:24:41 +0100 Subject: [PATCH 1/3] [DEVOPS-650] On Update, wait for other resources to be ready in stead of sames --- pkg/failover/client.go | 10 ++- pkg/failover/client_test.go | 128 ++++++++++++++++++++++++++++++++++++ 2 files changed, 132 insertions(+), 6 deletions(-) diff --git a/pkg/failover/client.go b/pkg/failover/client.go index 539528704..00c33bb66 100644 --- a/pkg/failover/client.go +++ b/pkg/failover/client.go @@ -739,7 +739,6 @@ func (r *RedisFailoverKubeClient) createPodDisruptionBudget(rf *RedisFailover, n // UpdateSentinelDeployment updates the spec of the existing sentinel deployment func (r *RedisFailoverKubeClient) UpdateSentinelDeployment(rf *RedisFailover) error { - name := r.GetSentinelName(rf) namespace := rf.Metadata.Namespace logger := r.logger.WithField(logNameField, rf.Metadata.Name).WithField(logNamespaceField, rf.Metadata.Namespace) @@ -766,11 +765,11 @@ func (r *RedisFailoverKubeClient) UpdateSentinelDeployment(rf *RedisFailover) er oldSD.Spec.Template.Spec.Containers[0].Image = getRedisImage(rf) oldSD.Spec.Template.Spec.Containers[0].Resources = getSentinelResources(rf.Spec) - if _, err := r.Client.AppsV1beta1().Deployments(namespace).Update(oldSD); err != nil { + if err := r.waitForStatefulset(r.GetRedisName(rf), namespace, rf.Spec.Redis.Replicas, logger); err != nil { return err } - if err := r.waitForDeployment(name, namespace, replicas, logger); err != nil { + if _, err := r.Client.AppsV1beta1().Deployments(namespace).Update(oldSD); err != nil { return err } @@ -779,7 +778,6 @@ func (r *RedisFailoverKubeClient) UpdateSentinelDeployment(rf *RedisFailover) er // UpdateRedisStatefulset updates the spec of the existing redis statefulset func (r *RedisFailoverKubeClient) UpdateRedisStatefulset(rf *RedisFailover) error { - name := r.GetRedisName(rf) namespace := rf.Metadata.Namespace logger := r.logger.WithField(logNameField, rf.Metadata.Name).WithField(logNamespaceField, rf.Metadata.Namespace) @@ -805,11 +803,11 @@ func (r *RedisFailoverKubeClient) UpdateRedisStatefulset(rf *RedisFailover) erro } } - if _, err := r.Client.AppsV1beta1().StatefulSets(namespace).Update(oldSS); err != nil { + if err := r.waitForDeployment(r.GetSentinelName(rf), namespace, rf.Spec.Sentinel.Replicas, logger); err != nil { return err } - if err := r.waitForStatefulset(name, namespace, replicas, logger); err != nil { + if _, err := r.Client.AppsV1beta1().StatefulSets(namespace).Update(oldSS); err != nil { return err } diff --git a/pkg/failover/client_test.go b/pkg/failover/client_test.go index c41ba3cca..a9a27b30e 100644 --- a/pkg/failover/client_test.go +++ b/pkg/failover/client_test.go @@ -1690,9 +1690,29 @@ func TestUpdateSentinelDeploymentError(t *testing.T) { return true, deployment, nil }) + statefulsetSize := int32(3) + // Add a reactor when calling pods + client.Fake.AddReactor("get", "statefulsets", func(action k8stesting.Action) (bool, runtime.Object, error) { + // Create the statefulset to be returned with Replicas = 3 + statefulset := &v1beta1.StatefulSet{ + ObjectMeta: metav1.ObjectMeta{ + Name: redisName, + Namespace: namespace, + }, + Status: v1beta1.StatefulSetStatus{ + ReadyReplicas: statefulsetSize, + }, + } + + // Return the statefulset as if we where the API responding to GET statefulsets + return true, statefulset, nil + }) + mc := &mocks.Clock{} mc.On("NewTicker", mock.Anything). Once().Return(time.NewTicker(1)) + mc.On("After", mock.Anything). + Once().Return(time.After(time.Hour)) r := failover.NewRedisFailoverKubeClient(client, mc, log.Nil) redisFailover := &failover.RedisFailover{ @@ -1757,6 +1777,22 @@ func TestUpdateSentinelDeploymentTimeoutError(t *testing.T) { return true, deployment, nil }) + client.Fake.AddReactor("get", "statefulsets", func(action k8stesting.Action) (bool, runtime.Object, error) { + // Create the statefulset to be returned with Replicas = 3 + statefulset := &v1beta1.StatefulSet{ + ObjectMeta: metav1.ObjectMeta{ + Name: redisName, + Namespace: namespace, + }, + Status: v1beta1.StatefulSetStatus{ + ReadyReplicas: int32(3), + }, + } + + // Return the statefulset as if we where the API responding to GET statefulsets + return true, statefulset, nil + }) + mc := &mocks.Clock{} mc.On("NewTicker", mock.Anything). Once().Return(time.NewTicker(time.Hour)) @@ -1849,6 +1885,24 @@ func TestUpdateSentinelDeployment(t *testing.T) { return true, deployment, nil }) + statefulsetSize := int32(3) + // Add a reactor when calling pods + client.Fake.AddReactor("get", "statefulsets", func(action k8stesting.Action) (bool, runtime.Object, error) { + // Create the statefulset to be returned with Replicas = 3 + statefulset := &v1beta1.StatefulSet{ + ObjectMeta: metav1.ObjectMeta{ + Name: redisName, + Namespace: namespace, + }, + Status: v1beta1.StatefulSetStatus{ + ReadyReplicas: statefulsetSize, + }, + } + + // Return the statefulset as if we where the API responding to GET statefulsets + return true, statefulset, nil + }) + mc := &mocks.Clock{} mc.On("NewTicker", mock.Anything). Once().Return(time.NewTicker(1)) @@ -1949,9 +2003,27 @@ func TestUpdateRedisStatefulsetError(t *testing.T) { return true, nil, errors.New("") }) + replicas := int32(3) + + client.Fake.AddReactor("get", "deployments", func(action k8stesting.Action) (bool, runtime.Object, error) { + deployment := &v1beta1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: sentinelName, + Namespace: namespace, + }, + Status: v1beta1.DeploymentStatus{ + ReadyReplicas: replicas, + UpdatedReplicas: replicas, + }, + } + return true, deployment, nil + }) + mc := &mocks.Clock{} mc.On("NewTicker", mock.Anything). Once().Return(time.NewTicker(1)) + mc.On("After", mock.Anything). + Once().Return(time.After(time.Hour)) r := failover.NewRedisFailoverKubeClient(client, mc, log.Nil) redisFailover := &failover.RedisFailover{ @@ -2055,6 +2127,20 @@ func TestUpdateRedisStatefulsetWithUpdate(t *testing.T) { return true, nil, nil }) + client.Fake.AddReactor("get", "deployments", func(action k8stesting.Action) (bool, runtime.Object, error) { + deployment := &v1beta1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: sentinelName, + Namespace: namespace, + }, + Status: v1beta1.DeploymentStatus{ + ReadyReplicas: replicas, + UpdatedReplicas: replicas, + }, + } + return true, deployment, nil + }) + mc := &mocks.Clock{} mc.On("NewTicker", mock.Anything). Once().Return(time.NewTicker(1)) @@ -2179,6 +2265,20 @@ func TestUpdateRedisStatefulsetWithoutUpdate(t *testing.T) { return true, nil, nil }) + client.Fake.AddReactor("get", "deployments", func(action k8stesting.Action) (bool, runtime.Object, error) { + deployment := &v1beta1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: sentinelName, + Namespace: namespace, + }, + Status: v1beta1.DeploymentStatus{ + ReadyReplicas: replicas, + UpdatedReplicas: replicas, + }, + } + return true, deployment, nil + }) + mc := &mocks.Clock{} mc.On("NewTicker", mock.Anything). Once().Return(time.NewTicker(1)) @@ -2277,6 +2377,20 @@ func TestUpdateRedisStatefulsetTimeoutError(t *testing.T) { return true, nil, nil }) + client.Fake.AddReactor("get", "deployments", func(action k8stesting.Action) (bool, runtime.Object, error) { + deployment := &v1beta1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: sentinelName, + Namespace: namespace, + }, + Status: v1beta1.DeploymentStatus{ + ReadyReplicas: replicas, + UpdatedReplicas: replicas, + }, + } + return true, deployment, nil + }) + mc := &mocks.Clock{} mc.On("NewTicker", mock.Anything). Once().Return(time.NewTicker(time.Hour)) @@ -2385,6 +2499,20 @@ func TestUpdateRedisStatefulset(t *testing.T) { return true, nil, nil }) + client.Fake.AddReactor("get", "deployments", func(action k8stesting.Action) (bool, runtime.Object, error) { + deployment := &v1beta1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: sentinelName, + Namespace: namespace, + }, + Status: v1beta1.DeploymentStatus{ + ReadyReplicas: replicas, + UpdatedReplicas: replicas, + }, + } + return true, deployment, nil + }) + mc := &mocks.Clock{} mc.On("NewTicker", mock.Anything). Once().Return(time.NewTicker(1)) From 8fbd547e31522e05e1dc2f21d3b750c17214da5c Mon Sep 17 00:00:00 2001 From: Julio Chana Date: Tue, 2 Jan 2018 17:32:24 +0100 Subject: [PATCH 2/3] [DEVOPS-650] Bump version to 0.1.5 --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 2ef77ada9..af766bfd6 100644 --- a/Makefile +++ b/Makefile @@ -1,6 +1,6 @@ # The following are targers that do not exist in the filesystem as real files and should be always executed by make .PHONY: default build deps-development docker-build shell run image unit-test test generate go-generate get-deps update-deps -VERSION := 0.1.4 +VERSION := 0.1.5 # Name of this service/application SERVICE_NAME := redis-operator From 56622f7ade6283e18434d1c01ea862729e23d628 Mon Sep 17 00:00:00 2001 From: Julio Chana Date: Tue, 2 Jan 2018 18:08:15 +0100 Subject: [PATCH 3/3] [DEVOPS-650] Fix bug. Only add redis-exporter node if not exists --- pkg/failover/client.go | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/pkg/failover/client.go b/pkg/failover/client.go index 00c33bb66..a705854b8 100644 --- a/pkg/failover/client.go +++ b/pkg/failover/client.go @@ -793,8 +793,16 @@ func (r *RedisFailoverKubeClient) UpdateRedisStatefulset(rf *RedisFailover) erro oldSS.Spec.Template.Spec.Containers[0].Image = getRedisImage(rf) if rf.Spec.Redis.Exporter { - exporter := createRedisExporterContainer() - oldSS.Spec.Template.Spec.Containers = append(oldSS.Spec.Template.Spec.Containers, exporter) + found := false + for _, container := range oldSS.Spec.Template.Spec.Containers { + if container.Name == exporterContainerName { + found = true + } + } + if !found { + exporter := createRedisExporterContainer() + oldSS.Spec.Template.Spec.Containers = append(oldSS.Spec.Template.Spec.Containers, exporter) + } } else { for pos, container := range oldSS.Spec.Template.Spec.Containers { if container.Name == exporterContainerName {