Merge pull request #44668 from NickrenREN/pv-cleanup

Automatic merge from submit-queue

Remove claimClass check and upgradeVolumeFrom1_2() function

**Release note**:
```release-note
NONE
```

@jsafrane  PTAL. Thanks

pkg/controller/volume/persistentvolume/pv_controller.go

@@ -1347,14 +1347,11 @@ func (ctrl *PersistentVolumeController) provisionClaimOperation(claimObj interfa
 	// Bind it to the claim
 	volume.Spec.ClaimRef = claimRef
 	volume.Status.Phase = v1.VolumeBound
+	volume.Spec.StorageClassName = claimClass
 
 	// Add annBoundByController (used in deleting the volume)
 	metav1.SetMetaDataAnnotation(&volume.ObjectMeta, annBoundByController, "yes")
 	metav1.SetMetaDataAnnotation(&volume.ObjectMeta, annDynamicallyProvisioned, plugin.GetPluginName())
-	// TODO: remove this check in 1.5, storage.StorageClassAnnotation will be always non-empty there.
-	if claimClass != "" {
-		volume.Spec.StorageClassName = claimClass
-	}
 
 	// Try to create the PV object several times
 	for i := 0; i < ctrl.createProvisionedPVRetryCount; i++ {

pkg/controller/volume/persistentvolume/pv_controller_base.go

@@ -131,18 +131,14 @@ func (ctrl *PersistentVolumeController) initializeCaches(volumeLister corelister
 		return
 	}
 	for _, volume := range volumeList {
-		// Ignore template volumes from kubernetes 1.2
+		clone, err := api.Scheme.DeepCopy(volume)
-		deleted := ctrl.upgradeVolumeFrom1_2(volume)
+		if err != nil {
-		if !deleted {
+			glog.Errorf("error cloning volume %q: %v", volume.Name, err)
-			clone, err := api.Scheme.DeepCopy(volume)
+			continue
-			if err != nil {
+		}
-				glog.Errorf("error cloning volume %q: %v", volume.Name, err)
+		volumeClone := clone.(*v1.PersistentVolume)
-				continue
+		if _, err = ctrl.storeVolumeUpdate(volumeClone); err != nil {
-			}
+			glog.Errorf("error updating volume cache: %v", err)
-			volumeClone := clone.(*v1.PersistentVolume)
-			if _, err = ctrl.storeVolumeUpdate(volumeClone); err != nil {
-				glog.Errorf("error updating volume cache: %v", err)
-			}
 		}
 	}
 
@@ -191,11 +187,6 @@ func (ctrl *PersistentVolumeController) storeClaimUpdate(claim interface{}) (boo
 // updateVolume runs in worker thread and handles "volume added",
 // "volume updated" and "periodic sync" events.
 func (ctrl *PersistentVolumeController) updateVolume(volume *v1.PersistentVolume) {
-	if deleted := ctrl.upgradeVolumeFrom1_2(volume); deleted {
-		// volume deleted
-		return
-	}
-
 	// Store the new volume version in the cache and do not process it if this
 	// is an old version.
 	new, err := ctrl.storeVolumeUpdate(volume)
@@ -407,44 +398,6 @@ func (ctrl *PersistentVolumeController) claimWorker() {
 	}
 }
 
-const (
-	// these pair of constants are used by the provisioner in Kubernetes 1.2.
-	pvProvisioningRequiredAnnotationKey    = "volume.experimental.kubernetes.io/provisioning-required"
-	pvProvisioningCompletedAnnotationValue = "volume.experimental.kubernetes.io/provisioning-completed"
-)
-
-// upgradeVolumeFrom1_2 updates PV from Kubernetes 1.2 to 1.3 and newer. In 1.2,
-// we used template PersistentVolume instances for dynamic provisioning. In 1.3
-// and later, these template (and not provisioned) instances must be removed to
-// make the controller to provision a new PV.
-// It returns true if the volume was deleted.
-// TODO: remove this function when upgrade from 1.2 becomes unsupported.
-func (ctrl *PersistentVolumeController) upgradeVolumeFrom1_2(volume *v1.PersistentVolume) bool {
-	annValue, found := volume.Annotations[pvProvisioningRequiredAnnotationKey]
-	if !found {
-		// The volume is not template
-		return false
-	}
-	if annValue == pvProvisioningCompletedAnnotationValue {
-		// The volume is already fully provisioned. The new controller will
-		// ignore this annotation and it will obey its ReclaimPolicy, which is
-		// likely to delete the volume when appropriate claim is deleted.
-		return false
-	}
-	glog.V(2).Infof("deleting unprovisioned template volume %q from Kubernetes 1.2.", volume.Name)
-	err := ctrl.kubeClient.Core().PersistentVolumes().Delete(volume.Name, nil)
-	if err != nil {
-		glog.Errorf("cannot delete unprovisioned template volume %q: %v", volume.Name, err)
-	}
-	// Remove from local cache
-	err = ctrl.volumes.store.Delete(volume)
-	if err != nil {
-		glog.Errorf("cannot remove volume %q from local cache: %v", volume.Name, err)
-	}
-
-	return true
-}
-
 // setClaimProvisioner saves
 // claim.Annotations[annStorageProvisioner] = class.Provisioner
 func (ctrl *PersistentVolumeController) setClaimProvisioner(claim *v1.PersistentVolumeClaim, class *storage.StorageClass) (*v1.PersistentVolumeClaim, error) {

pkg/controller/volume/persistentvolume/pv_controller_test.go

@@ -89,74 +89,6 @@ func TestControllerSync(t *testing.T) {
 				return nil
 			},
 		},
-		{
-			// addVolume with provisioned volume from Kubernetes 1.2. No "action"
-			// is expected - it should stay bound.
-			"5-5 - add bound volume from 1.2",
-			novolumes,
-			[]*v1.PersistentVolume{addVolumeAnnotation(newVolume("volume5-5", "1Gi", "uid5-5", "claim5-5", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty), pvProvisioningRequiredAnnotationKey, pvProvisioningCompletedAnnotationValue)},
-			newClaimArray("claim5-5", "uid5-5", "1Gi", "", v1.ClaimPending, nil),
-			newClaimArray("claim5-5", "uid5-5", "1Gi", "volume5-5", v1.ClaimBound, nil, annBindCompleted, annBoundByController),
-			noevents, noerrors,
-			// Custom test function that generates a add event
-			func(ctrl *PersistentVolumeController, reactor *volumeReactor, test controllerTest) error {
-				volume := newVolume("volume5-5", "1Gi", "uid5-5", "claim5-5", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty)
-				volume = addVolumeAnnotation(volume, pvProvisioningRequiredAnnotationKey, pvProvisioningCompletedAnnotationValue)
-				reactor.addVolumeEvent(volume)
-				return nil
-			},
-		},
-		{
-			// updateVolume with provisioned volume from Kubernetes 1.2. No
-			// "action" is expected - it should stay bound.
-			"5-6 - update bound volume from 1.2",
-			[]*v1.PersistentVolume{addVolumeAnnotation(newVolume("volume5-6", "1Gi", "uid5-6", "claim5-6", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty), pvProvisioningRequiredAnnotationKey, pvProvisioningCompletedAnnotationValue)},
-			[]*v1.PersistentVolume{addVolumeAnnotation(newVolume("volume5-6", "1Gi", "uid5-6", "claim5-6", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty), pvProvisioningRequiredAnnotationKey, pvProvisioningCompletedAnnotationValue)},
-			newClaimArray("claim5-6", "uid5-6", "1Gi", "volume5-6", v1.ClaimBound, nil),
-			newClaimArray("claim5-6", "uid5-6", "1Gi", "volume5-6", v1.ClaimBound, nil, annBindCompleted),
-			noevents, noerrors,
-			// Custom test function that generates a add event
-			func(ctrl *PersistentVolumeController, reactor *volumeReactor, test controllerTest) error {
-				volume := newVolume("volume5-6", "1Gi", "uid5-6", "claim5-6", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty)
-				volume = addVolumeAnnotation(volume, pvProvisioningRequiredAnnotationKey, pvProvisioningCompletedAnnotationValue)
-				reactor.modifyVolumeEvent(volume)
-				return nil
-			},
-		},
-		{
-			// addVolume with unprovisioned volume from Kubernetes 1.2. The
-			// volume should be deleted.
-			"5-7 - add unprovisioned volume from 1.2",
-			novolumes,
-			novolumes,
-			newClaimArray("claim5-7", "uid5-7", "1Gi", "", v1.ClaimPending, nil),
-			newClaimArray("claim5-7", "uid5-7", "1Gi", "", v1.ClaimPending, nil),
-			noevents, noerrors,
-			// Custom test function that generates a add event
-			func(ctrl *PersistentVolumeController, reactor *volumeReactor, test controllerTest) error {
-				volume := newVolume("volume5-7", "1Gi", "uid5-7", "claim5-7", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty)
-				volume = addVolumeAnnotation(volume, pvProvisioningRequiredAnnotationKey, "yes")
-				reactor.addVolumeEvent(volume)
-				return nil
-			},
-		},
-		{
-			// updateVolume with unprovisioned volume from Kubernetes 1.2. The
-			// volume should be deleted.
-			"5-8 - update bound volume from 1.2",
-			novolumes,
-			novolumes,
-			newClaimArray("claim5-8", "uid5-8", "1Gi", "", v1.ClaimPending, nil),
-			newClaimArray("claim5-8", "uid5-8", "1Gi", "", v1.ClaimPending, nil),
-			noevents, noerrors,
-			// Custom test function that generates a add event
-			func(ctrl *PersistentVolumeController, reactor *volumeReactor, test controllerTest) error {
-				volume := newVolume("volume5-8", "1Gi", "uid5-8", "claim5-8", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty)
-				volume = addVolumeAnnotation(volume, pvProvisioningRequiredAnnotationKey, "yes")
-				reactor.modifyVolumeEvent(volume)
-				return nil
-			},
-		},
 	}
 
 	for _, test := range tests {