Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not try to update the ServiceName in existing StatefulSets #105

Merged
merged 3 commits into from
May 26, 2021
+138 −98
Merged

Do not try to update the ServiceName in existing StatefulSets #105

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c7ba129
Do not try to update the ServiceName in existing StatefulSets
burmanm May 26, 2021
55a4a44
Fix existing unit tests
burmanm May 26, 2021
7b3b30d
Add small test to verify the ServiceName is not modified by the newSt…
burmanm May 26, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 13 additions & 23 deletions operator/pkg/reconciliation/construct_statefulset.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ package reconciliation

import (
"fmt"

api "github.com/k8ssandra/cass-operator/operator/pkg/apis/cassandra/v1beta1"
"github.com/k8ssandra/cass-operator/operator/pkg/httphelper"
"github.com/k8ssandra/cass-operator/operator/pkg/images"
Expand Down Expand Up @@ -49,25 +50,6 @@ func newNamespacedNameForStatefulSet(
}
}

// We have to account for the fact that they might use the old managed-by label value
// (oplabels.ManagedByLabelDefunctValue) for CassandraDatacenters originally
// created in version 1.1.0 or earlier.
func newStatefulSetForCassandraDatacenterWithDefunctPvcManagedBy(
rackName string,
dc *api.CassandraDatacenter,
replicaCount int) (*appsv1.StatefulSet, error) {

return newStatefulSetForCassandraDatacenterHelper(rackName, dc, replicaCount, true)
}

func newStatefulSetForCassandraDatacenter(
rackName string,
dc *api.CassandraDatacenter,
replicaCount int) (*appsv1.StatefulSet, error) {

return newStatefulSetForCassandraDatacenterHelper(rackName, dc, replicaCount, false)
}

// Check if we need to define a SecurityContext.
// If the user defines the DockerImageRunsAsCassandra field, we trust that.
// Otherwise if ServerType is "dse", the answer is true.
Expand All @@ -93,12 +75,12 @@ func rackNodeAffinitylabels(dc *api.CassandraDatacenter, rackName string) (map[s
if rack.Zone != "" {
if _, found := nodeAffinityLabels[zoneLabel]; found {
log.Error(nil,
"Deprecated parameter Zone is used and also defined in NodeAffinityLabels. " +
"You should only define it in NodeAffinityLabels")
"Deprecated parameter Zone is used and also defined in NodeAffinityLabels. "+
"You should only define it in NodeAffinityLabels")
}
nodeAffinityLabels = utils.MergeMap(
emptyMapIfNil(nodeAffinityLabels), map[string]string{zoneLabel: rack.Zone},
)
)
}
break
}
Expand All @@ -107,7 +89,11 @@ func rackNodeAffinitylabels(dc *api.CassandraDatacenter, rackName string) (map[s
}

// Create a statefulset object for the Datacenter.
func newStatefulSetForCassandraDatacenterHelper(
// We have to account for the fact that they might use the old managed-by label value
// (oplabels.ManagedByLabelDefunctValue) for CassandraDatacenters originally
// created in version 1.1.0 or earlier. Set useDefunctManagedByForPvc to true to use old ones.
func newStatefulSetForCassandraDatacenter(
sts *appsv1.StatefulSet,
rackName string,
dc *api.CassandraDatacenter,
replicaCount int,
Expand Down Expand Up @@ -195,6 +181,10 @@ func newStatefulSetForCassandraDatacenterHelper(
}
result.Annotations = map[string]string{}

if sts != nil && sts.Spec.ServiceName != result.Spec.ServiceName {
result.Spec.ServiceName = sts.Spec.ServiceName
}

if utils.IsPSPEnabled() {
result = psp.AddStatefulSetChanges(dc, result)
}
Expand Down
96 changes: 48 additions & 48 deletions operator/pkg/reconciliation/construct_statefulset_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ func Test_newStatefulSetForCassandraDatacenter(t *testing.T) {
}
for _, tt := range tests {
t.Log(tt.name)
got, err := newStatefulSetForCassandraDatacenter(tt.args.rackName, tt.args.dc, tt.args.replicaCount)
got, err := newStatefulSetForCassandraDatacenter(nil, tt.args.rackName, tt.args.dc, tt.args.replicaCount, false)
assert.NoError(t, err, "newStatefulSetForCassandraDatacenter should not have errored")
assert.NotNil(t, got, "newStatefulSetForCassandraDatacenter should not have returned a nil statefulset")
assert.Equal(t, map[string]string{"dedicated": "cassandra"}, got.Spec.Template.Spec.NodeSelector)
Expand All @@ -54,15 +54,15 @@ func Test_newStatefulSetForCassandraDatacenter(t *testing.T) {
func Test_newStatefulSetForCassandraDatacenter_rackNodeAffinitylabels(t *testing.T) {
dc := &api.CassandraDatacenter{
Spec: api.CassandraDatacenterSpec{
ClusterName: "bob",
ServerType: "cassandra",
ServerVersion: "3.11.7",
PodTemplateSpec: &corev1.PodTemplateSpec{},
ClusterName: "bob",
ServerType: "cassandra",
ServerVersion: "3.11.7",
PodTemplateSpec: &corev1.PodTemplateSpec{},
NodeAffinityLabels: map[string]string{"dclabel1": "dcvalue1", "dclabel2": "dcvalue2"},
Racks: []api.Rack{
{
Name: "rack1",
Zone: "z1",
Name: "rack1",
Zone: "z1",
NodeAffinityLabels: map[string]string{"r1label1": "r1value1", "r1label2": "r1value2"},
},
},
Expand All @@ -76,7 +76,7 @@ func Test_newStatefulSetForCassandraDatacenter_rackNodeAffinitylabels(t *testing
assert.NoError(t, nodeAffinityLabelsConfigurationError,
"should not have gotten error when getting NodeAffinitylabels of rack rack1")

expected := map[string]string {
expected := map[string]string{
"dclabel1": "dcvalue1",
"dclabel2": "dcvalue2",
"r1label1": "r1value1",
Expand Down Expand Up @@ -108,16 +108,16 @@ func Test_newStatefulSetForCassandraDatacenterWithAdditionalVolumes(t *testing.T
replicaCount: 1,
dc: &api.CassandraDatacenter{
Spec: api.CassandraDatacenterSpec{
ClusterName: "c1",
ClusterName: "c1",
PodTemplateSpec: &corev1.PodTemplateSpec{
Spec: corev1.PodSpec {
InitContainers: []corev1.Container {
Spec: corev1.PodSpec{
InitContainers: []corev1.Container{
{
Name: "initContainer1",
Name: "initContainer1",
Image: "initImage1",
VolumeMounts: []corev1.VolumeMount{
{
Name: "server-logs",
Name: "server-logs",
MountPath: "/var/log/cassandra",
},
},
Expand All @@ -130,16 +130,16 @@ func Test_newStatefulSetForCassandraDatacenterWithAdditionalVolumes(t *testing.T
StorageClassName: &customCassandraDataStorageClass,
},
AdditionalVolumes: api.AdditionalVolumesSlice{
api.AdditionalVolumes {
api.AdditionalVolumes{
MountPath: "/var/log/cassandra",
Name: "server-logs",
Name: "server-logs",
PVCSpec: corev1.PersistentVolumeClaimSpec{
StorageClassName: &customCassandraServerLogsStorageClass,
},
},
api.AdditionalVolumes{
MountPath: "/var/lib/cassandra/commitlog",
Name: "cassandra-commitlogs",
Name: "cassandra-commitlogs",
PVCSpec: corev1.PersistentVolumeClaimSpec{
StorageClassName: &customCassandraCommitLogsStorageClass,
},
Expand All @@ -155,7 +155,7 @@ func Test_newStatefulSetForCassandraDatacenterWithAdditionalVolumes(t *testing.T
}
for _, tt := range tests {
t.Log(tt.name)
got, err := newStatefulSetForCassandraDatacenter(tt.args.rackName, tt.args.dc, tt.args.replicaCount)
got, err := newStatefulSetForCassandraDatacenter(nil, tt.args.rackName, tt.args.dc, tt.args.replicaCount, false)
assert.NoError(t, err, "newStatefulSetForCassandraDatacenter should not have errored")
assert.NotNil(t, got, "newStatefulSetForCassandraDatacenter should not have returned a nil statefulset")

Expand Down Expand Up @@ -214,20 +214,20 @@ func Test_newStatefulSetForCassandraPodSecurityContext(t *testing.T) {
FSGroup: int64Ptr(999),
}

tests := []struct{
name string
dc *api.CassandraDatacenter
tests := []struct {
name string
dc *api.CassandraDatacenter
expected *corev1.PodSecurityContext
} {
}{
{
name: "run cassandra as non-root user",
dc: &api.CassandraDatacenter{
Spec: api.CassandraDatacenterSpec{
ClusterName: clusterName,
ServerType: "cassandra",
ServerVersion: "3.11.10",
ClusterName: clusterName,
ServerType: "cassandra",
ServerVersion: "3.11.10",
DockerImageRunsAsCassandra: boolPtr(true),
StorageConfig: storageConfig,
StorageConfig: storageConfig,
},
},
expected: defaultSecurityContext,
Expand All @@ -236,11 +236,11 @@ func Test_newStatefulSetForCassandraPodSecurityContext(t *testing.T) {
name: "run cassandra as root user",
dc: &api.CassandraDatacenter{
Spec: api.CassandraDatacenterSpec{
ClusterName: clusterName,
ServerType: "cassandra",
ServerVersion: "3.11.7",
ClusterName: clusterName,
ServerType: "cassandra",
ServerVersion: "3.11.7",
DockerImageRunsAsCassandra: boolPtr(false),
StorageConfig: storageConfig,
StorageConfig: storageConfig,
},
},
expected: nil,
Expand All @@ -250,8 +250,8 @@ func Test_newStatefulSetForCassandraPodSecurityContext(t *testing.T) {
name: "run dse as non-root user",
dc: &api.CassandraDatacenter{
Spec: api.CassandraDatacenterSpec{
ClusterName: clusterName,
ServerType: "dse",
ClusterName: clusterName,
ServerType: "dse",
ServerVersion: "6.8.7",
StorageConfig: storageConfig,
},
Expand All @@ -262,58 +262,58 @@ func Test_newStatefulSetForCassandraPodSecurityContext(t *testing.T) {
name: "run cassandra with pod security context override",
dc: &api.CassandraDatacenter{
Spec: api.CassandraDatacenterSpec{
ClusterName: clusterName,
ServerType: "cassandra",
ClusterName: clusterName,
ServerType: "cassandra",
ServerVersion: "3.11.10",
StorageConfig: storageConfig,
PodTemplateSpec: &corev1.PodTemplateSpec{
Spec: corev1.PodSpec{
SecurityContext: &corev1.PodSecurityContext{
RunAsUser: int64Ptr(12345),
RunAsUser: int64Ptr(12345),
RunAsGroup: int64Ptr(54321),
FSGroup: int64Ptr(11111),
FSGroup: int64Ptr(11111),
},
},
},
},
},
expected: &corev1.PodSecurityContext{
RunAsUser: int64Ptr(12345),
RunAsUser: int64Ptr(12345),
RunAsGroup: int64Ptr(54321),
FSGroup: int64Ptr(11111),
FSGroup: int64Ptr(11111),
},
},
{
name: "run dse with pod security context override",
dc: &api.CassandraDatacenter{
Spec: api.CassandraDatacenterSpec{
ClusterName: clusterName,
ServerType: "dse",
ClusterName: clusterName,
ServerType: "dse",
ServerVersion: "6.8.7",
StorageConfig: storageConfig,
PodTemplateSpec: &corev1.PodTemplateSpec{
Spec: corev1.PodSpec{
SecurityContext: &corev1.PodSecurityContext{
RunAsUser: int64Ptr(12345),
RunAsUser: int64Ptr(12345),
RunAsGroup: int64Ptr(54321),
FSGroup: int64Ptr(11111),
FSGroup: int64Ptr(11111),
},
},
},
},
},
expected: &corev1.PodSecurityContext{
RunAsUser: int64Ptr(12345),
RunAsUser: int64Ptr(12345),
RunAsGroup: int64Ptr(54321),
FSGroup: int64Ptr(11111),
FSGroup: int64Ptr(11111),
},
},
{
name: "run cassandra with empty pod security context override",
dc: &api.CassandraDatacenter{
Spec: api.CassandraDatacenterSpec{
ClusterName: clusterName,
ServerType: "cassandra",
ClusterName: clusterName,
ServerType: "cassandra",
ServerVersion: "3.11.10",
StorageConfig: storageConfig,
PodTemplateSpec: &corev1.PodTemplateSpec{
Expand All @@ -329,8 +329,8 @@ func Test_newStatefulSetForCassandraPodSecurityContext(t *testing.T) {
name: "run dse with empty pod security context override",
dc: &api.CassandraDatacenter{
Spec: api.CassandraDatacenterSpec{
ClusterName: clusterName,
ServerType: "dse",
ClusterName: clusterName,
ServerType: "dse",
ServerVersion: "6.8.7",
StorageConfig: storageConfig,
PodTemplateSpec: &corev1.PodTemplateSpec{
Expand All @@ -345,7 +345,7 @@ func Test_newStatefulSetForCassandraPodSecurityContext(t *testing.T) {
}
for _, tt := range tests {
t.Log(tt.name)
statefulSet, err := newStatefulSetForCassandraDatacenter(rack, tt.dc, replicas)
statefulSet, err := newStatefulSetForCassandraDatacenter(nil, rack, tt.dc, replicas, false)
assert.NoError(t, err, fmt.Sprintf("%s: failed to create new statefulset", tt.name))
assert.NotNil(t, statefulSet, fmt.Sprintf("%s: statefulset is nil", tt.name))

Expand Down
21 changes: 8 additions & 13 deletions operator/pkg/reconciliation/reconcile_racks.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,13 +174,7 @@ func (rc *ReconciliationContext) desiredStatefulSetForExistingStatefulSet(sts *a
// StatefulSet. Consequently, we must preserve the old labels in this case.
usesDefunct := usesDefunctPvcManagedByLabel(sts)

if usesDefunct {
desiredSts, err = newStatefulSetForCassandraDatacenterWithDefunctPvcManagedBy(rackName, dc, replicas)
} else {
desiredSts, err = newStatefulSetForCassandraDatacenter(rackName, dc, replicas)
}

return
return newStatefulSetForCassandraDatacenter(sts, rackName, dc, replicas, usesDefunct)
}

func (rc *ReconciliationContext) CheckRackPodTemplate() result.ReconcileResult {
Expand Down Expand Up @@ -342,7 +336,7 @@ func (rc *ReconciliationContext) CheckRackForceUpgrade() result.ReconcileResult

// have to use zero here, because each statefulset is created with no replicas
// in GetStatefulSetForRack()
desiredSts, err := newStatefulSetForCassandraDatacenter(rackName, dc, 0)
desiredSts, err := newStatefulSetForCassandraDatacenter(statefulSet, rackName, dc, 0, false)
if err != nil {
logger.Error(err, "error calling newStatefulSetForCassandraDatacenter")
return result.Error(err)
Expand Down Expand Up @@ -648,7 +642,7 @@ func hasPodPotentiallyBootstrapped(pod *corev1.Pod, nodeStatuses api.CassandraSt
// In effect, we want to know if 'nodetool status' would indicate the relevant cassandra node
// is part of the cluster

// Case 1: If we have a host ID for the pod, then we know it must be a member of the cluster
// Case 1: If we have a host ID for the pod, then we know it must be a member of the cluster
// (even if the pod does not exist)
nodeStatus, ok := nodeStatuses[pod.Name]
if ok {
Expand All @@ -663,7 +657,7 @@ func hasPodPotentiallyBootstrapped(pod *corev1.Pod, nodeStatuses api.CassandraSt
state, ok := pod.Labels[api.CassNodeState]
if ok && state != stateReadyToStart {
return true
}
}
}

return false
Expand Down Expand Up @@ -747,7 +741,7 @@ func allPodsBelongToSameNodeOrHaveNoNode(pods []*corev1.Pod) (string, bool) {
}
}

return nodeName, true
return nodeName, true
}

// CheckRackScale loops over each statefulset and makes sure that it has the right
Expand Down Expand Up @@ -1370,9 +1364,11 @@ func (rc *ReconciliationContext) GetStatefulSetForRack(
}

desiredStatefulSet, err := newStatefulSetForCassandraDatacenter(
currentStatefulSet,
nextRack.RackName,
rc.Datacenter,
0)
0,
false)
if err != nil {
return nil, false, err
}
Expand Down Expand Up @@ -2322,7 +2318,6 @@ func (rc *ReconciliationContext) ReconcileAllRacks() (reconcile.Result, error) {
return recResult.Output()
}


if recResult := rc.CheckRollingRestart(); recResult.Completed() {
return recResult.Output()
}
Expand Down
Loading