scheduler/volumebinding: remove Get[API]{PV,PVC}

should be replaced by generic Get[APIObj]
This commit is contained in:
胡玮文 2025-09-12 15:03:47 +08:00
parent ed19492dc2
commit 5a708a7ff0
4 changed files with 39 additions and 55 deletions

View file

@ -57,14 +57,6 @@ func NewPVAssumeCache(logger klog.Logger, informer informer) (PVAssumeCache, err
return PVAssumeCache{cache}, err
}
func (c PVAssumeCache) GetPV(pvName string) (*v1.PersistentVolume, error) {
return c.Get(pvName)
}
func (c PVAssumeCache) GetAPIPV(pvName string) (*v1.PersistentVolume, error) {
return c.GetAPIObj(pvName)
}
func (c PVAssumeCache) ListPVs(storageClassName string) ([]*v1.PersistentVolume, error) {
// This works because we will never change the storage class in scheduler
// Assumed PVs needs to be included here to ensure the same PVC will not be bound to another PV in the next scheduling cycle.
@ -82,11 +74,3 @@ func NewPVCAssumeCache(logger klog.Logger, informer informer) (PVCAssumeCache, e
cache, err := newAssumeCache[*v1.PersistentVolumeClaim](logger, informer, schema.GroupResource{Resource: "persistentvolumeclaims"})
return PVCAssumeCache{cache}, err
}
func (c PVCAssumeCache) GetPVC(pvName string) (*v1.PersistentVolumeClaim, error) {
return c.Get(pvName)
}
func (c PVCAssumeCache) GetAPIPVC(pvName string) (*v1.PersistentVolumeClaim, error) {
return c.GetAPIObj(pvName)
}

View file

@ -97,12 +97,12 @@ func verifyListPVs(t *testing.T, cache PVAssumeCache, expectedPVs map[string]*v1
}
func verifyPV(cache PVAssumeCache, name string, expectedPV *v1.PersistentVolume) error {
pv, err := cache.GetPV(name)
pv, err := cache.Get(name)
if err != nil {
return err
}
if pv != expectedPV {
return fmt.Errorf("GetPV() returned %p, expected %p", pv, expectedPV)
return fmt.Errorf("Get() returned %p, expected %p", pv, expectedPV)
}
return nil
}
@ -168,7 +168,7 @@ func TestAssumePV(t *testing.T) {
t.Errorf("Test %q failed: Assume() returned success but expected error", name)
}
// Check that GetPV returns correct PV
// Check that Get returns correct PV
expectedPV := scenario.newPV
if !scenario.shouldSucceed {
expectedPV = scenario.oldPV
@ -192,13 +192,13 @@ func TestRestorePV(t *testing.T) {
// Add oldPV to cache
informer.add(oldPV)
if err := verifyPV(cache, oldPV.Name, oldPV); err != nil {
t.Fatalf("Failed to GetPV() after initial update: %v", err)
t.Fatalf("Failed to Get() after initial update: %v", err)
}
// Restore PV
cache.Restore(oldPV)
if err := verifyPV(cache, oldPV.Name, oldPV); err != nil {
t.Fatalf("Failed to GetPV() after initial restore: %v", err)
t.Fatalf("Failed to Get() after initial restore: %v", err)
}
// Assume newPV
@ -206,13 +206,13 @@ func TestRestorePV(t *testing.T) {
t.Fatalf("Assume() returned error %v", err)
}
if err := verifyPV(cache, oldPV.Name, newPV); err != nil {
t.Fatalf("Failed to GetPV() after Assume: %v", err)
t.Fatalf("Failed to Get() after Assume: %v", err)
}
// Restore PV
cache.Restore(newPV)
if err := verifyPV(cache, oldPV.Name, oldPV); err != nil {
t.Fatalf("Failed to GetPV() after restore: %v", err)
t.Fatalf("Failed to Get() after restore: %v", err)
}
}
@ -220,12 +220,12 @@ func TestBasicPVCache(t *testing.T) {
informer, cache := newTestPVCache(t)
// Get object that doesn't exist
pv, err := cache.GetPV("nothere")
pv, err := cache.Get("nothere")
if err == nil {
t.Errorf("GetPV() returned unexpected success")
t.Errorf("Get() returned unexpected success")
}
if pv != nil {
t.Errorf("GetPV() returned unexpected PV %q", pv.Name)
t.Errorf("Get() returned unexpected PV %q", pv.Name)
}
// Add a bunch of PVs
@ -339,12 +339,12 @@ func makeClaim(name, version, namespace string) *v1.PersistentVolumeClaim {
}
func verifyPVC(cache PVCAssumeCache, pvcKey string, expectedPVC *v1.PersistentVolumeClaim) error {
pvc, err := cache.GetPVC(pvcKey)
pvc, err := cache.Get(pvcKey)
if err != nil {
return err
}
if pvc != expectedPVC {
return fmt.Errorf("GetPVC() returned %p, expected %p", pvc, expectedPVC)
return fmt.Errorf("Get() returned %p, expected %p", pvc, expectedPVC)
}
return nil
}
@ -407,7 +407,7 @@ func TestAssumePVC(t *testing.T) {
// Add oldPVC to cache
informer.add(scenario.oldPVC)
if err := verifyPVC(cache, getPVCName(scenario.oldPVC), scenario.oldPVC); err != nil {
t.Fatalf("Failed to GetPVC() after initial update: %v", err)
t.Fatalf("Failed to Get() after initial update: %v", err)
}
// Assume newPVC
@ -419,13 +419,13 @@ func TestAssumePVC(t *testing.T) {
t.Errorf("Test %q failed: Assume() returned success but expected error", name)
}
// Check that GetPVC returns correct PVC
// Check that Get returns correct PVC
expectedPV := scenario.newPVC
if !scenario.shouldSucceed {
expectedPV = scenario.oldPVC
}
if err := verifyPVC(cache, getPVCName(scenario.oldPVC), expectedPV); err != nil {
t.Errorf("Failed to GetPVC() after initial update: %v", err)
t.Errorf("Failed to Get() after initial update: %v", err)
}
})
}
@ -443,13 +443,13 @@ func TestRestorePVC(t *testing.T) {
// Add oldPVC to cache
informer.add(oldPVC)
if err := verifyPVC(cache, getPVCName(oldPVC), oldPVC); err != nil {
t.Fatalf("Failed to GetPVC() after initial update: %v", err)
t.Fatalf("Failed to Get() after initial update: %v", err)
}
// Restore PVC
cache.Restore(oldPVC)
if err := verifyPVC(cache, getPVCName(oldPVC), oldPVC); err != nil {
t.Fatalf("Failed to GetPVC() after initial restore: %v", err)
t.Fatalf("Failed to Get() after initial restore: %v", err)
}
// Assume newPVC
@ -457,13 +457,13 @@ func TestRestorePVC(t *testing.T) {
t.Fatalf("Assume() returned error %v", err)
}
if err := verifyPVC(cache, getPVCName(oldPVC), newPVC); err != nil {
t.Fatalf("Failed to GetPVC() after Assume: %v", err)
t.Fatalf("Failed to Get() after Assume: %v", err)
}
// Restore PVC
cache.Restore(newPVC)
if err := verifyPVC(cache, getPVCName(oldPVC), oldPVC); err != nil {
t.Fatalf("Failed to GetPVC() after restore: %v", err)
t.Fatalf("Failed to Get() after restore: %v", err)
}
}
@ -480,7 +480,7 @@ func TestConcurrentAssumePVC(t *testing.T) {
t.Fatalf("Assume() returned error %v", err)
}
if err := verifyPVC(cache, getPVCName(pvc1Update), pvc1Update); err != nil {
t.Fatalf("Failed to GetPVC() after Assume: %v", err)
t.Fatalf("Failed to Get() after Assume: %v", err)
}
pvc2 := makeClaim("pvc1", "7", "ns1")
@ -496,7 +496,7 @@ func TestConcurrentAssumePVC(t *testing.T) {
cache.Restore(pvc1Update)
// Should still have pvc 2 in cache
if err := verifyPVC(cache, getPVCName(pvc2Update), pvc2Update); err != nil {
t.Fatalf("Failed to GetPVC() after restore: %v", err)
t.Fatalf("Failed to Get() after restore: %v", err)
}
}

View file

@ -625,12 +625,12 @@ func (b *volumeBinder) checkBindings(logger klog.Logger, pod *v1.Pod, bindings [
}
for _, binding := range bindings {
pv, err := b.pvCache.GetAPIPV(binding.pv.Name)
pv, err := b.pvCache.GetAPIObj(binding.pv.Name)
if err != nil {
return false, fmt.Errorf("failed to check binding: %w", err)
}
pvc, err := b.pvcCache.GetAPIPVC(getPVCName(binding.pvc))
pvc, err := b.pvcCache.GetAPIObj(getPVCName(binding.pvc))
if err != nil {
return false, fmt.Errorf("failed to check binding: %w", err)
}
@ -663,7 +663,7 @@ func (b *volumeBinder) checkBindings(logger klog.Logger, pod *v1.Pod, bindings [
}
for _, claim := range claimsToProvision {
pvc, err := b.pvcCache.GetAPIPVC(getPVCName(claim))
pvc, err := b.pvcCache.GetAPIObj(getPVCName(claim))
if err != nil {
return false, fmt.Errorf("failed to check provisioning pvc: %w", err)
}
@ -688,7 +688,7 @@ func (b *volumeBinder) checkBindings(logger klog.Logger, pod *v1.Pod, bindings [
// If the PVC is bound to a PV, check its node affinity
if pvc.Spec.VolumeName != "" {
pv, err := b.pvCache.GetAPIPV(pvc.Spec.VolumeName)
pv, err := b.pvCache.GetAPIObj(pvc.Spec.VolumeName)
if err != nil {
if apierrors.IsNotFound(err) {
// We tolerate NotFound error here, because PV is possibly
@ -754,7 +754,7 @@ func (b *volumeBinder) isPVCBound(logger klog.Logger, namespace, pvcName string)
},
}
pvcKey := getPVCName(claim)
pvc, err := b.pvcCache.GetPVC(pvcKey)
pvc, err := b.pvcCache.Get(pvcKey)
if err != nil || pvc == nil {
return false, nil, fmt.Errorf("error getting PVC %q: %v", pvcKey, err)
}
@ -845,7 +845,7 @@ func (b *volumeBinder) checkBoundClaims(logger klog.Logger, claims []*v1.Persist
for _, pvc := range claims {
pvName := pvc.Spec.VolumeName
pv, err := b.pvCache.GetPV(pvName)
pv, err := b.pvCache.Get(pvName)
if err != nil {
if apierrors.IsNotFound(err) {
err = nil

View file

@ -336,7 +336,7 @@ func (env *testEnv) updateVolumes(ctx context.Context, pvs []*v1.PersistentVolum
}
return wait.PollUntilContextTimeout(ctx, 100*time.Millisecond, 3*time.Second, false, func(ctx context.Context) (bool, error) {
for _, pv := range pvs {
pvInCache, err := env.internalBinder.pvCache.GetAPIPV(pv.Name)
pvInCache, err := env.internalBinder.pvCache.GetAPIObj(pv.Name)
if pvInCache == nil || err != nil {
return false, nil
}
@ -358,7 +358,7 @@ func (env *testEnv) updateClaims(ctx context.Context, pvcs []*v1.PersistentVolum
}
return wait.PollUntilContextTimeout(ctx, 100*time.Millisecond, 3*time.Second, false, func(ctx context.Context) (bool, error) {
for _, pvc := range pvcs {
pvcInCache, err := env.internalBinder.pvcCache.GetAPIPVC(getPVCName(pvc))
pvcInCache, err := env.internalBinder.pvcCache.GetAPIObj(getPVCName(pvc))
if pvcInCache == nil || err != nil {
return false, nil
}
@ -454,9 +454,9 @@ func (env *testEnv) validateAssume(t *testing.T, pod *v1.Pod, bindings []*Bindin
// Check pv cache
pvCache := env.internalBinder.pvCache
for _, b := range bindings {
pv, err := pvCache.GetPV(b.pv.Name)
pv, err := pvCache.Get(b.pv.Name)
if err != nil {
t.Errorf("GetPV %q returned error: %v", b.pv.Name, err)
t.Errorf("Get PV %q returned error: %v", b.pv.Name, err)
continue
}
if pv.Spec.ClaimRef == nil {
@ -475,9 +475,9 @@ func (env *testEnv) validateAssume(t *testing.T, pod *v1.Pod, bindings []*Bindin
pvcCache := env.internalBinder.pvcCache
for _, p := range provisionings {
pvcKey := getPVCName(p)
pvc, err := pvcCache.GetPVC(pvcKey)
pvc, err := pvcCache.Get(pvcKey)
if err != nil {
t.Errorf("GetPVC %q returned error: %v", pvcKey, err)
t.Errorf("Get PVC %q returned error: %v", pvcKey, err)
continue
}
if pvc.Annotations[volume.AnnSelectedNode] != nodeLabelValue {
@ -502,9 +502,9 @@ func (env *testEnv) validateCacheRestored(t *testing.T, pod *v1.Pod, bindings []
pvcCache := env.internalBinder.pvcCache
for _, p := range provisionings {
pvcKey := getPVCName(p)
pvc, err := pvcCache.GetPVC(pvcKey)
pvc, err := pvcCache.Get(pvcKey)
if err != nil {
t.Errorf("GetPVC %q returned error: %v", pvcKey, err)
t.Errorf("Get PVC %q returned error: %v", pvcKey, err)
continue
}
if pvc.Annotations[volume.AnnSelectedNode] != "" {
@ -522,9 +522,9 @@ func (env *testEnv) validateBind(
// Check pv cache
pvCache := env.internalBinder.pvCache
for _, pv := range expectedPVs {
cachedPV, err := pvCache.GetPV(pv.Name)
cachedPV, err := pvCache.Get(pv.Name)
if err != nil {
t.Errorf("GetPV %q returned error: %v", pv.Name, err)
t.Errorf("Get PV %q returned error: %v", pv.Name, err)
}
// Cache may be overridden by API object with higher version, compare but ignore resource version.
newCachedPV := cachedPV.DeepCopy()
@ -549,9 +549,9 @@ func (env *testEnv) validateProvision(
// Check pvc cache
pvcCache := env.internalBinder.pvcCache
for _, pvc := range expectedPVCs {
cachedPVC, err := pvcCache.GetPVC(getPVCName(pvc))
cachedPVC, err := pvcCache.Get(getPVCName(pvc))
if err != nil {
t.Errorf("GetPVC %q returned error: %v", getPVCName(pvc), err)
t.Errorf("Get PVC %q returned error: %v", getPVCName(pvc), err)
}
// Cache may be overridden by API object with higher version, compare but ignore resource version.
newCachedPVC := cachedPVC.DeepCopy()