diff --git a/plugin/pkg/admission/storage/persistentvolume/label/BUILD b/plugin/pkg/admission/storage/persistentvolume/label/BUILD index 2c327b9f0e2..86b8a7c9c04 100644 --- a/plugin/pkg/admission/storage/persistentvolume/label/BUILD +++ b/plugin/pkg/admission/storage/persistentvolume/label/BUILD @@ -16,8 +16,10 @@ go_library( deps = [ "//pkg/apis/core:go_default_library", "//pkg/apis/core/v1:go_default_library", + "//pkg/controller/volume/persistentvolume/util:go_default_library", "//pkg/kubeapiserver/admission:go_default_library", "//staging/src/k8s.io/api/core/v1:go_default_library", + "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", "//staging/src/k8s.io/apiserver/pkg/admission:go_default_library", "//staging/src/k8s.io/cloud-provider:go_default_library", "//staging/src/k8s.io/cloud-provider/volume:go_default_library", @@ -32,6 +34,7 @@ go_test( embed = [":go_default_library"], deps = [ "//pkg/apis/core:go_default_library", + "//pkg/controller/volume/persistentvolume/util:go_default_library", "//staging/src/k8s.io/api/core/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/api/errors:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", diff --git a/plugin/pkg/admission/storage/persistentvolume/label/admission.go b/plugin/pkg/admission/storage/persistentvolume/label/admission.go index ce42d676906..62fe2b3a5fe 100644 --- a/plugin/pkg/admission/storage/persistentvolume/label/admission.go +++ b/plugin/pkg/admission/storage/persistentvolume/label/admission.go @@ -25,6 +25,7 @@ import ( "sync" v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apiserver/pkg/admission" cloudprovider "k8s.io/cloud-provider" cloudvolume "k8s.io/cloud-provider/volume" @@ -32,6 +33,7 @@ import ( "k8s.io/klog" api "k8s.io/kubernetes/pkg/apis/core" k8s_api_v1 "k8s.io/kubernetes/pkg/apis/core/v1" + persistentvolume "k8s.io/kubernetes/pkg/controller/volume/persistentvolume/util" kubeapiserveradmission "k8s.io/kubernetes/pkg/kubeapiserver/admission" ) @@ -110,42 +112,11 @@ func (l *persistentVolumeLabel) Admit(ctx context.Context, a admission.Attribute return nil } - var volumeLabels map[string]string - if volume.Spec.AWSElasticBlockStore != nil { - labels, err := l.findAWSEBSLabels(volume) - if err != nil { - return admission.NewForbidden(a, fmt.Errorf("error querying AWS EBS volume %s: %v", volume.Spec.AWSElasticBlockStore.VolumeID, err)) - } - volumeLabels = labels - } - if volume.Spec.GCEPersistentDisk != nil { - labels, err := l.findGCEPDLabels(volume) - if err != nil { - return admission.NewForbidden(a, fmt.Errorf("error querying GCE PD volume %s: %v", volume.Spec.GCEPersistentDisk.PDName, err)) - } - volumeLabels = labels - } - if volume.Spec.AzureDisk != nil { - labels, err := l.findAzureDiskLabels(volume) - if err != nil { - return admission.NewForbidden(a, fmt.Errorf("error querying AzureDisk volume %s: %v", volume.Spec.AzureDisk.DiskName, err)) - } - volumeLabels = labels - } - if volume.Spec.Cinder != nil { - labels, err := l.findCinderDiskLabels(volume) - if err != nil { - return admission.NewForbidden(a, fmt.Errorf("error querying Cinder volume %s: %v", volume.Spec.Cinder.VolumeID, err)) - } - volumeLabels = labels - } - if volume.Spec.VsphereVolume != nil { - labels, err := l.findVsphereVolumeLabels(volume) - if err != nil { - return admission.NewForbidden(a, fmt.Errorf("error querying vSphere Volume %s: %v", volume.Spec.VsphereVolume.VolumePath, err)) - } - volumeLabels = labels + volumeLabels, err := l.findVolumeLabels(volume) + if err != nil { + return admission.NewForbidden(a, err) } + requirements := make([]api.NodeSelectorRequirement, 0) if len(volumeLabels) != 0 { if volume.Labels == nil { @@ -197,6 +168,58 @@ func (l *persistentVolumeLabel) Admit(ctx context.Context, a admission.Attribute return nil } +func (l *persistentVolumeLabel) findVolumeLabels(volume *api.PersistentVolume) (map[string]string, error) { + existingLabels := volume.Labels + + // All cloud providers set only these two labels. + domain, domainOK := existingLabels[v1.LabelZoneFailureDomain] + region, regionOK := existingLabels[v1.LabelZoneRegion] + isDynamicallyProvisioned := metav1.HasAnnotation(volume.ObjectMeta, persistentvolume.AnnDynamicallyProvisioned) + if isDynamicallyProvisioned && domainOK && regionOK { + // PV already has all the labels and we can trust the dynamic provisioning that it provided correct values. + return map[string]string{ + v1.LabelZoneFailureDomain: domain, + v1.LabelZoneRegion: region, + }, nil + } + + // Either missing labels or we don't trust the user provided correct values. + switch { + case volume.Spec.AWSElasticBlockStore != nil: + labels, err := l.findAWSEBSLabels(volume) + if err != nil { + return nil, fmt.Errorf("error querying AWS EBS volume %s: %v", volume.Spec.AWSElasticBlockStore.VolumeID, err) + } + return labels, nil + case volume.Spec.GCEPersistentDisk != nil: + labels, err := l.findGCEPDLabels(volume) + if err != nil { + 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.Cinder != nil: + labels, err := l.findCinderDiskLabels(volume) + if err != nil { + return nil, fmt.Errorf("error querying Cinder volume %s: %v", volume.Spec.Cinder.VolumeID, 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 +} + func (l *persistentVolumeLabel) findAWSEBSLabels(volume *api.PersistentVolume) (map[string]string, error) { // Ignore any volumes that are being provisioned if volume.Spec.AWSElasticBlockStore.VolumeID == cloudvolume.ProvisionedVolumeName { diff --git a/plugin/pkg/admission/storage/persistentvolume/label/admission_test.go b/plugin/pkg/admission/storage/persistentvolume/label/admission_test.go index eff70238b2e..ae890c8daa0 100644 --- a/plugin/pkg/admission/storage/persistentvolume/label/admission_test.go +++ b/plugin/pkg/admission/storage/persistentvolume/label/admission_test.go @@ -31,6 +31,7 @@ import ( admissiontesting "k8s.io/apiserver/pkg/admission/testing" cloudprovider "k8s.io/cloud-provider" api "k8s.io/kubernetes/pkg/apis/core" + persistentvolume "k8s.io/kubernetes/pkg/controller/volume/persistentvolume/util" ) type mockVolumes struct { @@ -232,6 +233,136 @@ func Test_PVLAdmission(t *testing.T) { }, err: nil, }, + { + name: "existing labels from dynamic provisioning are not changed", + handler: newPersistentVolumeLabel(), + pvlabeler: mockVolumeLabels(map[string]string{ + v1.LabelZoneFailureDomain: "domain1", + v1.LabelZoneRegion: "region1", + }), + preAdmissionPV: &api.PersistentVolume{ + ObjectMeta: metav1.ObjectMeta{ + Name: "awsebs", Namespace: "myns", + Labels: map[string]string{ + v1.LabelZoneFailureDomain: "existingDomain", + v1.LabelZoneRegion: "existingRegion", + }, + Annotations: map[string]string{ + persistentvolume.AnnDynamicallyProvisioned: "kubernetes.io/aws-ebs", + }, + }, + Spec: api.PersistentVolumeSpec{ + PersistentVolumeSource: api.PersistentVolumeSource{ + AWSElasticBlockStore: &api.AWSElasticBlockStoreVolumeSource{ + VolumeID: "123", + }, + }, + }, + }, + postAdmissionPV: &api.PersistentVolume{ + ObjectMeta: metav1.ObjectMeta{ + Name: "awsebs", + Namespace: "myns", + Labels: map[string]string{ + v1.LabelZoneFailureDomain: "existingDomain", + v1.LabelZoneRegion: "existingRegion", + }, + Annotations: map[string]string{ + persistentvolume.AnnDynamicallyProvisioned: "kubernetes.io/aws-ebs", + }, + }, + Spec: api.PersistentVolumeSpec{ + PersistentVolumeSource: api.PersistentVolumeSource{ + AWSElasticBlockStore: &api.AWSElasticBlockStoreVolumeSource{ + VolumeID: "123", + }, + }, + NodeAffinity: &api.VolumeNodeAffinity{ + Required: &api.NodeSelector{ + NodeSelectorTerms: []api.NodeSelectorTerm{ + { + MatchExpressions: []api.NodeSelectorRequirement{ + { + Key: v1.LabelZoneRegion, + Operator: api.NodeSelectorOpIn, + Values: []string{"existingRegion"}, + }, + { + Key: v1.LabelZoneFailureDomain, + Operator: api.NodeSelectorOpIn, + Values: []string{"existingDomain"}, + }, + }, + }, + }, + }, + }, + }, + }, + err: nil, + }, + { + name: "existing labels from user are changed", + handler: newPersistentVolumeLabel(), + pvlabeler: mockVolumeLabels(map[string]string{ + v1.LabelZoneFailureDomain: "domain1", + v1.LabelZoneRegion: "region1", + }), + preAdmissionPV: &api.PersistentVolume{ + ObjectMeta: metav1.ObjectMeta{ + Name: "awsebs", Namespace: "myns", + Labels: map[string]string{ + v1.LabelZoneFailureDomain: "existingDomain", + v1.LabelZoneRegion: "existingRegion", + }, + }, + Spec: api.PersistentVolumeSpec{ + PersistentVolumeSource: api.PersistentVolumeSource{ + AWSElasticBlockStore: &api.AWSElasticBlockStoreVolumeSource{ + VolumeID: "123", + }, + }, + }, + }, + postAdmissionPV: &api.PersistentVolume{ + ObjectMeta: metav1.ObjectMeta{ + Name: "awsebs", + Namespace: "myns", + Labels: map[string]string{ + v1.LabelZoneFailureDomain: "domain1", + v1.LabelZoneRegion: "region1", + }, + }, + Spec: api.PersistentVolumeSpec{ + PersistentVolumeSource: api.PersistentVolumeSource{ + AWSElasticBlockStore: &api.AWSElasticBlockStoreVolumeSource{ + VolumeID: "123", + }, + }, + NodeAffinity: &api.VolumeNodeAffinity{ + Required: &api.NodeSelector{ + NodeSelectorTerms: []api.NodeSelectorTerm{ + { + MatchExpressions: []api.NodeSelectorRequirement{ + { + Key: v1.LabelZoneRegion, + Operator: api.NodeSelectorOpIn, + Values: []string{"region1"}, + }, + { + Key: v1.LabelZoneFailureDomain, + Operator: api.NodeSelectorOpIn, + Values: []string{"domain1"}, + }, + }, + }, + }, + }, + }, + }, + }, + err: nil, + }, { name: "GCE PD PV labeled correctly", handler: newPersistentVolumeLabel(),