ko-364 fix managed by (#84)

The managed-by flag now uses 'cass-operator' instead of 'cassandra-operator'. The use of 'cassandra-operator' had been an oversight. However, we will continue to use 'cassandra-operator' for labeling PVCs on old CassandraDatacenter resources as the PVC template on StatefulSets cannot be modified.

mage/ginkgo/lib.go

@@ -21,7 +21,8 @@ import (
 )
 
 const (
-	EnvNoCleanup = "M_NO_CLEANUP"
+	EnvNoCleanup  = "M_NO_CLEANUP"
+	OperatorImage = "datastax/cass-operator:latest"
 )
 
 func duplicate(value string, count int) string {
@@ -364,7 +365,7 @@ func (ns *NsWrapper) WaitForOperatorReady() {
 }
 
 func (ns NsWrapper) HelmInstall(chartPath string) {
-	var overrides = map[string]string{"image": "datastax/cass-operator:latest"}
+	var overrides = map[string]string{"image": OperatorImage}
 	err := helm_util.Install(chartPath, "cass-operator", ns.Namespace, overrides)
 	mageutil.PanicOnError(err)
 }

mage/helm/lib.go

@@ -9,12 +9,8 @@ import (
 	shutil "github.com/datastax/cass-operator/mage/sh"
 )
 
-func Install(chartPath string, releaseName string, namespace string, overrides map[string]string) error {
-	args := []string{
-		"install",
-		fmt.Sprintf("--namespace=%s", namespace),
-	}
-
+func buildOverrideArgs(overrides map[string]string) []string {
+	args := []string{}
 	if overrides != nil && len(overrides) > 0 {
 		var overrideString = ""
 		for key, val := range overrides {
@@ -27,7 +23,27 @@ func Install(chartPath string, releaseName string, namespace string, overrides m
 
 		args = append(args, "--set", overrideString)
 	}
+	return args
+}
+
+func Install(chartPath string, releaseName string, namespace string, overrides map[string]string) error {
+	args := []string{
+		"install",
+		fmt.Sprintf("--namespace=%s", namespace),
+	}
+
+	args = append(args, buildOverrideArgs(overrides)...)
+	args = append(args, releaseName, chartPath)
+	return shutil.RunV("helm", args...)
+}
+
+func Upgrade(chartPath string, releaseName string, namespace string, overrides map[string]string) error {
+	args := []string{
+		"upgrade",
+		fmt.Sprintf("--namespace=%s", namespace),
+	}
 
+	args = append(args, buildOverrideArgs(overrides)...)
 	args = append(args, releaseName, chartPath)
 	return shutil.RunV("helm", args...)
 }

operator/pkg/controller/cassandradatacenter/cassandradatacenter_controller.go

@@ -72,6 +72,11 @@ func add(mgr manager.Manager, r reconcile.Reconciler) error {
 		},
 	}
 
+	// NOTE: We do not currently watch PVC resources, but if we did, we'd 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.
+
 	err = c.Watch(
 		&source.Kind{Type: &appsv1.StatefulSet{}},
 		&handler.EnqueueRequestForOwner{

operator/pkg/oplabels/labels.go

@@ -4,14 +4,19 @@
 package oplabels
 
 const (
-	ManagedByLabel      = "app.kubernetes.io/managed-by"
-	ManagedByLabelValue = "cassandra-operator"
+	ManagedByLabel              = "app.kubernetes.io/managed-by"
+	ManagedByLabelValue         = "cass-operator"
+	ManagedByLabelDefunctValue  = "cassandra-operator"
 )
 
 func AddManagedByLabel(m map[string]string) {
 	m[ManagedByLabel] = ManagedByLabelValue
 }
 
+func AddDefunctManagedByLabel(m map[string]string) {
+	m[ManagedByLabel] = ManagedByLabelDefunctValue
+}
+
 func HasManagedByCassandraOperatorLabel(m map[string]string) bool {
 	v, ok := m[ManagedByLabel]
 	return ok && v == ManagedByLabelValue

operator/pkg/reconciliation/constructor.go

@@ -39,6 +39,9 @@ func newServiceForCassandraDatacenter(dc *api.CassandraDatacenter) *corev1.Servi
 			Name: "mgmt-api", Port: 8080, TargetPort: intstr.FromInt(8080),
 		},
 	}
+
+	addHashAnnotation(service)
+
 	return service
 }
 
@@ -63,6 +66,9 @@ func newSeedServiceForCassandraDatacenter(dc *api.CassandraDatacenter) *corev1.S
 
 	service.Spec.Selector = buildLabelSelectorForSeedService(dc)
 	service.Spec.PublishNotReadyAddresses = true
+
+	addHashAnnotation(service)
+
 	return service
 }
 
@@ -72,6 +78,9 @@ func newAllPodsServiceForCassandraDatacenter(dc *api.CassandraDatacenter) *corev
 	service := makeGenericHeadlessService(dc)
 	service.ObjectMeta.Name = dc.GetAllPodsServiceName()
 	service.Spec.PublishNotReadyAddresses = true
+
+	addHashAnnotation(service)
+
 	return service
 }
 
@@ -82,10 +91,11 @@ func newAllPodsServiceForCassandraDatacenter(dc *api.CassandraDatacenter) *corev
 func makeGenericHeadlessService(dc *api.CassandraDatacenter) *corev1.Service {
 	labels := dc.GetDatacenterLabels()
 	oplabels.AddManagedByLabel(labels)
+	selector := dc.GetDatacenterLabels()
 	var service corev1.Service
 	service.ObjectMeta.Namespace = dc.Namespace
 	service.ObjectMeta.Labels = labels
-	service.Spec.Selector = labels
+	service.Spec.Selector = selector
 	service.Spec.Type = "ClusterIP"
 	service.Spec.ClusterIP = "None"
 	return &service
@@ -104,18 +114,55 @@ func newNamespacedNameForStatefulSet(
 	}
 }
 
-// Create a statefulset object for the Datacenter.
+// 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 usesDefunctPvcManagedByLabel(sts *appsv1.StatefulSet) bool {
+	usesDefunct := false
+	for _, pvc := range sts.Spec.VolumeClaimTemplates {
+		value, ok := pvc.Labels[oplabels.ManagedByLabel]
+		if ok && value == oplabels.ManagedByLabelDefunctValue {
+			usesDefunct = true
+			break
+		} 
+	}
+
+	return usesDefunct
+}
+
 func newStatefulSetForCassandraDatacenter(
 	rackName string,
 	dc *api.CassandraDatacenter,
 	replicaCount int) (*appsv1.StatefulSet, error) {
 
+	return newStatefulSetForCassandraDatacenterHelper(rackName, dc, replicaCount, false)
+}
+
+// Create a statefulset object for the Datacenter.
+func newStatefulSetForCassandraDatacenterHelper(
+	rackName string,
+	dc *api.CassandraDatacenter,
+	replicaCount int,
+	useDefunctManagedByForPvc bool) (*appsv1.StatefulSet, error) {
+
 	replicaCountInt32 := int32(replicaCount)
 
 	// see https://github.com/kubernetes/kubernetes/pull/74941
 	// pvc labels are ignored before k8s 1.15.0
 	pvcLabels := dc.GetRackLabels(rackName)
-	oplabels.AddManagedByLabel(pvcLabels)
+	if useDefunctManagedByForPvc {
+		oplabels.AddDefunctManagedByLabel(pvcLabels)
+	} else {
+		oplabels.AddManagedByLabel(pvcLabels)
+	}
 
 	statefulSetLabels := dc.GetRackLabels(rackName)
 	oplabels.AddManagedByLabel(statefulSetLabels)

operator/pkg/reconciliation/constructor_test.go

@@ -8,6 +8,7 @@ import (
 	"testing"
 
 	api "github.com/datastax/cass-operator/operator/pkg/apis/cassandra/v1beta1"
+	"github.com/datastax/cass-operator/operator/pkg/oplabels"
 	"github.com/stretchr/testify/assert"
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
@@ -211,7 +212,7 @@ func TestCassandraDatacenter_buildPodTemplateSpec_labels_merge(t *testing.T) {
 
 	expected := dc.GetRackLabels("testrack")
 	expected[api.CassNodeState] = stateReadyToStart
-	expected["app.kubernetes.io/managed-by"] = "cassandra-operator"
+	expected["app.kubernetes.io/managed-by"] = oplabels.ManagedByLabelValue
 	expected["abc"] = "123"
 
 	assert.NoError(t, err, "should not have gotten error when building podTemplateSpec")

operator/pkg/reconciliation/reconcile_racks.go

@@ -146,6 +146,28 @@ func (rc *ReconciliationContext) CheckRackCreation() result.ReconcileResult {
 	return result.Continue()
 }
 
+func (rc *ReconciliationContext) desiredStatefulSetForExistingStatefulSet(sts *appsv1.StatefulSet, rackName string) (desiredSts *appsv1.StatefulSet, err error) {
+	dc := rc.Datacenter
+
+	// have to use zero here, because each statefulset is created with no replicas
+	// in GetStatefulSetForRack()
+	replicas := 0
+
+	// when Cass Operator was released, we accidentally used the incorrect managed-by
+	// label of "cassandra-operator" we have since fixed this to be "cass-operator",
+	// but unfortunately, we cannot modify the labels in the volumeClaimTemplates of 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
+}
+
 func (rc *ReconciliationContext) CheckRackPodTemplate() result.ReconcileResult {
 	logger := rc.ReqLogger
 	dc := rc.Datacenter
@@ -161,9 +183,8 @@ func (rc *ReconciliationContext) CheckRackPodTemplate() result.ReconcileResult {
 		}
 		statefulSet := rc.statefulSets[idx]
 
-		// have to use zero here, because each statefulset is created with no replicas
-		// in GetStatefulSetForRack()
-		desiredSts, err := newStatefulSetForCassandraDatacenter(rackName, dc, 0)
+		desiredSts, err := rc.desiredStatefulSetForExistingStatefulSet(statefulSet, rackName)
+
 		if err != nil {
 			logger.Error(err, "error calling newStatefulSetForCassandraDatacenter")
 			return result.Error(err)

operator/pkg/reconciliation/reconcile_services.go

@@ -4,13 +4,13 @@
 package reconciliation
 
 import (
-	"reflect"
-
 	"github.com/datastax/cass-operator/operator/internal/result"
 	api "github.com/datastax/cass-operator/operator/pkg/apis/cassandra/v1beta1"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/types"
+
+	"github.com/datastax/cass-operator/operator/pkg/utils"
 )
 
 func (rc *ReconciliationContext) CreateHeadlessServices() result.ReconcileResult {
@@ -90,19 +90,23 @@ func (rc *ReconciliationContext) CheckHeadlessServices() result.ReconcileResult
 			return result.Error(err)
 
 		} else {
-			// if we found the service already, check if the labels are right
-			currentLabels := currentService.GetLabels()
-			desiredLabels := desiredSvc.GetLabels()
-			shouldUpdateLabels := !reflect.DeepEqual(currentLabels, desiredLabels)
-			if shouldUpdateLabels {
-				logger.Info("Updating labels",
+			// if we found the service already, check if they need updating
+			if !resourcesHaveSameHash(currentService, desiredSvc) {
+				resourceVersion := currentService.GetResourceVersion()
+				// preserve any labels and annotations that were added to the service post-creation
+				desiredSvc.Labels = utils.MergeMap(map[string]string{}, currentService.Labels, desiredSvc.Labels)
+				desiredSvc.Annotations = utils.MergeMap(map[string]string{}, currentService.Annotations, desiredSvc.Annotations)
+
+				logger.Info("Updating service",
 					"service", currentService,
-					"current", currentLabels,
-					"desired", desiredLabels)
-				currentService.SetLabels(desiredLabels)
+					"desired", desiredSvc)
+
+				desiredSvc.DeepCopyInto(currentService)
+
+				currentService.SetResourceVersion(resourceVersion)
 
 				if err := client.Update(rc.Ctx, currentService); err != nil {
-					logger.Error(err, "Unable to update service with labels",
+					logger.Error(err, "Unable to update service",
 						"service", currentService)
 					return result.Error(err)
 				}

tests/config_change_condition/config_change_condition_suite_test.go

@@ -1,7 +1,7 @@
 // Copyright DataStax, Inc.
 // Please see the included license file for details.
 
-package config_change
+package config_change_condition
 
 import (
 	"fmt"

tests/testdata/cass-operator-1.1.0-chart/Chart.yaml

@@ -0,0 +1,4 @@
+apiVersion: v2
+name: cass-operator
+version: 1.0.0
+description: Helm chart for Cass Operator.

tests/testdata/cass-operator-1.1.0-chart/templates/clusterrole.yaml

@@ -0,0 +1,16 @@
+apiVersion: rbac.authorization.k8s.io/v1
+kind: ClusterRole
+metadata:
+  creationTimestamp: null
+  name: {{ .Values.clusterRoleName }}
+rules:
+- apiGroups:
+  - admissionregistration.k8s.io
+  resources:
+  - validatingwebhookconfigurations
+  verbs:
+  - create
+  - get
+  - update
+  resourceNames:
+  - "cassandradatacenter-webhook-registration"

tests/testdata/cass-operator-1.1.0-chart/templates/clusterrolebinding.yaml

@@ -0,0 +1,12 @@
+kind: ClusterRoleBinding
+apiVersion: rbac.authorization.k8s.io/v1
+metadata:
+  name: {{ .Values.clusterRoleBindingName }}
+subjects:
+- kind: ServiceAccount
+  name: {{ .Values.serviceAccountName }}
+  namespace: {{ .Release.Namespace }}
+roleRef:
+  kind: ClusterRole
+  name: {{ .Values.clusterRoleName }}
+  apiGroup: rbac.authorization.k8s.io