Support redunce volume size

Signed-off-by: d-kuro <kurosawa7620@gmail.com>
cybozu-go · Aug 9, 2023 · cc713ec · cc713ec
1 parent e6f9b4d
commit cc713ec
Show file tree

Hide file tree

Showing 7 changed files with 170 additions and 111 deletions.
diff --git a/api/v1beta2/mysqlcluster_types.go b/api/v1beta2/mysqlcluster_types.go
@@ -271,9 +271,9 @@ func (s MySQLClusterSpec) validateUpdate(ctx context.Context, apiReader client.R
 
 			switch cmp := newSize.Cmp(oldSize); {
 			case cmp == -1:
-				p := p.Child("volumeClaimTemplates").Index(i).
-					Child("spec").Child("resources").Child("requests").Key("storage")
-				allErrs = append(allErrs, field.Forbidden(p, "storage size cannot be reduced"))
+				// noop
+				// Allow users to reduce the volume size by operating.
+				// ref: docs/designdoc/support_reduce_volume_size.md
 			case cmp == 1:
 				volumeExpansionTargetIndices = append(volumeExpansionTargetIndices, i)
 			case cmp == 0:

diff --git a/api/v1beta2/mysqlcluster_webhook_test.go b/api/v1beta2/mysqlcluster_webhook_test.go
@@ -498,25 +498,6 @@ var _ = Describe("MySQLCluster Webhook", func() {
 		Expect(err).NotTo(HaveOccurred())
 	})
 
-	It("should deny reduced storage size", func() {
-		r := makeMySQLCluster()
-		err := k8sClient.Create(ctx, r)
-		Expect(err).NotTo(HaveOccurred())
-
-		r.Spec.VolumeClaimTemplates[0].Spec = mocov1beta2.PersistentVolumeClaimSpecApplyConfiguration(
-			*corev1ac.PersistentVolumeClaimSpec().
-				WithStorageClassName("default").
-				WithResources(corev1ac.ResourceRequirements().
-					WithRequests(corev1.ResourceList{
-						corev1.ResourceStorage: resource.MustParse("1Mi"),
-					}),
-				),
-		)
-
-		err = k8sClient.Update(ctx, r)
-		Expect(err).To(HaveOccurred())
-	})
-
 	It("should deny storage size expansion for not support volume expansion storage class", func() {
 		r := makeMySQLCluster()
 		r.Spec.VolumeClaimTemplates = make([]mocov1beta2.PersistentVolumeClaim, 2)

diff --git a/controllers/pvc.go b/controllers/pvc.go
@@ -174,7 +174,12 @@ func (*MySQLClusterReconciler) needResizePVC(cluster *mocov1beta2.MySQLCluster,
 		case i == 0: // volume size is equal
 			continue
 		case i == 1: // volume size is greater
-			return nil, false, fmt.Errorf("failed to resize pvc %q, want size: %s, deployed size: %s: %w", pvc.Name, wantSize, deployedSize, ErrReduceVolumeSize)
+			// Due to the lack of support for volume size reduction, resizing will not be executed if it implies a smaller size.
+			// It's important to highlight that this does not induce an error.
+			// Instead, the recreation of the StatefulSet will be managed in the reconcileV1StatefulSet() operation, which follows this one.
+			// Hence, the execution flow remains uninterrupted.
+			// ref: docs/designdoc/support_reduce_volume_size.md
+			continue
 		case i == -1: // volume size is smaller
 			resizeTarget[pvc.Name] = pvcSet[pvc.Name]
 			continue

diff --git a/e2e/pvc_test.go b/e2e/pvc_test.go
@@ -6,15 +6,12 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
-	"reflect"
 
 	mocov1beta2 "github.com/cybozu-go/moco/api/v1beta2"
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
 	"github.com/prometheus/common/expfmt"
-	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
-	"k8s.io/apimachinery/pkg/api/resource"
 )
 
 //go:embed testdata/pvc_test.yaml
@@ -46,6 +43,7 @@ var _ = Context("pvc_test", func() {
 	})
 
 	It("should pvc template change succeed", func() {
+		// 500Mi -> 1Gi
 		kubectlSafe(fillTemplate(pvcApplyYAML), "apply", "-f", "-")
 		Eventually(func() error {
 			cluster, err := getCluster("pvc", "cluster")
@@ -66,107 +64,71 @@ var _ = Context("pvc_test", func() {
 	})
 
 	It("should statefulset re-created", func() {
-		cluster, err := getCluster("pvc", "cluster")
-		Expect(err).NotTo(HaveOccurred())
-
-		wantLabels := make(map[string]map[string]string)
-		for _, pvc := range cluster.Spec.VolumeClaimTemplates {
-			wantLabels[pvc.Name] = pvc.ObjectMeta.Labels
-		}
+		comparePVCTemplate("pvc", "cluster")
+	})
 
-		wantSizes := make(map[string]*resource.Quantity)
-		for _, pvc := range cluster.Spec.VolumeClaimTemplates {
-			wantSizes[pvc.Name] = pvc.Spec.Resources.Requests.Storage()
-		}
+	It("should pvc resized", func() {
+		comparePVCSize("pvc", "cluster")
+	})
 
+	It("should pvc template storage size reduce succeed", func() {
+		// 1Gi -> 500Mi
+		kubectlSafe(fillTemplate(pvcTestYAML), "apply", "-f", "-")
 		Eventually(func() error {
-			out, err := kubectl(nil,
-				"get", "sts",
-				"-n", "pvc",
-				"moco-cluster",
-				"-o", "json",
-			)
+			cluster, err := getCluster("pvc", "cluster")
 			if err != nil {
 				return err
 			}
-
-			var sts appsv1.StatefulSet
-			if err := json.Unmarshal(out, &sts); err != nil {
-				return err
-			}
-
-			for _, pvc := range sts.Spec.VolumeClaimTemplates {
-				labels, ok := wantLabels[pvc.Name]
-				if !ok {
-					return fmt.Errorf("pvc %s is not expected", pvc.Name)
-				}
-
-				if !reflect.DeepEqual(pvc.ObjectMeta.Labels, labels) {
-					return fmt.Errorf("pvc %s labels are not expected", pvc.Name)
-				}
-
-				want, ok := wantSizes[pvc.Name]
-				if !ok {
-					return fmt.Errorf("pvc %s is not expected", pvc.Name)
+			for _, cond := range cluster.Status.Conditions {
+				if cond.Type != mocov1beta2.ConditionHealthy {
+					continue
 				}
-
-				if pvc.Spec.Resources.Requests.Storage().Cmp(*want) != 0 {
-					return fmt.Errorf("pvc %s is not expected size: %s", pvc.Name, pvc.Spec.Resources.Requests.Storage())
+				if cond.Status == corev1.ConditionTrue {
+					return nil
 				}
+				return fmt.Errorf("cluster is not healthy: %s", cond.Status)
 			}
-
-			return nil
+			return errors.New("no health condition")
 		}).Should(Succeed())
 	})
 
-	It("should pvc resized", func() {
-		cluster, err := getCluster("pvc", "cluster")
-		Expect(err).NotTo(HaveOccurred())
-
-		wantSizes := make(map[string]*resource.Quantity)
-		for _, pvc := range cluster.Spec.VolumeClaimTemplates {
-			for i := int32(0); i < cluster.Spec.Replicas; i++ {
-				name := fmt.Sprintf("%s-%s-%d", pvc.Name, "moco-cluster", i)
-				wantSizes[name] = pvc.Spec.Resources.Requests.Storage()
-			}
-		}
-
-		Eventually(func() error {
-			out, err := kubectl(nil,
-				"get", "pvc",
-				"-n", "pvc",
-				"-l", "app.kubernetes.io/instance=cluster",
-				"-o", "json",
-			)
-			if err != nil {
-				return err
-			}
+	It("should statefulset re-created", func() {
+		comparePVCTemplate("pvc", "cluster")
+	})
 
-			var pvcList corev1.PersistentVolumeClaimList
-			if err := json.Unmarshal(out, &pvcList); err != nil {
-				return err
-			}
-			if len(pvcList.Items) < 1 {
-				return errors.New("not found pvcs")
-			}
+	It("should volume size reduce succeed", func() {
+		out := kubectlSafe(nil, "get", "pods", "-n", "pvc", "-o", "json")
+		pods := &corev1.PodList{}
+		err := json.Unmarshal(out, pods)
+		Expect(err).NotTo(HaveOccurred())
+		Expect(pods.Items).To(HaveLen(3))
 
-			if len(pvcList.Items) != len(wantSizes) {
-				return fmt.Errorf("pvc count is not expected: %d", len(pvcList.Items))
-			}
+		for _, pod := range pods.Items {
+			kubectlSafe(nil, "delete", "pvc", "-n", "pvc", "--wait=false", fmt.Sprintf("mysql-data-%s", pod.Name))
+			kubectlSafe(nil, "delete", "pod", "-n", "pvc", "--grace-period=1", pod.Name)
 
-			for _, pvc := range pvcList.Items {
-				want, ok := wantSizes[pvc.Name]
-				if !ok {
-					return fmt.Errorf("pvc %s is not expected", pvc.Name)
+			Eventually(func() error {
+				cluster, err := getCluster("pvc", "cluster")
+				if err != nil {
+					return err
 				}
 
-				if pvc.Spec.Resources.Requests.Storage().Cmp(*want) != 0 {
-					return fmt.Errorf("pvc %s is not expected size: %s", pvc.Name, pvc.Spec.Resources.Requests.Storage())
+				for _, cond := range cluster.Status.Conditions {
+					if cond.Type != mocov1beta2.ConditionHealthy {
+						continue
+					}
+					if cond.Status == corev1.ConditionTrue {
+						return nil
+					}
+					return fmt.Errorf("cluster is not healthy: %s", cond.Status)
 				}
-			}
+				return errors.New("no health condition")
+			}).Should(Succeed())
+		}
+	})
 
-			return nil
-		}).Should(Succeed())
+	It("should pvc resized", func() {
+		comparePVCSize("pvc", "cluster")
 	})
 
 	It("metrics", func() {
@@ -192,7 +154,7 @@ var _ = Context("pvc_test", func() {
 		Expect(stsMf).NotTo(BeNil())
 		stsMetric := findMetric(stsMf, map[string]string{"namespace": "pvc", "name": "cluster"})
 		Expect(stsMetric).NotTo(BeNil())
-		Expect(stsMetric.GetCounter().GetValue()).To(BeNumerically("==", 1))
+		Expect(stsMetric.GetCounter().GetValue()).To(BeNumerically("==", 2))
 	})
 
 	It("should delete clusters", func() {

diff --git a/e2e/run_test.go b/e2e/run_test.go
@@ -3,13 +3,20 @@ package e2e
 import (
 	"bytes"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"os/exec"
+	"reflect"
 	"text/template"
 
+	corev1 "k8s.io/api/core/v1"
+
+	appsv1 "k8s.io/api/apps/v1"
+
 	mocov1beta2 "github.com/cybozu-go/moco/api/v1beta2"
 	. "github.com/onsi/gomega"
 	dto "github.com/prometheus/client_model/go"
+	"k8s.io/apimachinery/pkg/api/resource"
 )
 
 func kubectl(input []byte, args ...string) ([]byte, error) {
@@ -82,3 +89,107 @@ OUTER:
 	}
 	return nil
 }
+
+func comparePVCTemplate(ns string, clusterName string) {
+	cluster, err := getCluster(ns, clusterName)
+	Expect(err).NotTo(HaveOccurred())
+
+	wantLabels := make(map[string]map[string]string)
+	for _, pvc := range cluster.Spec.VolumeClaimTemplates {
+		wantLabels[pvc.Name] = pvc.ObjectMeta.Labels
+	}
+
+	wantSizes := make(map[string]*resource.Quantity)
+	for _, pvc := range cluster.Spec.VolumeClaimTemplates {
+		wantSizes[pvc.Name] = pvc.Spec.Resources.Requests.Storage()
+	}
+
+	Eventually(func() error {
+		out, err := kubectl(nil,
+			"get", "sts",
+			"-n", ns,
+			fmt.Sprintf("moco-%s", clusterName),
+			"-o", "json",
+		)
+		if err != nil {
+			return err
+		}
+
+		var sts appsv1.StatefulSet
+		if err := json.Unmarshal(out, &sts); err != nil {
+			return err
+		}
+
+		for _, pvc := range sts.Spec.VolumeClaimTemplates {
+			labels, ok := wantLabels[pvc.Name]
+			if !ok {
+				return fmt.Errorf("pvc %s is not expected", pvc.Name)
+			}
+
+			if !reflect.DeepEqual(pvc.ObjectMeta.Labels, labels) {
+				return fmt.Errorf("pvc %s labels are not expected", pvc.Name)
+			}
+
+			want, ok := wantSizes[pvc.Name]
+			if !ok {
+				return fmt.Errorf("pvc %s is not expected", pvc.Name)
+			}
+
+			if pvc.Spec.Resources.Requests.Storage().Cmp(*want) != 0 {
+				return fmt.Errorf("pvc %s is not expected size: %s", pvc.Name, pvc.Spec.Resources.Requests.Storage())
+			}
+		}
+
+		return nil
+	}).Should(Succeed())
+}
+
+func comparePVCSize(ns string, clusterName string) {
+	cluster, err := getCluster(ns, clusterName)
+	Expect(err).NotTo(HaveOccurred())
+
+	wantSizes := make(map[string]*resource.Quantity)
+	for _, pvc := range cluster.Spec.VolumeClaimTemplates {
+		for i := int32(0); i < cluster.Spec.Replicas; i++ {
+			name := fmt.Sprintf("%s-moco-%s-%d", pvc.Name, clusterName, i)
+			wantSizes[name] = pvc.Spec.Resources.Requests.Storage()
+		}
+	}
+
+	Eventually(func() error {
+		out, err := kubectl(nil,
+			"get", "pvc",
+			"-n", ns,
+			"-l", fmt.Sprintf("app.kubernetes.io/instance=%s", clusterName),
+			"-o", "json",
+		)
+		if err != nil {
+			return err
+		}
+
+		var pvcList corev1.PersistentVolumeClaimList
+		if err := json.Unmarshal(out, &pvcList); err != nil {
+			return err
+		}
+		if len(pvcList.Items) < 1 {
+			return errors.New("not found pvcs")
+		}
+
+		if len(pvcList.Items) != len(wantSizes) {
+			return fmt.Errorf("pvc count is not expected: %d", len(pvcList.Items))
+		}
+
+		for _, pvc := range pvcList.Items {
+			want, ok := wantSizes[pvc.Name]
+			if !ok {
+				return fmt.Errorf("pvc %s is not expected", pvc.Name)
+			}
+
+			if pvc.Spec.Resources.Requests.Storage().Cmp(*want) != 0 {
+				return fmt.Errorf("pvc %s is not expected size: %s", pvc.Name, pvc.Spec.Resources.Requests.Storage())
+			}
+		}
+
+		return nil
+	}).Should(Succeed())
+}
diff --git a/e2e/testdata/pvc_test.yaml b/e2e/testdata/pvc_test.yaml
@@ -26,7 +26,7 @@ metadata:
   namespace: pvc
   name: cluster
 spec:
-  replicas: 1
+  replicas: 3
   mysqlConfigMapName: mycnf
   podTemplate:
     spec:

diff --git a/e2e/testdata/pvc_test_changed.yaml b/e2e/testdata/pvc_test_changed.yaml
@@ -4,7 +4,7 @@ metadata:
   namespace: pvc
   name: cluster
 spec:
-  replicas: 1
+  replicas: 3
   mysqlConfigMapName: mycnf
   podTemplate:
     spec: