Clean up PersistentVolumeLabel admission plugin

Azure and vSphere cloud providers were removed in v1.30. Remove their usage
from PersistentVolumeLabel admission plugin, otherwise the admission plugin
refuses creation of in-tree AzureDisk or vSphere volume.

plugin/pkg/admission/storage/persistentvolume/label/admission.go

@@ -55,11 +55,9 @@ var _ = admission.Interface(&persistentVolumeLabel{})
 type persistentVolumeLabel struct {
 	*admission.Handler
 
-	mutex            sync.Mutex
+	mutex        sync.Mutex
-	cloudConfig      []byte
+	cloudConfig  []byte
-	gcePVLabeler     cloudprovider.PVLabeler
+	gcePVLabeler cloudprovider.PVLabeler
-	azurePVLabeler   cloudprovider.PVLabeler
-	vspherePVLabeler cloudprovider.PVLabeler
 }
 
 var _ admission.MutationInterface = &persistentVolumeLabel{}
@@ -71,7 +69,7 @@ var _ kubeapiserveradmission.WantsCloudConfig = &persistentVolumeLabel{}
 // As a side effect, the cloud provider may block invalid or non-existent volumes.
 func newPersistentVolumeLabel() *persistentVolumeLabel {
 	// DEPRECATED: in a future release, we will use mutating admission webhooks to apply PV labels.
-	// Once the mutating admission webhook is used for Azure, and GCE,
+	// Once the mutating admission webhook is used for GCE,
 	// this admission controller will be removed.
 	klog.Warning("PersistentVolumeLabel admission controller is deprecated. " +
 		"Please remove this controller from your configuration files and scripts.")
@@ -205,18 +203,6 @@ func (l *persistentVolumeLabel) findVolumeLabels(volume *api.PersistentVolume) (
 			return nil, fmt.Errorf("error querying GCE PD volume %s: %v", volume.Spec.GCEPersistentDisk.PDName, err)
 		}
 		return labels, nil
-	case volume.Spec.AzureDisk != nil:
-		labels, err := l.findAzureDiskLabels(volume)
-		if err != nil {
-			return nil, fmt.Errorf("error querying AzureDisk volume %s: %v", volume.Spec.AzureDisk.DiskName, err)
-		}
-		return labels, nil
-	case volume.Spec.VsphereVolume != nil:
-		labels, err := l.findVsphereVolumeLabels(volume)
-		if err != nil {
-			return nil, fmt.Errorf("error querying vSphere Volume %s: %v", volume.Spec.VsphereVolume.VolumePath, err)
-		}
-		return labels, nil
 	}
 	// Unrecognized volume, do not add any labels
 	return nil, nil
@@ -270,96 +256,3 @@ func (l *persistentVolumeLabel) getGCEPVLabeler() (cloudprovider.PVLabeler, erro
 	}
 	return l.gcePVLabeler, nil
 }
-
-// getAzurePVLabeler returns the Azure implementation of PVLabeler
-func (l *persistentVolumeLabel) getAzurePVLabeler() (cloudprovider.PVLabeler, error) {
-	l.mutex.Lock()
-	defer l.mutex.Unlock()
-
-	if l.azurePVLabeler == nil {
-		var cloudConfigReader io.Reader
-		if len(l.cloudConfig) > 0 {
-			cloudConfigReader = bytes.NewReader(l.cloudConfig)
-		}
-
-		cloudProvider, err := cloudprovider.GetCloudProvider("azure", cloudConfigReader)
-		if err != nil || cloudProvider == nil {
-			return nil, err
-		}
-
-		azurePVLabeler, ok := cloudProvider.(cloudprovider.PVLabeler)
-		if !ok {
-			return nil, errors.New("Azure cloud provider does not implement PV labeling")
-		}
-		l.azurePVLabeler = azurePVLabeler
-	}
-
-	return l.azurePVLabeler, nil
-}
-
-func (l *persistentVolumeLabel) findAzureDiskLabels(volume *api.PersistentVolume) (map[string]string, error) {
-	// Ignore any volumes that are being provisioned
-	if volume.Spec.AzureDisk.DiskName == cloudvolume.ProvisionedVolumeName {
-		return nil, nil
-	}
-
-	pvlabler, err := l.getAzurePVLabeler()
-	if err != nil {
-		return nil, err
-	}
-	if pvlabler == nil {
-		return nil, fmt.Errorf("unable to build Azure cloud provider for AzureDisk")
-	}
-
-	pv := &v1.PersistentVolume{}
-	err = k8s_api_v1.Convert_core_PersistentVolume_To_v1_PersistentVolume(volume, pv, nil)
-	if err != nil {
-		return nil, fmt.Errorf("failed to convert PersistentVolume to core/v1: %q", err)
-	}
-	return pvlabler.GetLabelsForVolume(context.TODO(), pv)
-}
-
-func (l *persistentVolumeLabel) findVsphereVolumeLabels(volume *api.PersistentVolume) (map[string]string, error) {
-	pvlabler, err := l.getVspherePVLabeler()
-	if err != nil {
-		return nil, err
-	}
-	if pvlabler == nil {
-		return nil, fmt.Errorf("unable to build vSphere cloud provider")
-	}
-
-	pv := &v1.PersistentVolume{}
-	err = k8s_api_v1.Convert_core_PersistentVolume_To_v1_PersistentVolume(volume, pv, nil)
-	if err != nil {
-		return nil, fmt.Errorf("failed to convert PersistentVolume to core/v1: %q", err)
-	}
-	labels, err := pvlabler.GetLabelsForVolume(context.TODO(), pv)
-	if err != nil {
-		return nil, err
-	}
-
-	return labels, nil
-}
-
-func (l *persistentVolumeLabel) getVspherePVLabeler() (cloudprovider.PVLabeler, error) {
-	l.mutex.Lock()
-	defer l.mutex.Unlock()
-
-	if l.vspherePVLabeler == nil {
-		var cloudConfigReader io.Reader
-		if len(l.cloudConfig) > 0 {
-			cloudConfigReader = bytes.NewReader(l.cloudConfig)
-		}
-		cloudProvider, err := cloudprovider.GetCloudProvider("vsphere", cloudConfigReader)
-		if err != nil || cloudProvider == nil {
-			return nil, err
-		}
-		vspherePVLabeler, ok := cloudProvider.(cloudprovider.PVLabeler)
-		if !ok {
-			// GetCloudProvider failed
-			return nil, errors.New("vSphere Cloud Provider does not implement PV labeling")
-		}
-		l.vspherePVLabeler = vspherePVLabeler
-	}
-	return l.vspherePVLabeler, nil
-}

plugin/pkg/admission/storage/persistentvolume/label/admission_test.go

@@ -97,26 +97,26 @@ func Test_PVLAdmission(t *testing.T) {
 			handler:   newPersistentVolumeLabel(),
 			pvlabeler: mockVolumeFailure(errors.New("invalid volume")),
 			preAdmissionPV: &api.PersistentVolume{
-				ObjectMeta: metav1.ObjectMeta{Name: "vSpherePV", Namespace: "myns"},
+				ObjectMeta: metav1.ObjectMeta{Name: "gcepd", Namespace: "myns"},
 				Spec: api.PersistentVolumeSpec{
 					PersistentVolumeSource: api.PersistentVolumeSource{
-						VsphereVolume: &api.VsphereVirtualDiskVolumeSource{
+						GCEPersistentDisk: &api.GCEPersistentDiskVolumeSource{
-							VolumePath: "123",
+							PDName: "123",
 						},
 					},
 				},
 			},
 			postAdmissionPV: &api.PersistentVolume{
-				ObjectMeta: metav1.ObjectMeta{Name: "vSpherePV", Namespace: "myns"},
+				ObjectMeta: metav1.ObjectMeta{Name: "gcepd", Namespace: "myns"},
 				Spec: api.PersistentVolumeSpec{
 					PersistentVolumeSource: api.PersistentVolumeSource{
-						VsphereVolume: &api.VsphereVirtualDiskVolumeSource{
+						GCEPersistentDisk: &api.GCEPersistentDiskVolumeSource{
-							VolumePath: "123",
+							PDName: "123",
 						},
 					},
 				},
 			},
-			err: apierrors.NewForbidden(schema.ParseGroupResource("persistentvolumes"), "vSpherePV", errors.New("error querying vSphere Volume 123: invalid volume")),
+			err: apierrors.NewForbidden(schema.ParseGroupResource("persistentvolumes"), "gcepd", errors.New("error querying GCE PD volume 123: invalid volume")),
 		},
 		{
 			name:      "cloud provider returns no labels",
@@ -315,7 +315,7 @@ func Test_PVLAdmission(t *testing.T) {
 			}),
 			preAdmissionPV: &api.PersistentVolume{
 				ObjectMeta: metav1.ObjectMeta{
-					Name: "vSpherePV", Namespace: "myns",
+					Name: "gcePV", Namespace: "myns",
 					Labels: map[string]string{
 						v1.LabelTopologyZone:   "existingDomain",
 						v1.LabelTopologyRegion: "existingRegion",
@@ -323,15 +323,15 @@ func Test_PVLAdmission(t *testing.T) {
 				},
 				Spec: api.PersistentVolumeSpec{
 					PersistentVolumeSource: api.PersistentVolumeSource{
-						VsphereVolume: &api.VsphereVirtualDiskVolumeSource{
+						GCEPersistentDisk: &api.GCEPersistentDiskVolumeSource{
-							VolumePath: "123",
+							PDName: "123",
 						},
 					},
 				},
 			},
 			postAdmissionPV: &api.PersistentVolume{
 				ObjectMeta: metav1.ObjectMeta{
-					Name:      "vSpherePV",
+					Name:      "gcePV",
 					Namespace: "myns",
 					Labels: map[string]string{
 						v1.LabelTopologyZone:   "domain1",
@@ -340,8 +340,8 @@ func Test_PVLAdmission(t *testing.T) {
 				},
 				Spec: api.PersistentVolumeSpec{
 					PersistentVolumeSource: api.PersistentVolumeSource{
-						VsphereVolume: &api.VsphereVirtualDiskVolumeSource{
+						GCEPersistentDisk: &api.GCEPersistentDiskVolumeSource{
-							VolumePath: "123",
+							PDName: "123",
 						},
 					},
 					NodeAffinity: &api.VolumeNodeAffinity{
@@ -431,252 +431,6 @@ func Test_PVLAdmission(t *testing.T) {
 			},
 			err: nil,
 		},
-		{
-			name:    "Azure Disk PV labeled correctly",
-			handler: newPersistentVolumeLabel(),
-			pvlabeler: mockVolumeLabels(map[string]string{
-				"a":                           "1",
-				"b":                           "2",
-				v1.LabelFailureDomainBetaZone: "1__2__3",
-			}),
-			preAdmissionPV: &api.PersistentVolume{
-				ObjectMeta: metav1.ObjectMeta{
-					Name:      "azurepd",
-					Namespace: "myns",
-				},
-				Spec: api.PersistentVolumeSpec{
-					PersistentVolumeSource: api.PersistentVolumeSource{
-						AzureDisk: &api.AzureDiskVolumeSource{
-							DiskName: "123",
-						},
-					},
-				},
-			},
-			postAdmissionPV: &api.PersistentVolume{
-				ObjectMeta: metav1.ObjectMeta{
-					Name:      "azurepd",
-					Namespace: "myns",
-					Labels: map[string]string{
-						"a":                           "1",
-						"b":                           "2",
-						v1.LabelFailureDomainBetaZone: "1__2__3",
-					},
-				},
-				Spec: api.PersistentVolumeSpec{
-					PersistentVolumeSource: api.PersistentVolumeSource{
-						AzureDisk: &api.AzureDiskVolumeSource{
-							DiskName: "123",
-						},
-					},
-					NodeAffinity: &api.VolumeNodeAffinity{
-						Required: &api.NodeSelector{
-							NodeSelectorTerms: []api.NodeSelectorTerm{
-								{
-									MatchExpressions: []api.NodeSelectorRequirement{
-										{
-											Key:      "a",
-											Operator: api.NodeSelectorOpIn,
-											Values:   []string{"1"},
-										},
-										{
-											Key:      "b",
-											Operator: api.NodeSelectorOpIn,
-											Values:   []string{"2"},
-										},
-										{
-											Key:      v1.LabelFailureDomainBetaZone,
-											Operator: api.NodeSelectorOpIn,
-											Values:   []string{"1", "2", "3"},
-										},
-									},
-								},
-							},
-						},
-					},
-				},
-			},
-			err: nil,
-		},
-		{
-			name:    "vSphere PV non-conflicting affinity rules added",
-			handler: newPersistentVolumeLabel(),
-			pvlabeler: mockVolumeLabels(map[string]string{
-				"d": "1",
-				"e": "2",
-				"f": "3",
-			}),
-			preAdmissionPV: &api.PersistentVolume{
-				ObjectMeta: metav1.ObjectMeta{
-					Name:      "vSpherePV",
-					Namespace: "myns",
-					Labels: map[string]string{
-						"a": "1",
-						"b": "2",
-						"c": "3",
-					},
-				},
-				Spec: api.PersistentVolumeSpec{
-					PersistentVolumeSource: api.PersistentVolumeSource{
-						VsphereVolume: &api.VsphereVirtualDiskVolumeSource{
-							VolumePath: "123",
-						},
-					},
-					NodeAffinity: &api.VolumeNodeAffinity{
-						Required: &api.NodeSelector{
-							NodeSelectorTerms: []api.NodeSelectorTerm{
-								{
-									MatchExpressions: []api.NodeSelectorRequirement{
-										{
-											Key:      "a",
-											Operator: api.NodeSelectorOpIn,
-											Values:   []string{"1"},
-										},
-										{
-											Key:      "b",
-											Operator: api.NodeSelectorOpIn,
-											Values:   []string{"2"},
-										},
-										{
-											Key:      "c",
-											Operator: api.NodeSelectorOpIn,
-											Values:   []string{"3"},
-										},
-									},
-								},
-							},
-						},
-					},
-				},
-			},
-			postAdmissionPV: &api.PersistentVolume{
-				ObjectMeta: metav1.ObjectMeta{
-					Name:      "vSpherePV",
-					Namespace: "myns",
-					Labels: map[string]string{
-						"a": "1",
-						"b": "2",
-						"c": "3",
-						"d": "1",
-						"e": "2",
-						"f": "3",
-					},
-				},
-				Spec: api.PersistentVolumeSpec{
-					PersistentVolumeSource: api.PersistentVolumeSource{
-						VsphereVolume: &api.VsphereVirtualDiskVolumeSource{
-							VolumePath: "123",
-						},
-					},
-					NodeAffinity: &api.VolumeNodeAffinity{
-						Required: &api.NodeSelector{
-							NodeSelectorTerms: []api.NodeSelectorTerm{
-								{
-									MatchExpressions: []api.NodeSelectorRequirement{
-										{
-											Key:      "a",
-											Operator: api.NodeSelectorOpIn,
-											Values:   []string{"1"},
-										},
-										{
-											Key:      "b",
-											Operator: api.NodeSelectorOpIn,
-											Values:   []string{"2"},
-										},
-										{
-											Key:      "c",
-											Operator: api.NodeSelectorOpIn,
-											Values:   []string{"3"},
-										},
-										{
-											Key:      "d",
-											Operator: api.NodeSelectorOpIn,
-											Values:   []string{"1"},
-										},
-										{
-											Key:      "e",
-											Operator: api.NodeSelectorOpIn,
-											Values:   []string{"2"},
-										},
-										{
-											Key:      "f",
-											Operator: api.NodeSelectorOpIn,
-											Values:   []string{"3"},
-										},
-									},
-								},
-							},
-						},
-					},
-				},
-			},
-			err: nil,
-		},
-		{
-			name:    "vSphere PV labeled correctly",
-			handler: newPersistentVolumeLabel(),
-			pvlabeler: mockVolumeLabels(map[string]string{
-				"a":                           "1",
-				"b":                           "2",
-				v1.LabelFailureDomainBetaZone: "1__2__3",
-			}),
-			preAdmissionPV: &api.PersistentVolume{
-				ObjectMeta: metav1.ObjectMeta{
-					Name:      "vSpherePV",
-					Namespace: "myns",
-				},
-				Spec: api.PersistentVolumeSpec{
-					PersistentVolumeSource: api.PersistentVolumeSource{
-						VsphereVolume: &api.VsphereVirtualDiskVolumeSource{
-							VolumePath: "123",
-						},
-					},
-				},
-			},
-			postAdmissionPV: &api.PersistentVolume{
-				ObjectMeta: metav1.ObjectMeta{
-					Name:      "vSpherePV",
-					Namespace: "myns",
-					Labels: map[string]string{
-						"a":                           "1",
-						"b":                           "2",
-						v1.LabelFailureDomainBetaZone: "1__2__3",
-					},
-				},
-				Spec: api.PersistentVolumeSpec{
-					PersistentVolumeSource: api.PersistentVolumeSource{
-						VsphereVolume: &api.VsphereVirtualDiskVolumeSource{
-							VolumePath: "123",
-						},
-					},
-					NodeAffinity: &api.VolumeNodeAffinity{
-						Required: &api.NodeSelector{
-							NodeSelectorTerms: []api.NodeSelectorTerm{
-								{
-									MatchExpressions: []api.NodeSelectorRequirement{
-										{
-											Key:      "a",
-											Operator: api.NodeSelectorOpIn,
-											Values:   []string{"1"},
-										},
-										{
-											Key:      "b",
-											Operator: api.NodeSelectorOpIn,
-											Values:   []string{"2"},
-										},
-										{
-											Key:      v1.LabelFailureDomainBetaZone,
-											Operator: api.NodeSelectorOpIn,
-											Values:   []string{"1", "2", "3"},
-										},
-									},
-								},
-							},
-						},
-					},
-				},
-			},
-			err: nil,
-		},
 	}
 
 	for _, testcase := range testcases {
@@ -709,8 +463,6 @@ func Test_PVLAdmission(t *testing.T) {
 // the provider is then decided based on the type of PV (EBS, GCEPD, Azure Disk, etc)
 func setPVLabeler(handler *persistentVolumeLabel, pvlabeler cloudprovider.PVLabeler) {
 	handler.gcePVLabeler = pvlabeler
-	handler.azurePVLabeler = pvlabeler
-	handler.vspherePVLabeler = pvlabeler
 }
 
 // sortMatchExpressions sorts a PV's node selector match expressions by key name if it is not nil