Resizing PVC #403

d-kuro · 2022-04-20T04:27:08Z

refs: #265

Add enhancement to ｓupport applying PVC template changes to StatefulSet.
See #412 for design docs.

This PR implements the core functionality, but the following elements are done in a separate pull request

Add E2E tests
Documentation Updates

Manually tested before adding E2E test:

apply mysqlcluster

$ cat mycluster.yaml
apiVersion: moco.cybozu.com/v1beta1
kind: MySQLCluster
metadata:
  namespace: default
  name: test
spec:
  replicas: 3
  podTemplate:
    spec:
      containers:
      - name: mysqld
        image: quay.io/cybozu/mysql:8.0.28
  volumeClaimTemplates:
  - metadata:
      name: mysql-data
    spec:
      accessModes: [ "ReadWriteOnce" ]
      resources:
        requests:
          storage: 1Gi

$ kubectl apply -f mycluster.yaml
mysqlcluster.moco.cybozu.com/test created

check PVC

$ kubectl get pvc -o yaml | grep -C 2 requests
    - ReadWriteOnce
    resources:
      requests:
        storage: 1Gi
    storageClassName: standard
--
    - ReadWriteOnce
    resources:
      requests:
        storage: 1Gi
    storageClassName: standard
--
    - ReadWriteOnce
    resources:
      requests:
        storage: 1Gi
    storageClassName: standard

check statefulset

apiVersion: apps/v1
kind: StatefulSet
metadata:
  name: moco-test
  namespace: default
spec:
<snip>
  volumeClaimTemplates:
  - apiVersion: v1
    kind: PersistentVolumeClaim
    metadata:
      name: mysql-data
    spec:
      accessModes:
      - ReadWriteOnce
      resources:
        requests:
          storage: 1Gi
      volumeMode: Filesystem

edit mysqlcluster

  apiVersion: moco.cybozu.com/v1beta1
  kind: MySQLCluster
  metadata:
    namespace: default
    name: test
  spec:
    replicas: 3
    podTemplate:
      spec:
        containers:
        - name: mysqld
          image: quay.io/cybozu/mysql:8.0.28
    volumeClaimTemplates:
    - metadata:
        name: mysql-data
+       labels:
+         foo: bar
      spec:
        accessModes: [ "ReadWriteOnce" ]
        resources:
          requests:
+           storage: 2Gi
-           storage: 1Gi

check PVC

$ kubectl get pvc -o yaml | grep -C 2 requests
    - ReadWriteOnce
    resources:
      requests:
        storage: 2Gi
    storageClassName: standard
--
    - ReadWriteOnce
    resources:
      requests:
        storage: 2Gi
    storageClassName: standard
--
    - ReadWriteOnce
    resources:
      requests:
        storage: 2Gi
    storageClassName: standard

check sts

apiVersion: apps/v1
kind: StatefulSet
metadata:
  name: moco-test
  namespace: default
spec:
<snip>
  volumeClaimTemplates:
  - apiVersion: v1
    kind: PersistentVolumeClaim
    metadata:
      labels:
        foo: bar
      name: mysql-data
    spec:
      accessModes:
      - ReadWriteOnce
      resources:
        requests:
          storage: 2Gi
      volumeMode: Filesystem

controllers/resize_pvc.go

ymmt2005 · 2022-04-26T20:38:32Z

controllers/resize_pvc.go

+		pvcac := corev1ac.PersistentVolumeClaim(pvc.Name, pvc.Namespace).
+			WithSpec(corev1ac.PersistentVolumeClaimSpec().
+				WithResources(corev1ac.ResourceRequirements().
+					WithRequests(corev1.ResourceList{corev1.ResourceStorage: *newSize}),


Is there possibility that the current size is already bigger than the new size?
For example, with the help of pvc-autoresizer.

Sorry for the late reply, I have created a design document to discuss the specifications
Please review 🙏
#412

@d-kuro Thank you for the document. LGTM.

ymmt2005 · 2022-05-18T05:22:57Z

@d-kuro Could you extend the scope of the original issue to include cases where users might edit metadata of volumeClaimTemplates?

d-kuro · 2022-05-18T05:29:10Z

@ymmt2005
I understood.
I will fix the code and design document 🙏

Signed-off-by: d-kuro <[email protected]>

d-kuro · 2022-06-08T03:34:32Z

controllers/mysqlcluster_controller.go

+			opt := metav1.DeletePropagationOrphan
+			if err := r.Delete(ctx, &orig, &client.DeleteOptions{
+				PropagationPolicy: &opt,


envtest is not able to test using DeletePropagationOrphan because kube-controller-manager does not exist.
Will verify when adding the E2E test.

d-kuro · 2022-06-08T03:35:15Z

controllers/pvc_test.go

+	}
+}
+
+func setupMockClient(t *testing.T, cluster *mocov1beta2.MySQLCluster, sts *appsv1.StatefulSet) client.Client {


In the env test, controller-manager is not running, so testing with child resources is not possible (e.g. StatefulSet's child PVCs).
Therefore, the mock client is used.

I will add the E2E test later.

Signed-off-by: d-kuro <[email protected]>

d-kuro · 2022-06-08T04:23:55Z

@ymmt2005 @masa213f

I fixed codes corrections based on the design document (#412)
Please re-review 🙏

masa213f · 2022-06-16T05:10:30Z

controllers/pvc.go

+		return true
+	}
+
+	if sts.Generation > sts.Status.ObservedGeneration && *sts.Spec.Replicas == sts.Status.Replicas {


Could you tell me the purpose of this condition(*sts.Spec.Replicas == sts.Status.Replicas)?
I cannot understand this condition just by reading this code.

I looked at the code that kubectl is doing to determine if the StatefulSet is in the process of updating, and it seems that this condition is incorrect.
I will fix it.

https://github.com/kubernetes/kubectl/blob/master/pkg/polymorphichelpers/rollout_status.go#L119-L152

fixed: 8335537

Thank you for the update.
Could you write the URL which links to the referenced code as a comment?

In the original code, sts.Status.ObservedGeneration == 0 is determined to be in updating.
The decision seems to be different from MOCO's. Is this intentional?

Sorry, I had omitted the Object condition for testing purposes, and it was still there.
Fixed: 8bddef7

masa213f · 2022-06-16T05:43:12Z

controllers/pvc.go

+
+	for _, pvc := range cluster.Spec.VolumeClaimTemplates {
+		if _, ok := pvcSet[pvc.Name]; !ok {
+			delete(pvcSet, pvc.Name)


This delete is unnecessary.

Thanks, I found another problem, so I fixed it and added a test.
eb8ec57

masa213f · 2022-06-17T02:47:52Z

api/v1beta2/mysqlcluster_types.go

@@ -242,6 +243,24 @@ func (s MySQLClusterSpec) validateUpdate(old MySQLClusterSpec) field.ErrorList {
 		allErrs = append(allErrs, field.Forbidden(p, "not editable"))
 	}

+	oldPVCSet := make(map[string]PersistentVolumeClaim)


Could you confirm the existance of resources and resources.requests.storage in the pvcTemplates?

When the resources is missing:

apiVersion: moco.cybozu.com/v1beta2 kind: MySQLCluster ... spec: ... volumeClaimTemplates: - metadata: name: mysql-data spec: accessModes: [ "ReadWriteOnce" ] # resources: # requests: # storage: 1Gi logRotationSchedule: "@every 10s"

The controller will be panic.

panic: runtime error: invalid memory address or nil pointer dereference [signal SIGSEGV: segmentation violation code=0x1 addr=0x8 pc=0x150f2c2] goroutine 475 [running]: github.com/cybozu-go/moco/controllers.(*MySQLClusterReconciler).needResizePVC(0x0, 0xc000367900, 0xc0005cef00) /work/controllers/pvc.go:159 +0x2e2 github.com/cybozu-go/moco/controllers.(*MySQLClusterReconciler).reconcilePVC(0xc0005ca000, {0x1af42d8, 0xc0002bc450}, {{{0xc000a51a67, 0x0}, {0xc000a51aa0, 0xc000a51aa0}}}, 0xc000367900) /work/controllers/pvc.go:51 +0x2f0 github.com/cybozu-go/moco/controllers.(*MySQLClusterReconciler).reconcileV1(0xc0005ca000, {0x1af42d8, 0xc0002bc450}, {{{0xc000a51a67, 0xc000a51aa0}, {0xc000a51aa0, 0x1b42ab0}}}, 0xc000367900) /work/controllers/mysqlcluster_controller.go:233 +0x585 github.com/cybozu-go/moco/controllers.(*MySQLClusterReconciler).Reconcile(0xc0005ca000, {0x1af42d8, 0xc0002bc450}, {{{0xc000a51a67, 0x1791b80}, {0xc000a51aa0, 0x30}}}) /work/controllers/mysqlcluster_controller.go:168 +0x1c7 sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).Reconcile(0xc0004c29a0, {0x1af42d8, 0xc0002bc330}, {{{0xc000a51a67, 0x1791b80}, {0xc000a51aa0, 0x413974}}}) /go/pkg/mod/sigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:114 +0x26f sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).reconcileHandler(0xc0004c29a0, {0x1af4230, 0xc0005aa0c0}, {0x16ce7e0, 0xc000acaf40}) /go/pkg/mod/sigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:311 +0x33e sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).processNextWorkItem(0xc0004c29a0, {0x1af4230, 0xc0005aa0c0}) /go/pkg/mod/sigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:266 +0x205 sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).Start.func2.2() /go/pkg/mod/sigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:227 +0x85 created by sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).Start.func2 /go/pkg/mod/sigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:223 +0x357

When the resources.requests.storage is missing:

volumeClaimTemplates: - metadata: name: mysql-data spec: accessModes: [ "ReadWriteOnce" ] resources: requests: storoge: 1Gi # typo

The StatefulSet controller cannot create PVC.

Events: Type Reason Age From Message ---- ------ ---- ---- ------- Warning FailedCreate 29s (x12 over 40s) statefulset-controller create Pod moco-test-0 in StatefulSet moco-test failed error: failed to create PVC mysql-data-moco-test-0: PersistentVolumeClaim "mysql-data-moco-test-0" is invalid: spec.resources[storage]: Required value Warning FailedCreate 19s (x13 over 40s) statefulset-controller create Claim mysql-data-moco-test-0 for Pod moco-test-0 in StatefulSet moco-test failed error: PersistentVolumeClaim "mysql-data-moco-test-0" is invalid: spec.resources[storage]: Required value

I fixed validation webhook to validate storage size.
a6de68a

Signed-off-by: d-kuro <[email protected]>

masa213f

Sorry for the late replay. Please respond #403 (comment).
Other than it, it's fine.

Signed-off-by: d-kuro <[email protected]>

masa213f

LGTM

d-kuro self-assigned this Apr 20, 2022

d-kuro force-pushed the d-kuro/resize-pvc branch from 4ef2717 to 35ea861 Compare April 26, 2022 15:51

ymmt2005 reviewed Apr 26, 2022

View reviewed changes

controllers/resize_pvc.go Outdated Show resolved Hide resolved

ymmt2005 reviewed Apr 26, 2022

View reviewed changes

d-kuro force-pushed the d-kuro/resize-pvc branch 2 times, most recently from 20273d6 to 87b3539 Compare May 18, 2022 04:27

d-kuro changed the title ~~WIP: Resizing PVC~~ Resizing PVC May 18, 2022

d-kuro requested review from masa213f, zoetrope and ymmt2005 May 18, 2022 04:31

d-kuro marked this pull request as ready for review May 18, 2022 04:32

d-kuro added 7 commits June 5, 2022 23:20

Resizing PVC

45ff735

Signed-off-by: d-kuro <[email protected]>

Add RBAC

295f177

Signed-off-by: d-kuro <[email protected]>

Check support volume expansion

9a30608

Signed-off-by: d-kuro <[email protected]>

Fix linter erorrs

6a5e94b

Signed-off-by: d-kuro <[email protected]>

Add resize pvcs tests.

bfbcdef

Recreate statefulsets when pvc template chganges

aca07ed

Signed-off-by: d-kuro <[email protected]>

Export metrics for pvc extensions and sts recreation.

cb806e3

Signed-off-by: d-kuro <[email protected]>

d-kuro force-pushed the d-kuro/resize-pvc branch from 87b3539 to cb806e3 Compare June 8, 2022 02:02

d-kuro added 3 commits June 8, 2022 11:04

Rename designdoc for Support apply PVC template changes to StatefulSet

e82e472

Signed-off-by: d-kuro <[email protected]>

Deny storage size reduction

8177a44

Signed-off-by: d-kuro <[email protected]>

Remove unused method.

d79783d

Signed-off-by: d-kuro <[email protected]>

d-kuro commented Jun 8, 2022

View reviewed changes

d-kuro added 2 commits June 8, 2022 12:37

Update godoc

2481b1d

Signed-off-by: d-kuro <[email protected]>

Fix RBAC manifests

d2047f7

Signed-off-by: d-kuro <[email protected]>

masa213f requested changes Jun 17, 2022

View reviewed changes

d-kuro added 3 commits June 22, 2022 13:36

Fix update checks in statefulset

8335537

Signed-off-by: d-kuro <[email protected]>

Require storage size in PVC template

a6de68a

Signed-off-by: d-kuro <[email protected]>

Fix find resize target.

eb8ec57

Signed-off-by: d-kuro <[email protected]>

d-kuro force-pushed the d-kuro/resize-pvc branch from 4e86677 to 5ee1909 Compare June 22, 2022 04:38

Wait sts delete.

0a1fc69

Signed-off-by: d-kuro <[email protected]>

d-kuro force-pushed the d-kuro/resize-pvc branch from 5ee1909 to 0a1fc69 Compare June 22, 2022 04:40

d-kuro requested a review from masa213f June 22, 2022 06:22

masa213f reviewed Jun 28, 2022

View reviewed changes

Fix statefulset mock object.

8bddef7

Signed-off-by: d-kuro <[email protected]>

d-kuro requested a review from masa213f June 28, 2022 21:10

d-kuro mentioned this pull request Jun 29, 2022

Added e2e test to change pvc template #417

Merged

masa213f approved these changes Jun 29, 2022

View reviewed changes

masa213f merged commit 7cbca53 into main Jun 29, 2022

masa213f deleted the d-kuro/resize-pvc branch June 29, 2022 05:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Resizing PVC #403

Resizing PVC #403

d-kuro commented Apr 20, 2022 •

edited

Loading

ymmt2005 Apr 26, 2022

d-kuro May 14, 2022

ymmt2005 May 16, 2022

ymmt2005 commented May 18, 2022

d-kuro commented May 18, 2022

d-kuro Jun 8, 2022

d-kuro Jun 8, 2022

d-kuro commented Jun 8, 2022

masa213f Jun 16, 2022

d-kuro Jun 18, 2022

d-kuro Jun 22, 2022

masa213f Jun 28, 2022

d-kuro Jun 28, 2022

masa213f Jun 16, 2022

d-kuro Jun 22, 2022

masa213f Jun 17, 2022

d-kuro Jun 22, 2022

masa213f left a comment

masa213f left a comment

Resizing PVC #403

Resizing PVC #403

Conversation

d-kuro commented Apr 20, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ymmt2005 commented May 18, 2022

d-kuro commented May 18, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

d-kuro commented Jun 8, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

When the resources is missing:

When the resources.requests.storage is missing:

Choose a reason for hiding this comment

masa213f left a comment

Choose a reason for hiding this comment

masa213f left a comment

Choose a reason for hiding this comment

d-kuro commented Apr 20, 2022 •

edited

Loading

When the `resources` is missing:

When the `resources.requests.storage` is missing: