Merge pull request #8 from ashangit/skip

Skip io-tuning if it has already been done

pkg/api/v1/cluster_types.go

@@ -17,7 +17,6 @@ limitations under the License.
 package v1
 
 import (
-	"github.com/blang/semver"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
@@ -334,14 +333,6 @@ type ScyllaClusterList struct {
 	Items           []ScyllaCluster `json:"items"`
 }
 
-// Version of scylla docker starting from which passing arguments via entry-point is supported
-var ScyllaVersionThatSupportsArgsText = "4.2.0"
-var ScyllaVersionThatSupportsArgs semver.Version
-
 func init() {
-	var err error
-	if ScyllaVersionThatSupportsArgs, err = semver.Parse(ScyllaVersionThatSupportsArgsText); err != nil {
-		panic(err)
-	}
 	SchemeBuilder.Register(&ScyllaCluster{}, &ScyllaClusterList{})
 }

pkg/api/v1/cluster_validation.go

@@ -4,7 +4,6 @@ import (
 	"reflect"
 	"strconv"
 
-	"github.com/blang/semver"
 	"github.com/pkg/errors"
 	"github.com/scylladb/go-set/strset"
 	"k8s.io/apimachinery/pkg/util/sets"
@@ -42,9 +41,9 @@ func checkValues(c *ScyllaCluster) error {
 	}
 
 	if len(c.Spec.ScyllaArgs) > 0 {
-		version, err := semver.Parse(c.Spec.Version)
-		if err == nil && version.LT(ScyllaVersionThatSupportsArgs) {
-			return errors.Errorf("ScyllaArgs is only supported starting from %s", ScyllaVersionThatSupportsArgsText)
+		version := NewScyllaVersion(c.Spec.Version)
+		if !version.SupportFeatureUnsafe(ScyllaVersionThatSupportsArgs) {
+			return errors.Errorf("ScyllaArgs is only supported starting from %s", ScyllaVersionThatSupportsArgs)
 		}
 	}
 

pkg/api/v1/version.go

@@ -0,0 +1,50 @@
+/*
+
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package v1
+
+import (
+	"github.com/blang/semver"
+)
+
+var (
+	ScyllaVersionThatSupportsArgs              = semver.MustParse("4.2.0")
+	ScyllaVersionThatSupportsDisablingIOTuning = semver.MustParse("4.3.0")
+)
+
+// ScyllaVersion contains the version of a cluster with unkown version support
+type ScyllaVersion struct {
+	version semver.Version
+	unknown bool
+}
+
+func NewScyllaVersion(v string) ScyllaVersion {
+	version, err := semver.Parse(v)
+	if err != nil {
+		return ScyllaVersion{unknown: true}
+	}
+	return ScyllaVersion{version: version, unknown: false}
+}
+
+// SupportFeatureUnsafe return true if a feature is supported (and always true if the version is unknown)
+func (sv ScyllaVersion) SupportFeatureUnsafe(featureVersion semver.Version) bool {
+	return sv.unknown || sv.version.GTE(featureVersion)
+}
+
+// SupportFeatureSafe return true if a feature is supported (and always false if the version is unknown)
+func (sv ScyllaVersion) SupportFeatureSafe(featureVersion semver.Version) bool {
+	return !sv.unknown && sv.version.GTE(featureVersion)
+}

pkg/api/v1/version_test.go

@@ -0,0 +1,51 @@
+/*
+
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package v1
+
+import (
+	"testing"
+
+	"github.com/blang/semver"
+)
+
+func TestSupportFeature(t *testing.T) {
+	recentVersion := NewScyllaVersion("99.0.0")
+	oldVersion := NewScyllaVersion("0.0.0")
+	badVersion := NewScyllaVersion("foobar")
+
+	fakeFeature := semver.MustParse("4.2.0")
+
+	if !recentVersion.SupportFeatureUnsafe(fakeFeature) {
+		t.Errorf("Recent version should support a previous version")
+	}
+	if oldVersion.SupportFeatureUnsafe(fakeFeature) {
+		t.Errorf("Old version should not support a future version")
+	}
+	if !badVersion.SupportFeatureUnsafe(fakeFeature) {
+		t.Errorf("Unkown version should support any version when unsafe")
+	}
+
+	if !recentVersion.SupportFeatureSafe(fakeFeature) {
+		t.Errorf("Recent version should support a previous version")
+	}
+	if oldVersion.SupportFeatureSafe(fakeFeature) {
+		t.Errorf("Recent version should support a previous version")
+	}
+	if badVersion.SupportFeatureSafe(fakeFeature) {
+		t.Errorf("Unkown version should not support any version when safe")
+	}
+}

pkg/controllers/sidecar/config/config.go

@@ -10,14 +10,13 @@ import (
 	"strconv"
 	"strings"
 
-	"github.com/blang/semver"
 	"k8s.io/utils/pointer"
 
 	"github.com/ghodss/yaml"
 	"github.com/magiconair/properties"
 	"github.com/pkg/errors"
 	"github.com/scylladb/go-log"
-	"github.com/scylladb/scylla-operator/pkg/api/v1"
+	v1 "github.com/scylladb/scylla-operator/pkg/api/v1"
 	"github.com/scylladb/scylla-operator/pkg/cmd/scylla-operator/options"
 	"github.com/scylladb/scylla-operator/pkg/controllers/sidecar/identity"
 	"github.com/scylladb/scylla-operator/pkg/naming"
@@ -28,8 +27,10 @@ import (
 
 const (
 	configDirScylla                     = "/etc/scylla"
+	configDirScyllaD                    = "/etc/scylla.d"
 	scyllaYAMLPath                      = configDirScylla + "/" + naming.ScyllaConfigName
 	scyllaYAMLConfigMapPath             = naming.ScyllaConfigDirName + "/" + naming.ScyllaConfigName
+	scyllaIOPropertiesPath              = configDirScyllaD + "/" + naming.ScyllaIOPropertiesName
 	scyllaRackDCPropertiesPath          = configDirScylla + "/" + naming.ScyllaRackDCPropertiesName
 	scyllaRackDCPropertiesConfigMapPath = naming.ScyllaConfigDirName + "/" + naming.ScyllaRackDCPropertiesName
 	entrypointPath                      = "/docker-entrypoint.py"
@@ -247,18 +248,24 @@ func (s *ScyllaConfig) setupEntrypoint(ctx context.Context) (*exec.Cmd, error) {
 		args["cpuset"] = &cpusAllowed
 	}
 
+	version := v1.NewScyllaVersion(cluster.Spec.Version)
+
+	s.logger.Info(ctx, "Scylla version detected", "version", version)
+
 	if len(cluster.Spec.ScyllaArgs) > 0 {
-		version, err := semver.Parse(cluster.Spec.Version)
-		if err != nil {
-			s.logger.Info(ctx, "This scylla version might not support ScyllaArgs", "version", cluster.Spec.Version)
-			appendScyllaArguments(ctx, s, cluster.Spec.ScyllaArgs, args)
-		} else if version.LT(v1.ScyllaVersionThatSupportsArgs) {
+		if !version.SupportFeatureUnsafe(v1.ScyllaVersionThatSupportsArgs) {
 			s.logger.Info(ctx, "This scylla version does not support ScyllaArgs. ScyllaArgs is ignored", "version", cluster.Spec.Version)
 		} else {
 			appendScyllaArguments(ctx, s, cluster.Spec.ScyllaArgs, args)
 		}
 	}
 
+	if _, err := os.Stat(scyllaIOPropertiesPath); err == nil && version.SupportFeatureSafe(v1.ScyllaVersionThatSupportsDisablingIOTuning) {
+		s.logger.Info(ctx, "Scylla IO properties are already set, skipping io tuning")
+		ioSetup := "0"
+		args["io-setup"] = &ioSetup
+	}
+
 	var argsList []string
 	for key, value := range args {
 		if value == nil {

pkg/naming/constants.go

@@ -79,6 +79,7 @@ const (
 	ScyllaClientConfigFileName   = "scylla-client.yaml"
 	ScyllaConfigName             = "scylla.yaml"
 	ScyllaRackDCPropertiesName   = "cassandra-rackdc.properties"
+	ScyllaIOPropertiesName       = "io_properties.yaml"
 
 	DataDir = "/var/lib/scylla"