Skip to content

Commit

Permalink
Make command for redis and sentinel configurable
Browse files Browse the repository at this point in the history 
Signed-off-by: Julio Chana <[email protected]>
  • Loading branch information
Julio Chana committed Mar 5, 2019
1 parent a970bf6 commit 37d9aff
Show file tree
Hide file tree
Showing 3 changed files with 130 additions and 9 deletions.
2 changes: 2 additions & 0 deletions api/redisfailover/v1alpha2/types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ type RedisSettings struct {
Image string `json:"image,omitempty"`
Version string `json:"version,omitempty"`
CustomConfig []string `json:"customConfig,omitempty"`
Command []string `json:"command,omitempty"`
ShutdownConfigMap string `json:"shutdownConfigMap,omitempty"`
Storage RedisStorage `json:"storage,omitempty"`
}
Expand All @@ -56,6 +57,7 @@ type SentinelSettings struct {
Replicas int32 `json:"replicas,omitempty"`
Resources RedisFailoverResources `json:"resources,omitempty"`
CustomConfig []string `json:"customConfig,omitempty"`
Command []string `json:"command,omitempty"`
}

// RedisFailoverResources sets the limits and requests for a container
Expand Down
34 changes: 25 additions & 9 deletions operator/redisfailover/service/generator.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,7 @@ func generateRedisStatefulSet(rf *redisfailoverv1alpha2.RedisFailover, labels ma

spec := rf.Spec
redisImage := getRedisImage(rf)
redisCommand := getRedisCommand(rf)
resources := getRedisResources(spec)
labels = util.MergeLabels(labels, generateLabels(redisRoleName, rf.Name))
volumeMounts := getRedisVolumeMounts(rf)
Expand Down Expand Up @@ -203,10 +204,7 @@ func generateRedisStatefulSet(rf *redisfailoverv1alpha2.RedisFailover, labels ma
},
},
VolumeMounts: volumeMounts,
Command: []string{
"redis-server",
fmt.Sprintf("/redis/%s", redisConfigFileName),
},
Command: redisCommand,
ReadinessProbe: &corev1.Probe{
InitialDelaySeconds: graceTime,
TimeoutSeconds: 5,
Expand Down Expand Up @@ -274,6 +272,7 @@ func generateSentinelDeployment(rf *redisfailoverv1alpha2.RedisFailover, labels

spec := rf.Spec
redisImage := getRedisImage(rf)
sentinelCommand := getSentinelCommand(rf)
resources := getSentinelResources(spec)
labels = util.MergeLabels(labels, generateLabels(sentinelRoleName, rf.Name))

Expand Down Expand Up @@ -349,11 +348,7 @@ func generateSentinelDeployment(rf *redisfailoverv1alpha2.RedisFailover, labels
MountPath: "/redis",
},
},
Command: []string{
"redis-server",
fmt.Sprintf("/redis/%s", sentinelConfigFileName),
"--sentinel",
},
Command: sentinelCommand,
ReadinessProbe: &corev1.Probe{
InitialDelaySeconds: graceTime,
TimeoutSeconds: 5,
Expand Down Expand Up @@ -653,3 +648,24 @@ func getRedisDataVolumeName(rf *redisfailoverv1alpha2.RedisFailover) string {
return redisStorageVolumeName
}
}

func getRedisCommand(rf *redisfailoverv1alpha2.RedisFailover) []string {
if len(rf.Spec.Redis.Command) > 0 {
return rf.Spec.Redis.Command
}
return []string{
"redis-server",
fmt.Sprintf("/redis/%s", redisConfigFileName),
}
}

func getSentinelCommand(rf *redisfailoverv1alpha2.RedisFailover) []string {
if len(rf.Spec.Sentinel.Command) > 0 {
return rf.Spec.Sentinel.Command
}
return []string{
"redis-server",
fmt.Sprintf("/redis/%s", sentinelConfigFileName),
"--sentinel",
}
}
103 changes: 103 additions & 0 deletions operator/redisfailover/service/generator_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -452,3 +452,106 @@ func TestRedisStatefulSetStorageGeneration(t *testing.T) {
assert.NoError(err)
}
}

func TestRedisStatefulSetCommands(t *testing.T) {
tests := []struct {
name string
givenCommands []string
expectedCommands []string
}{
{
name: "Default values",
givenCommands: []string{},
expectedCommands: []string{
"redis-server",
"/redis/redis.conf",
},
},
{
name: "Given commands should be used in redis container",
givenCommands: []string{
"test",
"command",
},
expectedCommands: []string{
"test",
"command",
},
},
}

for _, test := range tests {
assert := assert.New(t)

// Generate a default RedisFailover and attaching the required storage
rf := generateRF()
rf.Spec.Redis.Command = test.givenCommands

gotCommands := []string{}

ms := &mK8SService.Services{}
ms.On("CreateOrUpdatePodDisruptionBudget", namespace, mock.Anything).Once().Return(nil, nil)
ms.On("CreateOrUpdateStatefulSet", namespace, mock.Anything).Once().Run(func(args mock.Arguments) {
ss := args.Get(1).(*appsv1beta2.StatefulSet)
gotCommands = ss.Spec.Template.Spec.Containers[0].Command
}).Return(nil)

client := rfservice.NewRedisFailoverKubeClient(ms, log.Dummy)
err := client.EnsureRedisStatefulset(rf, nil, []metav1.OwnerReference{})

assert.Equal(test.expectedCommands, gotCommands)
assert.NoError(err)
}
}

func TestSentinelDeploymentCommands(t *testing.T) {
tests := []struct {
name string
givenCommands []string
expectedCommands []string
}{
{
name: "Default values",
givenCommands: []string{},
expectedCommands: []string{
"redis-server",
"/redis/sentinel.conf",
"--sentinel",
},
},
{
name: "Given commands should be used in sentinel container",
givenCommands: []string{
"test",
"command",
},
expectedCommands: []string{
"test",
"command",
},
},
}

for _, test := range tests {
assert := assert.New(t)

// Generate a default RedisFailover and attaching the required storage
rf := generateRF()
rf.Spec.Sentinel.Command = test.givenCommands

gotCommands := []string{}

ms := &mK8SService.Services{}
ms.On("CreateOrUpdatePodDisruptionBudget", namespace, mock.Anything).Once().Return(nil, nil)
ms.On("CreateOrUpdateDeployment", namespace, mock.Anything).Once().Run(func(args mock.Arguments) {
d := args.Get(1).(*appsv1beta2.Deployment)
gotCommands = d.Spec.Template.Spec.Containers[0].Command
}).Return(nil)

client := rfservice.NewRedisFailoverKubeClient(ms, log.Dummy)
err := client.EnsureSentinelDeployment(rf, nil, []metav1.OwnerReference{})

assert.Equal(test.expectedCommands, gotCommands)
assert.NoError(err)
}
}

3 comments on commit 37d9aff

@marcemq
Copy link
Contributor

@marcemq marcemq commented on 37d9aff Mar 5, 2019

Choose a reason for hiding this comment

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

Since these commands are going to be used when launching the redis and sentinel instances I suggest to rename the introduced variables and functions to:

  • Command => InitCommand
  • getRedisCommand => getRedisInitCommand
  • getSentinelCommand => getSentinelInitCommand

@jchanam
Copy link
Collaborator

@jchanam jchanam commented on 37d9aff Mar 6, 2019

Choose a reason for hiding this comment

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

The name maintains the one defined by Kubernetes. Command means the command to be run when starting the container.

There are a few things why I think InitCommand don't fit:

  • There should be only one process running in each container (to manage it's livecycle). If we follow this premise, the command is the only command that will be run (so it does not make sense to call it initCommand, Which command will be after that one?).
  • InitCommand can be interpreted as a previous command to be run, before running redis/sentinel. Also. it could be interpreted as the command for an InitContainer.

In general, I think that naming could cause confusion and I prefer to keep it as simple as possible and as similar to k8s as possible.

@marcemq
Copy link
Contributor

@marcemq marcemq commented on 37d9aff Mar 6, 2019

Choose a reason for hiding this comment

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

Thanks for the informative reply.
And now, I agree with you.

Please sign in to comment.