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.

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

fix: support 3.x to 4.x upgrades when num_tokens are not set in k8ssandraCluster #1118

Merged
merged 9 commits into from
Dec 20, 2023
8 changes: 8 additions & 0 deletions apis/k8ssandra/v1alpha1/k8ssandracluster_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package v1alpha1

import (
"github.com/Masterminds/semver/v3"
cassdcapi "github.com/k8ssandra/cass-operator/apis/cassandra/v1beta1"
medusaapi "github.com/k8ssandra/k8ssandra-operator/apis/medusa/v1alpha1"
reaperapi "github.com/k8ssandra/k8ssandra-operator/apis/reaper/v1alpha1"
Expand Down Expand Up @@ -524,3 +525,10 @@
ServerDistributionCassandra = ServerDistribution("cassandra")
ServerDistributionDse = ServerDistribution("dse")
)

func (kc *K8ssandraCluster) DefaultNumTokens(serverVersion *semver.Version) float64 {
if kc.Spec.Cassandra.ServerType == ServerDistributionCassandra && serverVersion.Major() == 3 {
return float64(256)
}
return float64(16)

Check warning on line 533 in apis/k8ssandra/v1alpha1/k8ssandracluster_types.go

View check run for this annotation

Codecov / codecov/patch

apis/k8ssandra/v1alpha1/k8ssandracluster_types.go#L533

Added line #L533 was not covered by tests
}
31 changes: 19 additions & 12 deletions apis/k8ssandra/v1alpha1/k8ssandracluster_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import (
"fmt"

"github.com/Masterminds/semver/v3"
"github.com/k8ssandra/k8ssandra-operator/pkg/clientcache"
"github.com/pkg/errors"
"k8s.io/apimachinery/pkg/runtime"
Expand Down Expand Up @@ -114,18 +115,24 @@

oldCassConfig := oldCluster.Spec.Cassandra.DatacenterOptions.CassandraConfig
newCassConfig := r.Spec.Cassandra.DatacenterOptions.CassandraConfig

var oldNumTokens, newNumTokens interface{}

if oldCassConfig != nil {
oldNumTokens = oldCassConfig.CassandraYaml["num_tokens"]
}
if newCassConfig != nil {
newNumTokens = newCassConfig.CassandraYaml["num_tokens"]
}

if oldNumTokens != newNumTokens {
return ErrNumTokens
if oldCassConfig != nil && newCassConfig != nil {
oldNumTokens, oldNumTokensExists := oldCassConfig.CassandraYaml["num_tokens"]
newNumTokens, newNumTokensExists := newCassConfig.CassandraYaml["num_tokens"]

if !oldNumTokensExists {
cassVersion, err := semver.NewVersion(oldCluster.Spec.Cassandra.ServerVersion)
if err != nil {
return err
}
defaultNumTokens := oldCluster.DefaultNumTokens(cassVersion)
if newNumTokensExists && newNumTokens.(float64) != defaultNumTokens {
return ErrNumTokens
}

Check warning on line 130 in apis/k8ssandra/v1alpha1/k8ssandracluster_webhook.go

View check run for this annotation

Codecov / codecov/patch

apis/k8ssandra/v1alpha1/k8ssandracluster_webhook.go#L129-L130

Added lines #L129 - L130 were not covered by tests
} else {
if oldNumTokens != newNumTokens {
return ErrNumTokens
}
}
}

// Verify that the cluster name override was not changed
Expand Down
20 changes: 19 additions & 1 deletion apis/k8ssandra/v1alpha1/k8ssandracluster_webhook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,7 @@ func TestWebhook(t *testing.T) {
t.Run("ReaperKeyspaceValidation", testReaperKeyspaceValidation)
t.Run("StorageConfigValidation", testStorageConfigValidation)
t.Run("NumTokensValidation", testNumTokens)
t.Run("NumTokensValidationInUpdate", testNumTokensInUpdate)
}

func testContextValidation(t *testing.T) {
Expand All @@ -159,6 +160,24 @@ func testContextValidation(t *testing.T) {
required.Error(err)
}

func testNumTokensInUpdate(t *testing.T) {
require := require.New(t)
createNamespace(require, "numtokensupdate-namespace")
cluster := createMinimalClusterObj("numtokens-test-update", "numtokensupdate-namespace")
cluster.Spec.Cassandra.ServerVersion = "3.11.10"
cluster.Spec.Cassandra.DatacenterOptions.CassandraConfig = &CassandraConfig{}
err := k8sClient.Create(ctx, cluster)
require.NoError(err)

// Now update to 4.1.3
cluster.Spec.Cassandra.DatacenterOptions.CassandraConfig.CassandraYaml = unstructured.Unstructured{"num_tokens": 256}
cluster.Spec.Cassandra.ServerVersion = "4.1.3"

// This should be acceptable change, since 3.11.10 defaulted to 256 and so it is the same value
err = k8sClient.Update(ctx, cluster)
require.NoError(err)
}

func testReaperKeyspaceValidation(t *testing.T) {
required := require.New(t)
createNamespace(required, "update-namespace")
Expand Down Expand Up @@ -316,7 +335,6 @@ func testNumTokens(t *testing.T) {

errorOnValidate = (*newCluster).ValidateUpdate(oldCluster)
required.Error(errorOnValidate, "expected error when changing the value of num tokens while also changing other field values")

}

func createNamespace(require *require.Assertions, namespace string) {
Expand Down
10 changes: 9 additions & 1 deletion controllers/k8ssandra/datacenters.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
"strconv"
"strings"

"github.com/Masterminds/semver/v3"
"github.com/go-logr/logr"
cassdcapi "github.com/k8ssandra/cass-operator/apis/cassandra/v1beta1"
cassctlapi "github.com/k8ssandra/cass-operator/apis/control/v1alpha1"
Expand Down Expand Up @@ -158,7 +159,14 @@
dcLogger.Error(err, "Stopped cannot be set to true until the CassandraDatacenter is fully rebuilt")
}

if err := cassandra.ValidateConfig(desiredDc, actualDc); err != nil {
// Set the proper default during upgrade from 3.x to 4.x
if kc.Spec.Cassandra.ServerType == api.ServerDistributionCassandra && semver.MustParse(actualDc.Spec.ServerVersion).Major() == 3 && semver.MustParse(desiredDc.Spec.ServerVersion).Major() > 3 {
if desiredDc, err = cassandra.SetNewDefaultNumTokens(kc, desiredDc, actualDc); err != nil {
return result.Error(fmt.Errorf("couldn't set the proper default during upgrade: %v", err)), actualDcs
}

Check warning on line 166 in controllers/k8ssandra/datacenters.go

View check run for this annotation

Codecov / codecov/patch

controllers/k8ssandra/datacenters.go#L164-L166

Added lines #L164 - L166 were not covered by tests
}

if err = cassandra.ValidateConfig(desiredDc, actualDc); err != nil {
return result.Error(fmt.Errorf("invalid Cassandra config: %v", err)), actualDcs
}

Expand Down
1 change: 1 addition & 0 deletions controllers/k8ssandra/k8ssandracluster_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package k8ssandra

import (
"context"

v1 "k8s.io/api/core/v1"

"github.com/go-logr/logr"
Expand Down
33 changes: 33 additions & 0 deletions pkg/cassandra/datacenter.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package cassandra

import (
"encoding/json"
"fmt"

"github.com/Masterminds/semver/v3"
Expand Down Expand Up @@ -517,6 +518,38 @@
return nil
}

func SetNewDefaultNumTokens(kc *api.K8ssandraCluster, desiredDc, actualDc *cassdcapi.CassandraDatacenter) (*cassdcapi.CassandraDatacenter, error) {
desiredConfig, err := utils.UnmarshalToMap(desiredDc.Spec.Config)
if err != nil {
return nil, err
}

Check warning on line 525 in pkg/cassandra/datacenter.go

View check run for this annotation

Codecov / codecov/patch

pkg/cassandra/datacenter.go#L524-L525

Added lines #L524 - L525 were not covered by tests
actualConfig, err := utils.UnmarshalToMap(actualDc.Spec.Config)
if err != nil {
return nil, err
}

Check warning on line 529 in pkg/cassandra/datacenter.go

View check run for this annotation

Codecov / codecov/patch

pkg/cassandra/datacenter.go#L528-L529

Added lines #L528 - L529 were not covered by tests

actualCassYaml := actualConfig["cassandra-yaml"].(map[string]interface{})
desiredCassYaml := desiredConfig["cassandra-yaml"].(map[string]interface{})

var numTokensExists bool
cassConfig := kc.Spec.Cassandra.CassandraConfig
if cassConfig != nil {
_, numTokensExists = cassConfig.CassandraYaml["num_tokens"]
}

if !numTokensExists {
desiredCassYaml["num_tokens"] = actualCassYaml["num_tokens"]
desiredConfig["cassandra-yaml"] = desiredCassYaml
newConfig, err := json.Marshal(desiredConfig)
if err != nil {
return nil, err
}

Check warning on line 546 in pkg/cassandra/datacenter.go

View check run for this annotation

Codecov / codecov/patch

pkg/cassandra/datacenter.go#L545-L546

Added lines #L545 - L546 were not covered by tests
desiredDc.Spec.Config = newConfig
}

return desiredDc, nil
}

func AddOrUpdateVolume(dcConfig *DatacenterConfig, volume *corev1.Volume, volumeIndex int, found bool) {
AddOrUpdateVolumeToSpec(&dcConfig.PodTemplateSpec, volume, volumeIndex, found)
}
Expand Down
87 changes: 79 additions & 8 deletions pkg/cassandra/datacenter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,23 +3,21 @@ package cassandra
import (
"testing"

"github.com/Masterminds/semver/v3"
cassdcapi "github.com/k8ssandra/cass-operator/apis/cassandra/v1beta1"
"github.com/k8ssandra/cass-operator/pkg/reconciliation"

api "github.com/k8ssandra/k8ssandra-operator/apis/k8ssandra/v1alpha1"
"github.com/k8ssandra/k8ssandra-operator/apis/telemetry/v1alpha1"
"github.com/k8ssandra/k8ssandra-operator/pkg/meta"
"github.com/k8ssandra/k8ssandra-operator/pkg/unstructured"
"k8s.io/utils/pointer"

"github.com/k8ssandra/k8ssandra-operator/pkg/utils"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"github.com/Masterminds/semver/v3"
cassdcapi "github.com/k8ssandra/cass-operator/apis/cassandra/v1beta1"
api "github.com/k8ssandra/k8ssandra-operator/apis/k8ssandra/v1alpha1"
"github.com/k8ssandra/k8ssandra-operator/apis/telemetry/v1alpha1"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/resource"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
"k8s.io/utils/pointer"
)

func TestCoalesce(t *testing.T) {
Expand Down Expand Up @@ -1482,3 +1480,76 @@ func GetDatacenterConfig() DatacenterConfig {
McacEnabled: true,
}
}

func TestSetNewDefaultNumTokens(t *testing.T) {
testCases := []struct {
name string
kc *api.K8ssandraCluster
actualDcConfig *unstructured.Unstructured
desiredDcConfig *unstructured.Unstructured
expectedDcNumTokens float64
}{
{
name: "num_tokens exists in CassandraConfig",
kc: &api.K8ssandraCluster{
Spec: api.K8ssandraClusterSpec{
Cassandra: &api.CassandraClusterTemplate{
DatacenterOptions: api.DatacenterOptions{
CassandraConfig: &api.CassandraConfig{
CassandraYaml: unstructured.Unstructured{"num_tokens": 33},
},
},
},
},
},
actualDcConfig: &unstructured.Unstructured{"num_tokens": 24},
desiredDcConfig: &unstructured.Unstructured{"num_tokens": 33},
expectedDcNumTokens: 33,
},
{
name: "num_tokens does not exist, set to value from actualDc",
kc: &api.K8ssandraCluster{
Spec: api.K8ssandraClusterSpec{
Cassandra: &api.CassandraClusterTemplate{
DatacenterOptions: api.DatacenterOptions{
CassandraConfig: &api.CassandraConfig{
CassandraYaml: unstructured.Unstructured{"other": "setting"},
},
},
},
},
},
actualDcConfig: &unstructured.Unstructured{"num_tokens": 256},
desiredDcConfig: &unstructured.Unstructured{"num_tokens": 16},
expectedDcNumTokens: 256,
},
}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
actualDcConfig := GetDatacenterConfig()

actualDcConfig.CassandraConfig.CassandraYaml = *tc.actualDcConfig
actualDc, err := NewDatacenter(
types.NamespacedName{Name: "testdc", Namespace: "test-namespace"},
&actualDcConfig,
)
assert.NoError(t, err)
desiredDcConfig := GetDatacenterConfig()
desiredDcConfig.CassandraConfig.CassandraYaml = *tc.desiredDcConfig
desiredDc, err := NewDatacenter(
types.NamespacedName{Name: "testdc", Namespace: "test-namespace"},
&desiredDcConfig,
)
assert.NoError(t, err)

got, err := SetNewDefaultNumTokens(tc.kc, desiredDc, actualDc)
assert.NoError(t, err)

config, err := utils.UnmarshalToMap(got.Spec.Config)
assert.NoError(t, err)
cassYaml := config["cassandra-yaml"].(map[string]interface{})
assert.Equal(t, tc.expectedDcNumTokens, cassYaml["num_tokens"])
})
}
}
Loading