Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Sync PVC's Status.Phase #351

Merged
merged 1 commit into from
Aug 18, 2023
+245 −0
Merged

Sync PVC's Status.Phase #351

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions virtualcluster/pkg/syncer/conversion/equality.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
}

Expand Down
136 changes: 136 additions & 0 deletions virtualcluster/pkg/syncer/conversion/equality_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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)
}
})
}
}
50 changes: 50 additions & 0 deletions virtualcluster/pkg/syncer/resources/persistentvolumeclaim/checker_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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 {
Expand Down
37 changes: 37 additions & 0 deletions virtualcluster/pkg/syncer/resources/persistentvolumeclaim/uws_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand All @@ -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")

Expand Down Expand Up @@ -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 {
Expand Down
8 changes: 8 additions & 0 deletions virtualcluster/pkg/syncer/util/featuregate/gate.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand All @@ -103,6 +110,7 @@ var defaultFeatures = FeatureList{
DisableCRDPreserveUnknownFields: {Default: false},
RootCACertConfigMapSupport: {Default: false},
VServiceExternalIP: {Default: false},
SyncTenantPVCStatusPhase: {Default: false},
}

type Feature string
Expand Down
Loading