Skip to content

Commit aadd150

Browse files
satoru-takeuchisatoru-takeuchi
authored
Merge pull request #103 from cybozu-go/dont-allow-resize
stop backup if target PV(C) is resized
2 parents 42bc2a0 + c0f7855 commit aadd150

File tree

7 files changed

+168
-8
lines changed

7 files changed

+168
-8
lines changed

‎internal/ceph/ceph.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ func (t *RBDTimeStamp) UnmarshalJSON(data []byte) error {
3333
type RBDSnapshot struct {
3434
Id int `json:"id,omitempty"`
3535
Name string `json:"name,omitempty"`
36-
Size int `json:"size,omitempty"`
36+
Size int64 `json:"size,omitempty"`
3737
Protected bool `json:"protected,string,omitempty"`
3838
Timestamp RBDTimeStamp `json:"timestamp,omitempty"`
3939
}

‎internal/ceph/rbd_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -242,7 +242,7 @@ var _ = Describe("CephCmd.RBDSnapLs", func() {
242242
snap := snaps[0]
243243
Expect(snap.Id).To(Equal(int(4)))
244244
Expect(snap.Name).To(Equal("test"))
245-
Expect(snap.Size).To(Equal(int(10737418240)))
245+
Expect(snap.Size).To(Equal(int64(10737418240)))
246246
Expect(snap.Protected).To(Equal(false))
247247
Expect(snap.Timestamp).To(Equal(NewRBDTimeStamp(time.Date(2024, 10, 1, 10, 11, 31, 0, time.UTC))))
248248
})

‎internal/controller/internal/testutil/fake_rbd.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ func (f *fakeRBD) RBDSnapCreate(pool, image, snap string) error {
5858
f.snapshots[key] = append(snaps, ceph.RBDSnapshot{
5959
Id: f.nextSnapId,
6060
Name: snap,
61-
Size: 10,
61+
Size: 5368709120, // 5Gi
6262
Protected: false,
6363
Timestamp: ceph.NewRBDTimeStamp(time.Now().UTC()),
6464
})

‎internal/controller/internal/testutil/resources.go

+10-5
Original file line numberDiff line numberDiff line change
@@ -81,13 +81,18 @@ func (r *ResourceManager) CreateStorageClass(ctx context.Context) error {
8181
// CreateUniquePVAndPVC creates a unique named PV and PVC. The PV is bound to the PVC.
8282
func (r *ResourceManager) CreateUniquePVAndPVC(ctx context.Context, ns string) (
8383
*corev1.PersistentVolume, *corev1.PersistentVolumeClaim, error) {
84-
return r.createPVAndPVC(ctx, ns, util.GetUniqueName("pv-"), util.GetUniqueName("pvc-"))
84+
return r.createPVAndPVC(ctx, ns, util.GetUniqueName("pv-"), util.GetUniqueName("pvc-"),
85+
resource.MustParse("5Gi"), resource.MustParse("1Gi"))
8586
}
8687

87-
func (r *ResourceManager) createPVAndPVC(ctx context.Context, ns, pvName, pvcName string) (
88+
func (r *ResourceManager) CreateUniquePVAndPVCSized(ctx context.Context, ns string, pvSize, pvcSize resource.Quantity) (
89+
*corev1.PersistentVolume, *corev1.PersistentVolumeClaim, error) {
90+
return r.createPVAndPVC(ctx, ns, util.GetUniqueName("pv-"), util.GetUniqueName("pvc-"), pvSize, pvcSize)
91+
}
92+
93+
func (r *ResourceManager) createPVAndPVC(ctx context.Context, ns, pvName, pvcName string, pvSize, pvcSize resource.Quantity) (
8894
*corev1.PersistentVolume, *corev1.PersistentVolumeClaim, error) {
8995
accessModes := []corev1.PersistentVolumeAccessMode{corev1.ReadWriteOnce}
90-
size := resource.MustParse("1Gi")
9196
volumeMode := corev1.PersistentVolumeFilesystem
9297

9398
pv := corev1.PersistentVolume{
@@ -96,7 +101,7 @@ func (r *ResourceManager) createPVAndPVC(ctx context.Context, ns, pvName, pvcNam
96101
},
97102
Spec: corev1.PersistentVolumeSpec{
98103
AccessModes: accessModes,
99-
Capacity: corev1.ResourceList{corev1.ResourceStorage: resource.MustParse("5Gi")},
104+
Capacity: corev1.ResourceList{corev1.ResourceStorage: pvSize},
100105
PersistentVolumeSource: corev1.PersistentVolumeSource{
101106
CSI: &corev1.CSIPersistentVolumeSource{
102107
Driver: "restore.rbd.csi.ceph.com",
@@ -135,7 +140,7 @@ func (r *ResourceManager) createPVAndPVC(ctx context.Context, ns, pvName, pvcNam
135140
StorageClassName: &r.StorageClassName,
136141
Resources: corev1.VolumeResourceRequirements{
137142
Requests: corev1.ResourceList{
138-
corev1.ResourceStorage: size,
143+
corev1.ResourceStorage: pvcSize,
139144
},
140145
},
141146
VolumeMode: &volumeMode,

‎internal/controller/mantlebackup_controller.go

+55
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"errors"
77
"fmt"
88
"path/filepath"
9+
"slices"
910
"time"
1011

1112
_ "embed"
@@ -628,6 +629,55 @@ func (r *MantleBackupReconciler) replicateManifests(
628629
return ctrl.Result{}, nil
629630
}
630631

632+
func (r *MantleBackupReconciler) checkSnapshotValid(
633+
target *snapshotTarget,
634+
snapshot *ceph.RBDSnapshot,
635+
) error {
636+
msgResizeNotSupported := "resize detected: resizing a PV(C) is not supported in Mantle: "
637+
638+
resizing := slices.ContainsFunc(
639+
target.pvc.Status.Conditions,
640+
func(c corev1.PersistentVolumeClaimCondition) bool {
641+
return c.Type == corev1.PersistentVolumeClaimResizing && c.Status == corev1.ConditionTrue
642+
},
643+
)
644+
if resizing {
645+
return fmt.Errorf("%s: Resizing condition is True", msgResizeNotSupported)
646+
}
647+
648+
pvcSize, ok := target.pvc.Spec.Resources.Requests.Storage().AsInt64()
649+
if !ok {
650+
return errors.New("failed to get PVC size")
651+
}
652+
pvSize, ok := target.pv.Spec.Capacity.Storage().AsInt64()
653+
if !ok {
654+
return errors.New("failed to get PV size")
655+
}
656+
657+
if pvSize < pvcSize {
658+
return fmt.Errorf("%s: pvSize (%d) < PVC size (%d)", msgResizeNotSupported, pvSize, pvcSize)
659+
}
660+
661+
if snapshot.Size != pvSize {
662+
return fmt.Errorf("%s: snapshot size (%d) != PV size (%d)", msgResizeNotSupported, snapshot.Size, pvSize)
663+
}
664+
665+
snapshots, err := r.ceph.RBDSnapLs(target.poolName, target.imageName)
666+
if err != nil {
667+
return fmt.Errorf("failed to list snapshots: %s: %s",
668+
target.poolName, target.imageName)
669+
}
670+
671+
for _, snapshot := range snapshots {
672+
if snapshot.Size != pvSize {
673+
return fmt.Errorf("%s: existing snapshot size (%d) != PV size (%d)",
674+
msgResizeNotSupported, snapshot.Size, pvSize)
675+
}
676+
}
677+
678+
return nil
679+
}
680+
631681
func (r *MantleBackupReconciler) provisionRBDSnapshot(
632682
ctx context.Context,
633683
backup *mantlev1.MantleBackup,
@@ -662,6 +712,11 @@ func (r *MantleBackupReconciler) provisionRBDSnapshot(
662712
return err
663713
}
664714

715+
if err := r.checkSnapshotValid(target, snapshot); err != nil {
716+
return fmt.Errorf("failed to create a valid snapshot: %s/%s: %w",
717+
backup.GetNamespace(), backup.GetName(), err)
718+
}
719+
665720
if err := updateStatus(ctx, r.Client, backup, func() error {
666721
pvcJs, err := json.Marshal(target.pvc)
667722
if err != nil {

‎internal/controller/mantlebackup_controller_test.go

+36
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,22 @@ var _ = Describe("MantleBackup controller", func() {
4949
var ns string
5050
var lastExpireQueuedBackups sync.Map
5151

52+
ensureReadyToUseNotTrue := func(ctx context.Context, backup *mantlev1.MantleBackup) {
53+
GinkgoHelper()
54+
Consistently(ctx, func(g Gomega, ctx context.Context) {
55+
namespacedName := types.NamespacedName{
56+
Namespace: backup.Namespace,
57+
Name: backup.Name,
58+
}
59+
err := k8sClient.Get(ctx, namespacedName, backup)
60+
g.Expect(err).NotTo(HaveOccurred())
61+
condition := meta.FindStatusCondition(backup.Status.Conditions, mantlev1.BackupConditionReadyToUse)
62+
if condition != nil {
63+
g.Expect(condition.Status).NotTo(Equal(metav1.ConditionTrue))
64+
}
65+
}, "10s", "1s").Should(Succeed())
66+
}
67+
5268
waitForBackupNotReady := func(ctx context.Context, backup *mantlev1.MantleBackup) {
5369
EventuallyWithOffset(1, func(g Gomega, ctx context.Context) {
5470
namespacedName := types.NamespacedName{
@@ -337,6 +353,26 @@ var _ = Describe("MantleBackup controller", func() {
337353
err = k8sClient.Create(ctx, backup)
338354
Expect(err).To(HaveOccurred())
339355
})
356+
357+
It("should fail to take a backup if the size of the taken snapshot is not equal to the PVC size", func(ctx SpecContext) {
358+
_, pvc, err := resMgr.CreateUniquePVAndPVCSized(ctx, ns, resource.MustParse("5Gi"), resource.MustParse("10Gi"))
359+
Expect(err).NotTo(HaveOccurred())
360+
361+
backup, err := resMgr.CreateUniqueBackupFor(ctx, pvc)
362+
Expect(err).NotTo(HaveOccurred())
363+
ensureReadyToUseNotTrue(ctx, backup)
364+
})
365+
366+
It("should fail to take a backup if the size of the taken snapshot is not equal to the PV size", func(ctx SpecContext) {
367+
// The snapshot size is fixed to 5Gi in fakeRBD, so we should make
368+
// Mantle fail to take a backup with a PV and PVC of different size.
369+
_, pvc, err := resMgr.CreateUniquePVAndPVCSized(ctx, ns, resource.MustParse("3Gi"), resource.MustParse("5Gi"))
370+
Expect(err).NotTo(HaveOccurred())
371+
372+
backup, err := resMgr.CreateUniqueBackupFor(ctx, pvc)
373+
Expect(err).NotTo(HaveOccurred())
374+
ensureReadyToUseNotTrue(ctx, backup)
375+
})
340376
})
341377

342378
Context("when the role is `primary`", func() {

‎test/e2e/singlek8s/backup_test.go

+64
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ func backupTestSuite() {
5555
Describe("setup environment", test.setupEnv)
5656
Describe("test case 1", test.testCase1)
5757
Describe("test case 2", test.testCase2)
58+
Describe("make sure resizing PV(C)s is not supported", test.testResizingNotSupported)
5859
Describe("teardown environment", test.teardownEnv)
5960
}
6061

@@ -364,3 +365,66 @@ func (test *backupTest) testCase2() {
364365
}).Should(Succeed())
365366
})
366367
}
368+
369+
func (test *backupTest) testResizingNotSupported() {
370+
var err error
371+
372+
waitMantleBackupReadyToUse := func(ctx SpecContext, namespace, name string) {
373+
GinkgoHelper()
374+
Eventually(ctx, func(g Gomega) {
375+
ready, err := isMantleBackupReady(namespace, name)
376+
g.Expect(err).NotTo(HaveOccurred())
377+
g.Expect(ready).To(BeTrue())
378+
}).Should(Succeed())
379+
}
380+
381+
It("should not perform an incremental backup for a PV(C) resized after a previous backup", func(ctx SpecContext) {
382+
namespace := util.GetUniqueName("ns-")
383+
pvcName1 := util.GetUniqueName("pvc-")
384+
mantleBackupName1 := util.GetUniqueName("mb-")
385+
mantleBackupName2 := util.GetUniqueName("mb-")
386+
387+
By("setting up the environment")
388+
err = createNamespace(namespace)
389+
Expect(err).NotTo(HaveOccurred())
390+
391+
By("creating PVC")
392+
err = applyPVCTemplate(namespace, pvcName1, test.storageClassName)
393+
Expect(err).NotTo(HaveOccurred())
394+
395+
By("creating MantleBackup1 for a PVC1")
396+
err = applyMantleBackupTemplate(namespace, pvcName1, mantleBackupName1)
397+
Expect(err).NotTo(HaveOccurred())
398+
waitMantleBackupReadyToUse(ctx, namespace, mantleBackupName1)
399+
400+
By("resizing PVC1")
401+
_, _, err = kubectl("patch", "-n", namespace, "pvc", pvcName1,
402+
"--type=json", `-p=[{"op": "replace", "path": "/spec/resources/requests/storage", "value":"2Gi"}]`)
403+
Expect(err).NotTo(HaveOccurred())
404+
405+
By("making sure PVC1 is resized")
406+
Eventually(ctx, func(g Gomega) {
407+
pvName, err := getPVFromPVC(namespace, pvcName1)
408+
g.Expect(err).NotTo(HaveOccurred())
409+
stdout, _, err := kubectl("get", "pv", pvName, "-o", "json")
410+
g.Expect(err).NotTo(HaveOccurred())
411+
pv := corev1.PersistentVolume{}
412+
err = json.Unmarshal(stdout, &pv)
413+
g.Expect(err).NotTo(HaveOccurred())
414+
capacity, ok := pv.Spec.Capacity.Storage().AsInt64()
415+
g.Expect(ok).To(BeTrue())
416+
g.Expect(capacity).To(Equal(int64(2 * 1024 * 1024 * 1024)))
417+
}).Should(Succeed())
418+
419+
By("creating MantleBackup2 for a PVC1")
420+
err = applyMantleBackupTemplate(namespace, pvcName1, mantleBackupName2)
421+
Expect(err).NotTo(HaveOccurred())
422+
423+
By("confirming MantleBackup2 is not ReadyToUse==True")
424+
Consistently(ctx, func(g Gomega) {
425+
ready, err := isMantleBackupReady(namespace, mantleBackupName2)
426+
g.Expect(err).NotTo(HaveOccurred())
427+
g.Expect(ready).To(BeFalse())
428+
}, "10s", "1s").Should(Succeed())
429+
})
430+
}

0 commit comments

Comments
 (0)