Skip to content

Commit

Permalink
Sync PVC's Status.Phase
Browse files Browse the repository at this point in the history 
Fix issues in nameing and format

Fix klog import issue

Update go.mod and go.sum

Revert "Update go.mod and go.sum"

This reverts commit fd7604abcf3cb81e3bf75fbc6ca166e1bc9bc34a.

Fix go.mod and go.sum

Add featuregate

Fix lint error

Fix lint errors
  • Loading branch information
@yuanchen8911
yuanchen8911 committed Aug 17, 2023
1 parent ba5a6e8 commit 8688b89
Show file tree
Hide file tree
Showing 5 changed files with 245 additions and 0 deletions.
14 changes: 14 additions & 0 deletions virtualcluster/pkg/syncer/conversion/equality.go
View file
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
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
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
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
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

0 comments on commit 8688b89

Please sign in to comment.