diff --git a/virtualcluster/pkg/syncer/conversion/equality.go b/virtualcluster/pkg/syncer/conversion/equality.go index 2e251416..4c05faae 100644 --- a/virtualcluster/pkg/syncer/conversion/equality.go +++ b/virtualcluster/pkg/syncer/conversion/equality.go @@ -28,6 +28,7 @@ import ( "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/sets" + "k8s.io/klog/v2" "k8s.io/utils/pointer" "sigs.k8s.io/cluster-api-provider-nested/virtualcluster/pkg/apis/tenancy/v1alpha1" @@ -741,6 +742,19 @@ func (e vcEquality) CheckUWPVCStatusEquality(pObj, vObj *v1.PersistentVolumeClai } updated.Status.Capacity["storage"] = pObj.Status.Capacity["storage"] } + + // Check if a tenant cluster's PVC(vPVC) is bound but a super cluster's PVC (pPVC) is not bound. + // In this case, update the vPVC's Status.Phase. + if featuregate.DefaultFeatureGate.Enabled(featuregate.SyncTenantPVCStatusPhase) { + if vObj.Status.Phase == v1.ClaimBound && pObj.Status.Phase != v1.ClaimBound { + if updated == nil { + updated = vObj.DeepCopy() + } + klog.Warningf("Virtual cluster's PVC %s in %s is %v while the super cluster's PVC %s is not", vObj.Name, vObj.ClusterName, v1.ClaimBound, pObj.Name) + updated.Status.Phase = pObj.Status.Phase + } + } + return updated } diff --git a/virtualcluster/pkg/syncer/conversion/equality_test.go b/virtualcluster/pkg/syncer/conversion/equality_test.go index ce9ab19d..efb62e4f 100644 --- a/virtualcluster/pkg/syncer/conversion/equality_test.go +++ b/virtualcluster/pkg/syncer/conversion/equality_test.go @@ -21,10 +21,12 @@ import ( v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/equality" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/utils/pointer" "sigs.k8s.io/cluster-api-provider-nested/virtualcluster/pkg/apis/tenancy/v1alpha1" "sigs.k8s.io/cluster-api-provider-nested/virtualcluster/pkg/syncer/apis/config" + "sigs.k8s.io/cluster-api-provider-nested/virtualcluster/pkg/syncer/util/featuregate" ) func TestCheckDWKVEquality(t *testing.T) { @@ -979,3 +981,137 @@ func TestCheckDWPodConditionEquality(t *testing.T) { }) } } + +func TestCheckUWPVCStatusEquality(t *testing.T) { + featuregate.DefaultFeatureGate.Set(featuregate.SyncTenantPVCStatusPhase, true) + for _, tt := range []struct { + name string + pObj *v1.PersistentVolumeClaim + vObj *v1.PersistentVolumeClaim + updatedObj *v1.PersistentVolumeClaim + }{ + { + name: "pPVC is pending and vPVC is pending", + pObj: &v1.PersistentVolumeClaim{ + Status: v1.PersistentVolumeClaimStatus{ + Phase: v1.ClaimPending, + }, + }, + vObj: &v1.PersistentVolumeClaim{ + Status: v1.PersistentVolumeClaimStatus{ + Phase: v1.ClaimPending, + }, + }, + updatedObj: nil, + }, + { + name: "pPVC is bound and vPVC is bound", + pObj: &v1.PersistentVolumeClaim{ + Status: v1.PersistentVolumeClaimStatus{ + Phase: v1.ClaimBound, + }, + }, + vObj: &v1.PersistentVolumeClaim{ + Status: v1.PersistentVolumeClaimStatus{ + Phase: v1.ClaimBound, + }, + }, + updatedObj: nil, + }, + { + name: "pPVC is lost and vPVC is lost", + pObj: &v1.PersistentVolumeClaim{ + Status: v1.PersistentVolumeClaimStatus{ + Phase: v1.ClaimLost, + }, + }, + vObj: &v1.PersistentVolumeClaim{ + Status: v1.PersistentVolumeClaimStatus{ + Phase: v1.ClaimLost, + }, + }, + updatedObj: nil, + }, + { + name: "pPVC is lost and vPVC is bound", + pObj: &v1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pPVC", + }, + Status: v1.PersistentVolumeClaimStatus{ + Phase: v1.ClaimLost, + }, + }, + vObj: &v1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: "vPVC", + }, + Status: v1.PersistentVolumeClaimStatus{ + Phase: v1.ClaimBound, + }, + }, + updatedObj: &v1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: "vPVC", + }, + Status: v1.PersistentVolumeClaimStatus{ + Phase: v1.ClaimLost, + }, + }, + }, + { + name: "pPVC is pending and vPVC is bound", + pObj: &v1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pPVC", + }, + Status: v1.PersistentVolumeClaimStatus{ + Phase: v1.ClaimPending, + }, + }, + vObj: &v1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: "vPVC", + }, + Status: v1.PersistentVolumeClaimStatus{ + Phase: v1.ClaimBound, + }, + }, + updatedObj: &v1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: "vPVC", + }, + Status: v1.PersistentVolumeClaimStatus{ + Phase: v1.ClaimPending, + }, + }, + }, + { + name: "pPVC is bound and vPVC is pending", + pObj: &v1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pPVC", + }, + Status: v1.PersistentVolumeClaimStatus{ + Phase: v1.ClaimBound, + }, + }, + vObj: &v1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: "vPVC", + }, + Status: v1.PersistentVolumeClaimStatus{ + Phase: v1.ClaimPending, + }, + }, + updatedObj: nil, + }, + } { + t.Run(tt.name, func(tc *testing.T) { + obj := Equality(nil, nil).CheckUWPVCStatusEquality(tt.pObj, tt.vObj) + if obj != nil && obj.Status.Phase != tt.updatedObj.Status.Phase { + tc.Errorf("expected vPVC's Status.Phase: %v, got: %v", tt.updatedObj.Status.Phase, obj.Status.Phase) + } + }) + } +} diff --git a/virtualcluster/pkg/syncer/resources/persistentvolumeclaim/checker_test.go b/virtualcluster/pkg/syncer/resources/persistentvolumeclaim/checker_test.go index bc674892..a9f80d9b 100644 --- a/virtualcluster/pkg/syncer/resources/persistentvolumeclaim/checker_test.go +++ b/virtualcluster/pkg/syncer/resources/persistentvolumeclaim/checker_test.go @@ -29,10 +29,29 @@ import ( "sigs.k8s.io/cluster-api-provider-nested/virtualcluster/pkg/apis/tenancy/v1alpha1" "sigs.k8s.io/cluster-api-provider-nested/virtualcluster/pkg/syncer/conversion" + "sigs.k8s.io/cluster-api-provider-nested/virtualcluster/pkg/syncer/util/featuregate" util "sigs.k8s.io/cluster-api-provider-nested/virtualcluster/pkg/syncer/util/test" ) +var ( + statusPending = &corev1.PersistentVolumeClaimStatus{ + Phase: corev1.ClaimPending, + } + statusBound = &corev1.PersistentVolumeClaimStatus{ + Phase: corev1.ClaimBound, + } + statusLost = &corev1.PersistentVolumeClaimStatus{ + Phase: corev1.ClaimLost, + } +) + +func applyStatusToPVC(pvc *corev1.PersistentVolumeClaim, pvs *corev1.PersistentVolumeClaimStatus) *corev1.PersistentVolumeClaim { + pvc.Status.Phase = pvs.Phase + return pvc +} + func TestPVCPatrol(t *testing.T) { + defer util.SetFeatureGateDuringTest(t, featuregate.DefaultFeatureGate, featuregate.SyncTenantPVCStatusPhase, true)() testTenant := &v1alpha1.VirtualCluster{ ObjectMeta: metav1.ObjectMeta{ Name: "test", @@ -130,6 +149,37 @@ func TestPVCPatrol(t *testing.T) { }, WaitDWS: true, }, + "pPVC is lost, vPVC is bound": { + ExistingObjectInSuper: []runtime.Object{ + applyStatusToPVC(superPVC("pvc-3", superDefaultNSName, "12345", defaultClusterKey), statusLost), + }, + ExistingObjectInTenant: []runtime.Object{ + applyStatusToPVC(tenantPVC("pvc-3", "default", "12345"), statusBound), + }, + // TODO: Set ExpectedUpdatedVObject with Status.Phase="Lost" + ExpectedNoOperation: false, + WaitUWS: true, + }, + "pPVC is bound, vPVC is pending": { + ExistingObjectInSuper: []runtime.Object{ + applyStatusToPVC(superPVC("pvc-3", superDefaultNSName, "12345", defaultClusterKey), statusBound), + }, + ExistingObjectInTenant: []runtime.Object{ + applyStatusToPVC(tenantPVC("pvc-3", "default", "12345"), statusPending), + }, + ExpectedUpdatedVObject: []runtime.Object{}, + ExpectedNoOperation: true, + }, + "pPVC is pending, vPVC is pending": { + ExistingObjectInSuper: []runtime.Object{ + applyStatusToPVC(superPVC("pvc-3", superDefaultNSName, "12345", defaultClusterKey), statusPending), + }, + ExistingObjectInTenant: []runtime.Object{ + applyStatusToPVC(tenantPVC("pvc-3", "default", "12345"), statusPending), + }, + ExpectedUpdatedVObject: []runtime.Object{}, + ExpectedNoOperation: true, + }, } for k, tc := range testcases { diff --git a/virtualcluster/pkg/syncer/resources/persistentvolumeclaim/uws_test.go b/virtualcluster/pkg/syncer/resources/persistentvolumeclaim/uws_test.go index 95ad6ca6..4e2bb34f 100644 --- a/virtualcluster/pkg/syncer/resources/persistentvolumeclaim/uws_test.go +++ b/virtualcluster/pkg/syncer/resources/persistentvolumeclaim/uws_test.go @@ -28,6 +28,7 @@ import ( core "k8s.io/client-go/testing" "sigs.k8s.io/cluster-api-provider-nested/virtualcluster/pkg/apis/tenancy/v1alpha1" "sigs.k8s.io/cluster-api-provider-nested/virtualcluster/pkg/syncer/conversion" + "sigs.k8s.io/cluster-api-provider-nested/virtualcluster/pkg/syncer/util/featuregate" util "sigs.k8s.io/cluster-api-provider-nested/virtualcluster/pkg/syncer/util/test" ) @@ -51,6 +52,7 @@ func TestUWPVCUpdate(t *testing.T) { }, } + defer util.SetFeatureGateDuringTest(t, featuregate.DefaultFeatureGate, featuregate.SyncTenantPVCStatusPhase, true)() defaultClusterKey := conversion.ToClusterKey(testTenant) superDefaultNSName := conversion.ToSuperClusterNamespace(defaultClusterKey, "default") @@ -99,6 +101,41 @@ func TestUWPVCUpdate(t *testing.T) { "default/pvc-1", }, }, + "pPVC is lost, vPVC is bound": { + ExistingObjectInSuper: []runtime.Object{ + applyStatusToPVC(superPVC("pvc-1", superDefaultNSName, "12345", defaultClusterKey), statusLost), + }, + ExistingObjectInTenant: []runtime.Object{ + applyStatusToPVC(tenantPVC("pvc-1", "default", "12345"), statusBound), + }, + EnqueuedKey: superDefaultNSName + "/pvc-1", + ExpectedError: "", + ExpectedUpdatedObject: []string{ + "default/pvc-1", + }, + }, + "pPVC is bound, vPVC is pending": { + ExistingObjectInSuper: []runtime.Object{ + applyStatusToPVC(superPVC("pvc-1", superDefaultNSName, "12345", defaultClusterKey), statusBound), + }, + ExistingObjectInTenant: []runtime.Object{ + applyStatusToPVC(tenantPVC("pvc-1", "default", "12345"), statusPending), + }, + EnqueuedKey: superDefaultNSName + "/pvc-1", + ExpectedError: "", + ExpectedUpdatedObject: []string{}, + }, + "pPVC is pending, vPVC is pending": { + ExistingObjectInSuper: []runtime.Object{ + applyStatusToPVC(superPVC("pvc-1", superDefaultNSName, "12345", defaultClusterKey), statusPending), + }, + ExistingObjectInTenant: []runtime.Object{ + applyStatusToPVC(tenantPVC("pvc-1", "default", "12345"), statusPending), + }, + EnqueuedKey: superDefaultNSName + "/pvc-1", + ExpectedError: "", + ExpectedUpdatedObject: []string{}, + }, } for k, tc := range testcases { diff --git a/virtualcluster/pkg/syncer/util/featuregate/gate.go b/virtualcluster/pkg/syncer/util/featuregate/gate.go index 7b0a514a..f50a55c9 100644 --- a/virtualcluster/pkg/syncer/util/featuregate/gate.go +++ b/virtualcluster/pkg/syncer/util/featuregate/gate.go @@ -88,6 +88,13 @@ const ( // add clusterIP of pService to vService's externalIPs. // So that vService can be resolved by using the k8s_external plugin in coredns. VServiceExternalIP = "VServiceExternalIP" + + // SyncTenantPVCStatusPhase is an experimental feature that enables the syncer to + // update the Status.Phase of a tenant cluster's PVC if it is Bound, + // but the corresponding PVC in the super cluster is not Bound, e.g., Lost. + // Although rare, this situation can arise due to potential bugs and race conditions. + // This feature allows users to perform separate investigation and resolution. + SyncTenantPVCStatusPhase = "SyncTenantPVCStatusPhase" ) var defaultFeatures = FeatureList{ @@ -103,6 +110,7 @@ var defaultFeatures = FeatureList{ DisableCRDPreserveUnknownFields: {Default: false}, RootCACertConfigMapSupport: {Default: false}, VServiceExternalIP: {Default: false}, + SyncTenantPVCStatusPhase: {Default: false}, } type Feature string