K8SPSMDB-733: Improve failure detection for PVC resize (#1543)

* K8SPSMDB-733: Improve failure detection for PVC resize

Operator were reporting that PVC resize completed successfully even
though it's failed. This commit fixes this behavior and introduce some
improvements.

Operator tries to understand if resize operation in progress by checking
the status.conditions of PVCs. If the condition says it's resizing
the operation is in progress, if there's no condition then the operation
completed successfully. This approach is naive since resize could be
failed. With these changes operator will check events regarding to
the PVC and check if `VolumeResizeFailed` event exists. If it does,
resize operation will declared as failed.

If a resize operation fails, operator will automatically revert the PVC
size in cr.yaml. This way the probability to have discrepancy between `CR <->
STS <-> PVC` will be lower. If resize operation fails only for some
PVCs, the operator will still revert the PVC size in cr.yaml with the
size of failed PVCs.

* remove unnecessary logs

---------

Co-authored-by: Viacheslav Sarzhan <slava.sarzhan@percona.com>

Author: egegunes

deploy/bundle.yaml

@@ -19059,10 +19059,13 @@ rules:
   - patch
   - delete
 - apiGroups:
-  - ""
+  - events.k8s.io
   resources:
   - events
   verbs:
+  - get
+  - list
+  - watch
   - create
   - patch
 - apiGroups:

deploy/cw-bundle.yaml

@@ -19079,10 +19079,13 @@ rules:
   - patch
   - delete
 - apiGroups:
-  - ""
+  - events.k8s.io
   resources:
   - events
   verbs:
+  - get
+  - list
+  - watch
   - create
   - patch
 - apiGroups:

deploy/cw-rbac.yaml

@@ -111,10 +111,13 @@ rules:
   - patch
   - delete
 - apiGroups:
-  - ""
+  - events.k8s.io
   resources:
   - events
   verbs:
+  - get
+  - list
+  - watch
   - create
   - patch
 - apiGroups:

deploy/rbac.yaml

@@ -91,10 +91,13 @@ rules:
   - patch
   - delete
 - apiGroups:
-  - ""
+  - events.k8s.io
   resources:
   - events
   verbs:
+  - get
+  - list
+  - watch
   - create
   - patch
 - apiGroups:

e2e-tests/conf/client.yml

@@ -12,6 +12,7 @@ spec:
       labels:
         name: psmdb-client
     spec:
+      terminationGracePeriodSeconds: 10
       containers:
         - name: psmdb-client
           image: percona/percona-server-mongodb:4.4

e2e-tests/pvc-resize/conf/resourcequota.yml

@@ -0,0 +1,7 @@
+apiVersion: v1
+kind: ResourceQuota
+metadata:
+  name: default-storage-quota
+spec:
+  hard:
+    STORAGECLASS.storageclass.storage.k8s.io/requests.storage: QUOTA

e2e-tests/pvc-resize/run

@@ -5,12 +5,68 @@ set -o errexit
 test_dir=$(realpath $(dirname $0))
 . ${test_dir}/../functions
 
+function patch_pvc_request() {
+	local cluster=$1
+	local size=$2
+
+	echo "Patching PVC request to ${size} in ${cluster}"
+
+	kubectl_bin patch psmdb ${cluster} --type=json -p='[{"op": "replace", "path": "/spec/replsets/0/volumeSpec/persistentVolumeClaim/resources/requests/storage", "value":"'"${size}"'"}]'
+}
+
+function get_default_storageclass() {
+	kubectl_bin get sc -o jsonpath='{.items[?(@.metadata.annotations.storageclass\.kubernetes\.io/is-default-class=="true")].metadata.name}'
+}
+
+function ensure_default_sc_allows_expansion() {
+	local default_sc=$(get_default_storageclass)
+
+    echo "Checking if default storageclass ${default_sc} allows volume expansion"
+
+    local allowVolumeExpansion=$(kubectl_bin get sc -o jsonpath='{.items[?(@.metadata.name=="'"${default_sc}"'")].allowVolumeExpansion}')
+
+	if [[ ${allowVolumeExpansion} != "true" ]]; then
+		echo "Default storageclass ${default_sc} does not allow volume expansion"
+		exit 0
+	fi
+}
+
+function apply_resourcequota() {
+    local quota=$1
+    local default_sc=$(get_default_storageclass)
+
+    echo "Applying resourcequota for default storageclass ${default_sc} with quota ${quota}"
+
+	cat ${test_dir}/conf/resourcequota.yml \
+		| sed "s/STORAGECLASS/${default_sc}/" \
+		| sed "s/QUOTA/${quota}/" \
+		| kubectl_bin apply -f -
+}
+
+function wait_cluster_status() {
+	local cluster=$1
+	local expected=$2
+
+	echo -n "Waiting for psmdb/${cluster} status to be ${expected}"
+	until [[ $(kubectl_bin get psmdb ${cluster} -o jsonpath='{.status.state}') == ${expected} ]]; do
+		if [[ $retry -ge 60 ]]; then
+			echo
+			echo "psmdb/${cluster} did not reach ${expected} status, max retries exceeded"
+			exit 1
+		fi
+		echo -n "."
+		sleep 5
+
+		retry=$((retry + 1))
+	done
+
+	echo
+	echo "psmdb/${cluster} status is ${expected}"
+}
+
 set_debug
 
-if [[ $EKS == 1 ]]; then
-    echo "Skip the test. We don't run it for EKS."
-    exit 0
-fi
+ensure_default_sc_allows_expansion
 
 create_infra ${namespace}
 
@@ -23,24 +79,87 @@ desc 'create PSMDB cluster'
 cluster="some-name"
 spinup_psmdb "${cluster}-rs0" "$conf_dir/$cluster-rs0.yml"
 
-kubectl_bin patch psmdb ${cluster} --type=json -p='[{"op": "replace", "path": "/spec/replsets/0/volumeSpec/persistentVolumeClaim/resources/requests/storage", "value":"5Gi"}]'
+patch_pvc_request "${cluster}" "2Gi"
 wait_cluster_consistency "$cluster" 3 2
+echo
 
 for pvc in $(kubectl_bin get pvc -l app.kubernetes.io/component=mongod -o name); do
     retry=0
-    until [[ $(kubectl_bin get ${pvc} -o jsonpath={.status.capacity.storage}) == "5Gi" ]]; do
+    echo -n "Waiting for pvc/${pvc} to be resized"
+    until [[ $(kubectl_bin get ${pvc} -o jsonpath={.status.capacity.storage}) == "2Gi" ]]; do
         if [[ $retry -ge 60 ]]; then
-            echo "PVC ${pvc} was not resized, max retries exceeded"
+            echo
+            echo "pvc/${pvc} was not resized, max retries exceeded"
             exit 1
         fi
+        echo -n "."
+        sleep 5
 
-        echo "Waiting for PVC ${pvc} to be resized"
+        retry=$((retry + 1))
+    done
+    echo
+    echo "pvc/${pvc} was resized"
+done
+
+desc 'create resourcequota'
+
+# We're setting the quota to 7Gi, so we can only resize the first PVC to 3Gi
+# the others should fail to resize due to the exceeded quota but operator should
+# handle the error and keep the cluster ready
+
+apply_resourcequota 7Gi
+patch_pvc_request "${cluster}" "3Gi"
+wait_cluster_consistency "$cluster" 3 2
+echo
+
+echo -n "Waiting for pvc/mongod-data-some-name-rs0-0 to be resized"
+until [[ $(kubectl_bin get pvc mongod-data-some-name-rs0-0 -o jsonpath={.status.capacity.storage}) == "3Gi" ]]; do
+	if [[ $retry -ge 60 ]]; then
+		echo
+		echo "pvc/mongod-data-some-name-rs0-0 was not resized, max retries exceeded"
+		exit 1
+	fi
+	echo -n "."
+	sleep 5
+
+	retry=$((retry + 1))
+done
+echo
+echo "pvc/mongod-data-some-name-rs0-0 was resized"
+
+# We're setting the quota to 9Gi, so we can resize all PVCs to 3Gi
+
+apply_resourcequota 9Gi
+patch_pvc_request "${cluster}" "3Gi"
+wait_cluster_consistency "$cluster" 3 2
+echo
+for pvc in $(kubectl_bin get pvc -l app.kubernetes.io/component=mongod -o name); do
+    retry=0
+    echo -n "Waiting for pvc/${pvc} to be resized"
+    until [[ $(kubectl_bin get ${pvc} -o jsonpath={.status.capacity.storage}) == "3Gi" ]]; do
+        if [[ $retry -ge 60 ]]; then
+            echo
+            echo "pvc/${pvc} was not resized, max retries exceeded"
+            exit 1
+        fi
+        echo -n "."
         sleep 5
 
         retry=$((retry + 1))
     done
-    echo "PVC ${pvc} was resized"
+    echo
+    echo "pvc/${pvc} was resized"
 done
 
+desc "test downscale"
+
+# operator shouldn't try to downscale the PVCs and set status to error
+patch_pvc_request "${cluster}" "1Gi"
+wait_cluster_status ${cluster} "error"
+
+# user should be able to restore to the previous size and make the cluster ready
+patch_pvc_request "${cluster}" "3Gi"
+wait_cluster_status ${cluster} "ready"
+
 destroy "${namespace}"
 desc "test passed"

pkg/apis/psmdb/v1/psmdb_types.go

@@ -1224,4 +1224,4 @@ const (
 func (cr *PerconaServerMongoDB) PVCResizeInProgress() bool {
 	_, ok := cr.Annotations[AnnotationPVCResizeInProgress]
 	return ok
-}
+}

pkg/controller/perconaservermongodb/statefulset.go

@@ -50,6 +50,11 @@ func (r *ReconcilePerconaServerMongoDB) reconcileStatefulSet(ctx context.Context
 		return nil, errors.Wrapf(err, "reconcile PVCs for %s", sfs.Name)
 	}
 
+	if _, ok := sfs.Annotations[api.AnnotationPVCResizeInProgress]; ok {
+		log.V(1).Info("PVC resize in progress, skipping reconciliation of statefulset", "name", sfs.Name)
+		return sfs, nil
+	}
+
 	err = r.createOrUpdate(ctx, sfs)
 	if err != nil {
 		return nil, errors.Wrapf(err, "update StatefulSet %s", sfs.Name)