Skip to content

Commit a0fe27f

Browse files
xing-yangxing-yang
committed
Validate volumeMode in CnsRegisterVolume
1 parent 2e7db95 commit a0fe27f

File tree

2 files changed

+352
-0
lines changed

2 files changed

+352
-0
lines changed

‎pkg/syncer/cnsoperator/controller/cnsregistervolume/cnsregistervolume_controller.go‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -534,6 +534,25 @@ func (r *ReconcileCnsRegisterVolume) Reconcile(ctx context.Context,
534534
return reconcile.Result{RequeueAfter: timeout}, nil
535535
}
536536

537+
// If existing PVC has DataSourceRef and volumeMode set, handle volumeMode validation and inheritance
538+
if pvc != nil && pvc.Spec.DataSourceRef != nil && pvc.Spec.VolumeMode != nil {
539+
if instance.Spec.VolumeMode == "" {
540+
// Inherit the volumeMode from the existing PVC
541+
log.Infof("Existing PVC %s in namespace %s has DataSourceRef and volumeMode set to %s. "+
542+
"CnsRegisterVolume does not have volumeMode set. Inheriting volumeMode from existing PVC.",
543+
pvc.Name, pvc.Namespace, *pvc.Spec.VolumeMode)
544+
instance.Spec.VolumeMode = *pvc.Spec.VolumeMode
545+
} else if instance.Spec.VolumeMode != *pvc.Spec.VolumeMode {
546+
// Both are set but don't match - this is an error
547+
msg := fmt.Sprintf("VolumeMode mismatch: existing PVC %s in namespace %s has volumeMode %s, "+
548+
"but CnsRegisterVolume specifies volumeMode %s",
549+
pvc.Name, pvc.Namespace, *pvc.Spec.VolumeMode, instance.Spec.VolumeMode)
550+
log.Error(msg)
551+
setInstanceError(ctx, r, instance, msg)
552+
return reconcile.Result{RequeueAfter: timeout}, nil
553+
}
554+
}
555+
537556
// Do this check before creating a PV. Otherwise, PVC will be bound to PV after PV
538557
// is created even if validation fails
539558
if pvc != nil {

‎pkg/syncer/cnsoperator/controller/cnsregistervolume/cnsregistervolume_controller_test.go‎

Lines changed: 333 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -796,6 +796,39 @@ var _ = Describe("checkExistingPVCDataSourceRef", func() {
796796
Expect(pvc).To(BeNil())
797797
})
798798
})
799+
800+
Context("when PVC exists with DataSourceRef and volumeMode set", func() {
801+
BeforeEach(func() {
802+
apiGroup := "vmoperator.vmware.com"
803+
volumeMode := corev1.PersistentVolumeBlock
804+
pvc := &corev1.PersistentVolumeClaim{
805+
ObjectMeta: metav1.ObjectMeta{
806+
Name: pvcName,
807+
Namespace: namespace,
808+
},
809+
Spec: corev1.PersistentVolumeClaimSpec{
810+
DataSourceRef: &corev1.TypedObjectReference{
811+
APIGroup: &apiGroup,
812+
Kind: "VirtualMachine",
813+
Name: "test-vm",
814+
},
815+
VolumeMode: &volumeMode,
816+
},
817+
}
818+
_, err := k8sclient.CoreV1().PersistentVolumeClaims(namespace).Create(ctx, pvc, metav1.CreateOptions{})
819+
Expect(err).To(BeNil())
820+
})
821+
822+
It("should return the PVC with volumeMode set", func() {
823+
pvc, err := checkExistingPVCDataSourceRef(ctx, k8sclient, pvcName, namespace)
824+
Expect(err).To(BeNil())
825+
Expect(pvc).ToNot(BeNil())
826+
Expect(pvc.Name).To(Equal(pvcName))
827+
Expect(pvc.Spec.DataSourceRef).ToNot(BeNil())
828+
Expect(pvc.Spec.VolumeMode).ToNot(BeNil())
829+
Expect(*pvc.Spec.VolumeMode).To(Equal(corev1.PersistentVolumeBlock))
830+
})
831+
})
799832
})
800833

801834
var _ = Describe("validatePVCTopologyCompatibility", func() {
@@ -1360,3 +1393,303 @@ func TestGetPersistentVolumeSpecWithVolumeMode(t *testing.T) {
13601393
int64(capacity), accessMode, volumeMode, scName, nil)
13611394
assert.Equal(t, volumeMode, *pv.Spec.VolumeMode)
13621395
}
1396+
1397+
func TestVolumeModeInheritanceFromExistingPVCWithDataSourceRef(t *testing.T) {
1398+
ctx := context.Background()
1399+
k8sclient := k8sfake.NewSimpleClientset()
1400+
namespace := "test-namespace"
1401+
pvcName := "test-pvc"
1402+
1403+
// Create a PVC with DataSourceRef and volumeMode set to Block
1404+
apiGroup := "vmoperator.vmware.com"
1405+
volumeMode := corev1.PersistentVolumeBlock
1406+
pvc := &corev1.PersistentVolumeClaim{
1407+
ObjectMeta: metav1.ObjectMeta{
1408+
Name: pvcName,
1409+
Namespace: namespace,
1410+
},
1411+
Spec: corev1.PersistentVolumeClaimSpec{
1412+
DataSourceRef: &corev1.TypedObjectReference{
1413+
APIGroup: &apiGroup,
1414+
Kind: "VirtualMachine",
1415+
Name: "test-vm",
1416+
},
1417+
VolumeMode: &volumeMode,
1418+
},
1419+
}
1420+
_, err := k8sclient.CoreV1().PersistentVolumeClaims(namespace).Create(ctx, pvc, metav1.CreateOptions{})
1421+
assert.NoError(t, err)
1422+
1423+
// Test case 1: CnsRegisterVolume without volumeMode should inherit from PVC
1424+
instance := &cnsregistervolumev1alpha1.CnsRegisterVolume{
1425+
ObjectMeta: metav1.ObjectMeta{
1426+
Name: "register-vol",
1427+
Namespace: namespace,
1428+
},
1429+
Spec: cnsregistervolumev1alpha1.CnsRegisterVolumeSpec{
1430+
PvcName: pvcName,
1431+
VolumeID: "123456",
1432+
AccessMode: corev1.ReadWriteOnce,
1433+
// VolumeMode is not set
1434+
},
1435+
}
1436+
1437+
// Simulate the logic from the Reconcile function
1438+
existingPVC, err := checkExistingPVCDataSourceRef(ctx, k8sclient, instance.Spec.PvcName, instance.Namespace)
1439+
assert.NoError(t, err)
1440+
assert.NotNil(t, existingPVC)
1441+
1442+
// Apply the volumeMode validation and inheritance logic
1443+
mismatchDetected := false
1444+
if existingPVC != nil && existingPVC.Spec.DataSourceRef != nil && existingPVC.Spec.VolumeMode != nil {
1445+
if instance.Spec.VolumeMode == "" {
1446+
instance.Spec.VolumeMode = *existingPVC.Spec.VolumeMode
1447+
} else if instance.Spec.VolumeMode != *existingPVC.Spec.VolumeMode {
1448+
mismatchDetected = true
1449+
}
1450+
}
1451+
1452+
// Verify that volumeMode was inherited and no mismatch detected
1453+
assert.False(t, mismatchDetected)
1454+
assert.Equal(t, corev1.PersistentVolumeBlock, instance.Spec.VolumeMode)
1455+
}
1456+
1457+
func TestVolumeModeNotInheritedWhenAlreadySet(t *testing.T) {
1458+
ctx := context.Background()
1459+
k8sclient := k8sfake.NewSimpleClientset()
1460+
namespace := "test-namespace"
1461+
pvcName := "test-pvc"
1462+
1463+
// Create a PVC with DataSourceRef and volumeMode set to Block
1464+
apiGroup := "vmoperator.vmware.com"
1465+
volumeMode := corev1.PersistentVolumeBlock
1466+
pvc := &corev1.PersistentVolumeClaim{
1467+
ObjectMeta: metav1.ObjectMeta{
1468+
Name: pvcName,
1469+
Namespace: namespace,
1470+
},
1471+
Spec: corev1.PersistentVolumeClaimSpec{
1472+
DataSourceRef: &corev1.TypedObjectReference{
1473+
APIGroup: &apiGroup,
1474+
Kind: "VirtualMachine",
1475+
Name: "test-vm",
1476+
},
1477+
VolumeMode: &volumeMode,
1478+
},
1479+
}
1480+
_, err := k8sclient.CoreV1().PersistentVolumeClaims(namespace).Create(ctx, pvc, metav1.CreateOptions{})
1481+
assert.NoError(t, err)
1482+
1483+
// Test case 2: CnsRegisterVolume with volumeMode already set should NOT be overridden
1484+
instance := &cnsregistervolumev1alpha1.CnsRegisterVolume{
1485+
ObjectMeta: metav1.ObjectMeta{
1486+
Name: "register-vol",
1487+
Namespace: namespace,
1488+
},
1489+
Spec: cnsregistervolumev1alpha1.CnsRegisterVolumeSpec{
1490+
PvcName: pvcName,
1491+
VolumeID: "123456",
1492+
AccessMode: corev1.ReadWriteOnce,
1493+
VolumeMode: corev1.PersistentVolumeFilesystem, // Already set to Filesystem
1494+
},
1495+
}
1496+
1497+
originalVolumeMode := instance.Spec.VolumeMode
1498+
1499+
// Simulate the logic from the Reconcile function
1500+
existingPVC, err := checkExistingPVCDataSourceRef(ctx, k8sclient, instance.Spec.PvcName, instance.Namespace)
1501+
assert.NoError(t, err)
1502+
assert.NotNil(t, existingPVC)
1503+
1504+
// Apply the volumeMode validation and inheritance logic
1505+
mismatchDetected := false
1506+
if existingPVC != nil && existingPVC.Spec.DataSourceRef != nil && existingPVC.Spec.VolumeMode != nil {
1507+
if instance.Spec.VolumeMode == "" {
1508+
instance.Spec.VolumeMode = *existingPVC.Spec.VolumeMode
1509+
} else if instance.Spec.VolumeMode != *existingPVC.Spec.VolumeMode {
1510+
mismatchDetected = true
1511+
}
1512+
}
1513+
1514+
// Verify that mismatch was detected since PVC has Block but instance has Filesystem
1515+
assert.True(t, mismatchDetected)
1516+
// Verify that volumeMode was NOT changed (still has original Filesystem)
1517+
assert.Equal(t, originalVolumeMode, instance.Spec.VolumeMode)
1518+
assert.Equal(t, corev1.PersistentVolumeFilesystem, instance.Spec.VolumeMode)
1519+
}
1520+
1521+
func TestVolumeModeNotInheritedWhenNoDataSourceRef(t *testing.T) {
1522+
ctx := context.Background()
1523+
k8sclient := k8sfake.NewSimpleClientset()
1524+
namespace := "test-namespace"
1525+
pvcName := "test-pvc"
1526+
1527+
// Create a PVC without DataSourceRef but with volumeMode set
1528+
volumeMode := corev1.PersistentVolumeBlock
1529+
pvc := &corev1.PersistentVolumeClaim{
1530+
ObjectMeta: metav1.ObjectMeta{
1531+
Name: pvcName,
1532+
Namespace: namespace,
1533+
},
1534+
Spec: corev1.PersistentVolumeClaimSpec{
1535+
DataSourceRef: nil, // No DataSourceRef
1536+
VolumeMode: &volumeMode,
1537+
},
1538+
}
1539+
_, err := k8sclient.CoreV1().PersistentVolumeClaims(namespace).Create(ctx, pvc, metav1.CreateOptions{})
1540+
assert.NoError(t, err)
1541+
1542+
// Test case 3: CnsRegisterVolume should NOT inherit volumeMode when PVC has no DataSourceRef
1543+
instance := &cnsregistervolumev1alpha1.CnsRegisterVolume{
1544+
ObjectMeta: metav1.ObjectMeta{
1545+
Name: "register-vol",
1546+
Namespace: namespace,
1547+
},
1548+
Spec: cnsregistervolumev1alpha1.CnsRegisterVolumeSpec{
1549+
PvcName: pvcName,
1550+
VolumeID: "123456",
1551+
AccessMode: corev1.ReadWriteOnce,
1552+
// VolumeMode is not set
1553+
},
1554+
}
1555+
1556+
// Simulate the logic from the Reconcile function
1557+
existingPVC, err := checkExistingPVCDataSourceRef(ctx, k8sclient, instance.Spec.PvcName, instance.Namespace)
1558+
assert.NoError(t, err)
1559+
assert.NotNil(t, existingPVC)
1560+
1561+
// Apply the volumeMode validation and inheritance logic
1562+
mismatchDetected := false
1563+
if existingPVC != nil && existingPVC.Spec.DataSourceRef != nil && existingPVC.Spec.VolumeMode != nil {
1564+
if instance.Spec.VolumeMode == "" {
1565+
instance.Spec.VolumeMode = *existingPVC.Spec.VolumeMode
1566+
} else if instance.Spec.VolumeMode != *existingPVC.Spec.VolumeMode {
1567+
mismatchDetected = true
1568+
}
1569+
}
1570+
1571+
// Verify that no mismatch detected and volumeMode was NOT inherited (should remain empty)
1572+
// because PVC has no DataSourceRef
1573+
assert.False(t, mismatchDetected)
1574+
assert.Equal(t, corev1.PersistentVolumeMode(""), instance.Spec.VolumeMode)
1575+
}
1576+
1577+
func TestVolumeModeNotInheritedWhenPVCVolumeModeIsNil(t *testing.T) {
1578+
ctx := context.Background()
1579+
k8sclient := k8sfake.NewSimpleClientset()
1580+
namespace := "test-namespace"
1581+
pvcName := "test-pvc"
1582+
1583+
// Create a PVC with DataSourceRef but VolumeMode is nil (not set)
1584+
apiGroup := "vmoperator.vmware.com"
1585+
pvc := &corev1.PersistentVolumeClaim{
1586+
ObjectMeta: metav1.ObjectMeta{
1587+
Name: pvcName,
1588+
Namespace: namespace,
1589+
},
1590+
Spec: corev1.PersistentVolumeClaimSpec{
1591+
DataSourceRef: &corev1.TypedObjectReference{
1592+
APIGroup: &apiGroup,
1593+
Kind: "VirtualMachine",
1594+
Name: "test-vm",
1595+
},
1596+
VolumeMode: nil, // VolumeMode is nil
1597+
},
1598+
}
1599+
_, err := k8sclient.CoreV1().PersistentVolumeClaims(namespace).Create(ctx, pvc, metav1.CreateOptions{})
1600+
assert.NoError(t, err)
1601+
1602+
// Test case: CnsRegisterVolume should NOT inherit when PVC's volumeMode is nil
1603+
instance := &cnsregistervolumev1alpha1.CnsRegisterVolume{
1604+
ObjectMeta: metav1.ObjectMeta{
1605+
Name: "register-vol",
1606+
Namespace: namespace,
1607+
},
1608+
Spec: cnsregistervolumev1alpha1.CnsRegisterVolumeSpec{
1609+
PvcName: pvcName,
1610+
VolumeID: "123456",
1611+
AccessMode: corev1.ReadWriteOnce,
1612+
// VolumeMode is not set
1613+
},
1614+
}
1615+
1616+
// Simulate the logic from the Reconcile function
1617+
existingPVC, err := checkExistingPVCDataSourceRef(ctx, k8sclient, instance.Spec.PvcName, instance.Namespace)
1618+
assert.NoError(t, err)
1619+
assert.NotNil(t, existingPVC)
1620+
1621+
// Apply the volumeMode validation and inheritance logic
1622+
mismatchDetected := false
1623+
if existingPVC != nil && existingPVC.Spec.DataSourceRef != nil && existingPVC.Spec.VolumeMode != nil {
1624+
if instance.Spec.VolumeMode == "" {
1625+
instance.Spec.VolumeMode = *existingPVC.Spec.VolumeMode
1626+
} else if instance.Spec.VolumeMode != *existingPVC.Spec.VolumeMode {
1627+
mismatchDetected = true
1628+
}
1629+
}
1630+
1631+
// Verify that no mismatch detected and volumeMode was NOT inherited (should remain empty)
1632+
// because PVC's volumeMode is nil
1633+
assert.False(t, mismatchDetected)
1634+
assert.Equal(t, corev1.PersistentVolumeMode(""), instance.Spec.VolumeMode)
1635+
}
1636+
1637+
func TestVolumeModeMatchesExistingPVC(t *testing.T) {
1638+
ctx := context.Background()
1639+
k8sclient := k8sfake.NewSimpleClientset()
1640+
namespace := "test-namespace"
1641+
pvcName := "test-pvc"
1642+
1643+
// Create a PVC with DataSourceRef and volumeMode set to Block
1644+
apiGroup := "vmoperator.vmware.com"
1645+
volumeMode := corev1.PersistentVolumeBlock
1646+
pvc := &corev1.PersistentVolumeClaim{
1647+
ObjectMeta: metav1.ObjectMeta{
1648+
Name: pvcName,
1649+
Namespace: namespace,
1650+
},
1651+
Spec: corev1.PersistentVolumeClaimSpec{
1652+
DataSourceRef: &corev1.TypedObjectReference{
1653+
APIGroup: &apiGroup,
1654+
Kind: "VirtualMachine",
1655+
Name: "test-vm",
1656+
},
1657+
VolumeMode: &volumeMode,
1658+
},
1659+
}
1660+
_, err := k8sclient.CoreV1().PersistentVolumeClaims(namespace).Create(ctx, pvc, metav1.CreateOptions{})
1661+
assert.NoError(t, err)
1662+
1663+
// Test case: Both have volumeMode set to Block - should succeed
1664+
instance := &cnsregistervolumev1alpha1.CnsRegisterVolume{
1665+
ObjectMeta: metav1.ObjectMeta{
1666+
Name: "register-vol",
1667+
Namespace: namespace,
1668+
},
1669+
Spec: cnsregistervolumev1alpha1.CnsRegisterVolumeSpec{
1670+
PvcName: pvcName,
1671+
VolumeID: "123456",
1672+
AccessMode: corev1.ReadWriteOnce,
1673+
VolumeMode: corev1.PersistentVolumeBlock, // Matches PVC
1674+
},
1675+
}
1676+
1677+
// Simulate the logic from the Reconcile function
1678+
existingPVC, err := checkExistingPVCDataSourceRef(ctx, k8sclient, instance.Spec.PvcName, instance.Namespace)
1679+
assert.NoError(t, err)
1680+
assert.NotNil(t, existingPVC)
1681+
1682+
// Apply the volumeMode validation logic
1683+
mismatchDetected := false
1684+
if existingPVC != nil && existingPVC.Spec.DataSourceRef != nil && existingPVC.Spec.VolumeMode != nil {
1685+
if instance.Spec.VolumeMode == "" {
1686+
instance.Spec.VolumeMode = *existingPVC.Spec.VolumeMode
1687+
} else if instance.Spec.VolumeMode != *existingPVC.Spec.VolumeMode {
1688+
mismatchDetected = true
1689+
}
1690+
}
1691+
1692+
// Verify that no mismatch was detected
1693+
assert.False(t, mismatchDetected)
1694+
assert.Equal(t, corev1.PersistentVolumeBlock, instance.Spec.VolumeMode)
1695+
}

0 commit comments

Comments
 (0)