Repository: scylladb/scylla-operator
Pull request #379 (2021-02-11 14:07:16)

Skip io-tuning if it has already been done


<!-- Please take a look at our [Contributing](https://github.com/scylladb/scylla-operator/blob/master/docs/contributing.md)
documentation before submitting a Pull Request!
Thank you for contributing to the Scylla Operator! -->

**Description of your changes:**

Right now, every-time the scylla container restart we have to redo the IO tuning.
Since it's quite long it can slow down operations on the cluster (like a simple rolling restart).
This change makes the sidecar skip io tuning if the properties file exists.

**Which issue is resolved by this Pull Request:**
Alternative to #179 

**Checklist:**
- [x] Documentation has been updated, if necessary.
- [x] Image has been built (`make docker-build`) on the last commit.


Patch:
diff --git a/docs/source/conf.py b/docs/source/conf.py
index ed595821d..ad6eff136 100644
--- a/docs/source/conf.py
+++ b/docs/source/conf.py
@@ -86,7 +86,7 @@ rst_prolog = """
 .. |mon_version| replace:: 3.1
 .. |man_version| replace:: 2.0
 .. |mon_root| replace::  :doc:`Scylla Monitoring Stack </operating-scylla/monitoring/index>`
-""" 
+"""
 
 # -- Options for HTML output ----------------------------------------------
 
@@ -131,7 +131,7 @@ html_sidebars = {'**': ['side-nav.html']}
 # Output file base name for HTML help builder.
 htmlhelp_basename = 'ScyllaDocumentationdoc'
 
-# URL which points to the root of the HTML documentation. 
+# URL which points to the root of the HTML documentation.
 html_baseurl = 'https://operator.docs.scylladb.com'
 
 # Dictionary of values to pass into the template engine’s context for all pages
@@ -155,7 +155,7 @@ redirects_file = "_utils/redirections.yaml"
 TAGS = []
 smv_tag_whitelist = multiversion_regex_builder(TAGS)
 # Whitelist pattern for branches (set to None to ignore all branches)
-BRANCHES = ['master', 'v0.3', 'v1.0']
+BRANCHES = ['master', 'v0.3', 'v1.0', 'v1.1']
 smv_branch_whitelist = multiversion_regex_builder(BRANCHES)
 # Defines which version is considered to be the latest stable version.
 # Must be listed in smv_tag_whitelist or smv_branch_whitelist.
diff --git a/docs/source/upgrade.md b/docs/source/upgrade.md
index feeec43ba..97b3b9db8 100644
--- a/docs/source/upgrade.md
+++ b/docs/source/upgrade.md
@@ -2,6 +2,46 @@
 
 This pages describes Scylla Operator upgrade procedures.
 
+## `v1.0.0` -> `v1.1.0`
+
+During this update we will change probes and image for Scylla Operator.
+A new version brings an automation for upgrade of sidecar image, so a rolling restart of managed Scylla clusters is expected.
+
+1. Get name of StatefulSet managing Scylla Operator
+   ```shell
+   kubectl --namespace scylla-operator-system get sts --selector="control-plane=controller-manager"
+
+   NAME                                 READY   AGE
+   scylla-operator-controller-manager   1/1     95m
+   ```
+
+1. Change probes and used container image by applying following patch:
+    ```yaml
+    spec:
+      template:
+        spec:
+          containers:
+          - name: manager
+            image: docker.io/scylladb/scylla-operator:1.1.0
+            livenessProbe:
+              httpGet:
+                path: /healthz
+                port: 8080
+                scheme: HTTP
+            readinessProbe:
+              $retainKeys:
+              - httpGet
+              httpGet:
+                path: /readyz
+                port: 8080
+                scheme: HTTP
+    ```
+    To apply above patch save it to file (`operator-patch.yaml` for example) and apply to Operator StatefulSet:
+    ```shell
+    kubectl -n scylla-operator-system patch sts scylla-operator-controller-manager --patch "$(cat operator-patch.yaml)"
+    ```
+
+
 ## `v0.3.0` -> `v1.0.0`
 
 ***Note:*** There's an experimental migration procedure available [here](migration.md).
@@ -50,4 +90,3 @@ In case you need to preserve your data, refer to backup and restore guide.
     kind: ScyllaCluster
     ```
 1. Once your cluster definition is ready, use `kubectl apply` to install fresh Scylla cluster.
-
diff --git a/examples/common/monitoring/scylla-service-monitor.yaml b/examples/common/monitoring/scylla-service-monitor.yaml
index 0ddad230d..fe252e52c 100644
--- a/examples/common/monitoring/scylla-service-monitor.yaml
+++ b/examples/common/monitoring/scylla-service-monitor.yaml
@@ -6,7 +6,7 @@ metadata:
 spec:
   jobLabel: "app"
   targetLabels: ["scylla/cluster"]
-  podTargetLabels: ["scylla/datacenter"]
+  podTargetLabels: ["scylla/datacenter","scylla/rack"]
   selector:
     matchLabels:
       app: scylla
diff --git a/pkg/api/v1/cluster_types.go b/pkg/api/v1/cluster_types.go
index e83f8b3a9..7afa3f2d2 100644
--- a/pkg/api/v1/cluster_types.go
+++ b/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{})
 }
diff --git a/pkg/api/v1/cluster_validation.go b/pkg/api/v1/cluster_validation.go
index 37f6ea968..402c14c1e 100644
--- a/pkg/api/v1/cluster_validation.go
+++ b/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)
 		}
 	}
 
diff --git a/pkg/api/v1/version.go b/pkg/api/v1/version.go
new file mode 100644
index 000000000..989bf90e2
--- /dev/null
+++ b/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)
+}
diff --git a/pkg/controllers/cluster/status.go b/pkg/controllers/cluster/status.go
index cf06e0d38..f44477c0c 100644
--- a/pkg/controllers/cluster/status.go
+++ b/pkg/controllers/cluster/status.go
@@ -57,8 +57,9 @@ func (cc *ClusterReconciler) updateStatus(ctx context.Context, cluster *scyllav1
 		}
 		// Get corresponding StatefulSet from lister
 		err := cc.Get(ctx, naming.NamespacedName(naming.StatefulSetNameForRack(rack, cluster), cluster.Namespace), sts)
-		// If it wasn't found, continue
+		// If it wasn't found, clear rack status and continue
 		if apierrors.IsNotFound(err) {
+			delete(cluster.Status.Racks, rack.Name)
 			continue
 		}
 		// If we got a different error, requeue and log it
diff --git a/pkg/controllers/sidecar/config/config.go b/pkg/controllers/sidecar/config/config.go
index e9484e029..917191d88 100644
--- a/pkg/controllers/sidecar/config/config.go
+++ b/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"
@@ -244,18 +245,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 {
diff --git a/pkg/naming/constants.go b/pkg/naming/constants.go
index 60baf06f8..9713909fb 100644
--- a/pkg/naming/constants.go
+++ b/pkg/naming/constants.go
@@ -78,6 +78,7 @@ const (
 	ScyllaClientConfigFileName   = "scylla-client.yaml"
 	ScyllaConfigName             = "scylla.yaml"
 	ScyllaRackDCPropertiesName   = "cassandra-rackdc.properties"
+	ScyllaIOPropertiesName       = "io_properties.yaml"
 
 	DataDir = "/var/lib/scylla"
 


Test patch:
diff --git a/pkg/api/v1/version_test.go b/pkg/api/v1/version_test.go
new file mode 100644
index 000000000..97a3dc3de
--- /dev/null
+++ b/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")
+	}
+}
diff --git a/pkg/controllers/cluster/status_test.go b/pkg/controllers/cluster/status_test.go
new file mode 100644
index 000000000..713ed17b7
--- /dev/null
+++ b/pkg/controllers/cluster/status_test.go
@@ -0,0 +1,34 @@
+// Copyright (C) 2021 ScyllaDB
+
+package cluster
+
+import (
+	"context"
+	"testing"
+
+	"github.com/scylladb/scylla-operator/pkg/test/unit"
+	"k8s.io/apimachinery/pkg/runtime"
+	clientgoscheme "k8s.io/client-go/kubernetes/scheme"
+	"sigs.k8s.io/controller-runtime/pkg/client/fake"
+)
+
+func TestClusterStatus_RackStatusIsRemovedWhenStatefulSetNoLongerExists(t *testing.T) {
+	cluster := unit.NewSingleRackCluster(1)
+	rack := cluster.Spec.Datacenter.Racks[0]
+
+	scheme := runtime.NewScheme()
+	if err := clientgoscheme.AddToScheme(scheme); err != nil {
+		t.Fatal(err)
+	}
+	cc := &ClusterReconciler{
+		Client: fake.NewFakeClientWithScheme(scheme),
+	}
+
+	if err := cc.updateStatus(context.Background(), cluster); err != nil {
+		t.Errorf("expected nil, got %s", err)
+	}
+
+	if v, ok := cluster.Status.Racks[rack.Name]; ok {
+		t.Errorf("expected rack status to be cleared, got %v", v)
+	}
+}