Skip to content

Commit

Permalink
Merge pull request #344 from davidz627/13
Browse files Browse the repository at this point in the history 
Ignore non-migrated in-tree PVC when provisioning
  • Loading branch information
@k8s-ci-robot
k8s-ci-robot authored Oct 1, 2019
2 parents 5c5deb3 + 44e6eb3 commit 9bccfb1
Show file tree
Hide file tree
Showing 3 changed files with 106 additions and 29 deletions.
2 changes: 1 addition & 1 deletion cmd/csi-provisioner/csi-provisioner.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ func main() {
} else if *leaderElectionType == "leases" {
le = leaderelection.NewLeaderElection(clientset, lockName, run)
} else {
klog.Error("--leader-election-type must be either 'endpoints' or 'lease'")
klog.Error("--leader-election-type must be either 'endpoints' or 'leases'")
os.Exit(1)
}

Expand Down
10 changes: 10 additions & 0 deletions pkg/controller/controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,8 @@ const (
tokenPVNameKey = "pv.name"
tokenPVCNameKey = "pvc.name"
tokenPVCNameSpaceKey = "pvc.namespace"

annStorageProvisioner = "volume.beta.kubernetes.io/storage-provisioner"
)

var (
Expand Down Expand Up @@ -367,6 +369,14 @@ func (p *csiProvisioner) ProvisionExt(options controller.ProvisionOptions) (*v1.
return nil, controller.ProvisioningFinished, errors.New("storage class was nil")
}

if options.PVC.Annotations[annStorageProvisioner] != p.driverName {
return nil, controller.ProvisioningFinished, &controller.IgnoredError{
Reason: fmt.Sprintf("PVC annotated with external-provisioner name %s does not match provisioner driver name %s. This could mean the PVC is not migrated",
options.PVC.Annotations[annStorageProvisioner],
p.driverName),
}
}

migratedVolume := false
if p.supportsMigrationFromInTreePluginName != "" {
// NOTE: we cannot depend on PVC.Annotations[volume.beta.kubernetes.io/storage-provisioner] to get
Expand Down
123 changes: 95 additions & 28 deletions pkg/controller/controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,8 @@ const (
var (
volumeModeFileSystem = v1.PersistentVolumeFilesystem
volumeModeBlock = v1.PersistentVolumeBlock

driverNameAnnotation = map[string]string{annStorageProvisioner: driverName}
)

type csiConnection struct {
Expand Down Expand Up @@ -472,8 +474,9 @@ func provisionFromPVCCapabilities() (connection.PluginCapabilitySet, connection.
func createFakePVC(requestBytes int64) *v1.PersistentVolumeClaim {
return &v1.PersistentVolumeClaim{
ObjectMeta: metav1.ObjectMeta{
UID: "testid",
Name: "fake-pvc",
UID: "testid",
Name: "fake-pvc",
Annotations: map[string]string{annStorageProvisioner: driverName},
},
Spec: v1.PersistentVolumeClaimSpec{
Selector: nil, // Provisioner doesn't support selector
Expand Down Expand Up @@ -874,7 +877,8 @@ func TestProvision(t *testing.T) {
PVName: "test-name",
PVC: &v1.PersistentVolumeClaim{
ObjectMeta: metav1.ObjectMeta{
UID: "testid",
UID: "testid",
Annotations: driverNameAnnotation,
},
Spec: v1.PersistentVolumeClaimSpec{
Selector: nil,
Expand Down Expand Up @@ -925,7 +929,8 @@ func TestProvision(t *testing.T) {
PVName: "test-name",
PVC: &v1.PersistentVolumeClaim{
ObjectMeta: metav1.ObjectMeta{
UID: "testid",
UID: "testid",
Annotations: driverNameAnnotation,
},
Spec: v1.PersistentVolumeClaimSpec{
Selector: nil,
Expand Down Expand Up @@ -976,7 +981,8 @@ func TestProvision(t *testing.T) {
PVName: "test-name",
PVC: &v1.PersistentVolumeClaim{
ObjectMeta: metav1.ObjectMeta{
UID: "testid",
UID: "testid",
Annotations: driverNameAnnotation,
},
Spec: v1.PersistentVolumeClaimSpec{
Selector: nil,
Expand Down Expand Up @@ -1027,7 +1033,8 @@ func TestProvision(t *testing.T) {
PVName: "test-name",
PVC: &v1.PersistentVolumeClaim{
ObjectMeta: metav1.ObjectMeta{
UID: "testid",
UID: "testid",
Annotations: driverNameAnnotation,
},
Spec: v1.PersistentVolumeClaimSpec{
Selector: nil,
Expand Down Expand Up @@ -1235,7 +1242,8 @@ func TestProvision(t *testing.T) {
PVName: "test-name",
PVC: &v1.PersistentVolumeClaim{
ObjectMeta: metav1.ObjectMeta{
UID: "testid",
UID: "testid",
Annotations: driverNameAnnotation,
},
Spec: v1.PersistentVolumeClaimSpec{
Selector: nil,
Expand Down Expand Up @@ -1293,7 +1301,8 @@ func TestProvision(t *testing.T) {
PVName: "test-name",
PVC: &v1.PersistentVolumeClaim{
ObjectMeta: metav1.ObjectMeta{
UID: "testid",
UID: "testid",
Annotations: driverNameAnnotation,
},
Spec: v1.PersistentVolumeClaimSpec{
Selector: nil,
Expand Down Expand Up @@ -1322,7 +1331,8 @@ func TestProvision(t *testing.T) {
PVName: "test-name",
PVC: &v1.PersistentVolumeClaim{
ObjectMeta: metav1.ObjectMeta{
UID: "testid",
UID: "testid",
Annotations: driverNameAnnotation,
},
Spec: v1.PersistentVolumeClaimSpec{
Selector: nil,
Expand All @@ -1345,7 +1355,49 @@ func TestProvision(t *testing.T) {
}

for k, tc := range testcases {
runProvisionTest(t, k, tc, requestedBytes)
runProvisionTest(t, k, tc, requestedBytes, driverName, "" /* no migration */)
}
}

func TestProvisionWithMigration(t *testing.T) {
inTreePluginName := "kubernetes.io/gce-pd"
migrationDriverName := "pd.csi.storage.gke.io"
var requestBytes int64 = 100000

deletePolicy := v1.PersistentVolumeReclaimDelete
testcases := map[string]provisioningTestcase{
"should ignore in-tree with migration": {
volOpts: controller.ProvisionOptions{
StorageClass: &storagev1.StorageClass{
Provisioner: inTreePluginName,
Parameters: map[string]string{
"fstype": "ext3",
},
ReclaimPolicy: &deletePolicy,
},
PVName: "test-name",
PVC: &v1.PersistentVolumeClaim{
ObjectMeta: metav1.ObjectMeta{
UID: "testid",
Name: "fake-pvc",
Annotations: map[string]string{annStorageProvisioner: inTreePluginName},
},
Spec: v1.PersistentVolumeClaimSpec{
Selector: nil, // Provisioner doesn't support selector
Resources: v1.ResourceRequirements{
Requests: v1.ResourceList{
v1.ResourceName(v1.ResourceStorage): resource.MustParse(strconv.FormatInt(requestBytes, 10)),
},
},
},
},
},
expectErr: true,
},
}

for k, tc := range testcases {
runProvisionTest(t, k, tc, requestBytes, migrationDriverName, inTreePluginName)
}
}

Expand Down Expand Up @@ -1378,7 +1430,7 @@ func newSnapshot(name, className, boundToContent, snapshotUID, claimName string,
return &snapshot
}

func runProvisionTest(t *testing.T, k string, tc provisioningTestcase, requestedBytes int64) {
func runProvisionTest(t *testing.T, k string, tc provisioningTestcase, requestedBytes int64, provisionDriverName, supportsMigrationFromInTreePluginName string) {
t.Logf("Running test: %v", k)

tmpdir := tempDir(t)
Expand Down Expand Up @@ -1419,7 +1471,7 @@ func runProvisionTest(t *testing.T, k string, tc provisioningTestcase, requested
}

pluginCaps, controllerCaps := provisionCapabilities()
csiProvisioner := NewCSIProvisioner(clientSet, 5*time.Second, "test-provisioner", "test", 5, csiConn.conn, nil, driverName, pluginCaps, controllerCaps, "", false)
csiProvisioner := NewCSIProvisioner(clientSet, 5*time.Second, "test-provisioner", "test", 5, csiConn.conn, nil, provisionDriverName, pluginCaps, controllerCaps, supportsMigrationFromInTreePluginName, false)

out := &csi.CreateVolumeResponse{
Volume: &csi.Volume{
Expand Down Expand Up @@ -1466,7 +1518,7 @@ func runProvisionTest(t *testing.T, k string, tc provisioningTestcase, requested
t.Errorf("test %q: Expected error, got none", k)
}
if !tc.expectErr && err != nil {
t.Errorf("test %q: got error: %v", k, err)
t.Fatalf("test %q: got error: %v", k, err)
}

if tc.expectState == "" {
Expand Down Expand Up @@ -1582,7 +1634,8 @@ func TestProvisionFromSnapshot(t *testing.T) {
PVName: "test-name",
PVC: &v1.PersistentVolumeClaim{
ObjectMeta: metav1.ObjectMeta{
UID: "testid",
UID: "testid",
Annotations: driverNameAnnotation,
},
Spec: v1.PersistentVolumeClaimSpec{
Selector: nil,
Expand Down Expand Up @@ -1626,7 +1679,8 @@ func TestProvisionFromSnapshot(t *testing.T) {
PVName: "test-name",
PVC: &v1.PersistentVolumeClaim{
ObjectMeta: metav1.ObjectMeta{
UID: "testid",
UID: "testid",
Annotations: driverNameAnnotation,
},
Spec: v1.PersistentVolumeClaimSpec{
Selector: nil,
Expand Down Expand Up @@ -1656,7 +1710,8 @@ func TestProvisionFromSnapshot(t *testing.T) {
PVName: "test-name",
PVC: &v1.PersistentVolumeClaim{
ObjectMeta: metav1.ObjectMeta{
UID: "testid",
UID: "testid",
Annotations: driverNameAnnotation,
},
Spec: v1.PersistentVolumeClaimSpec{
Selector: nil,
Expand Down Expand Up @@ -1685,7 +1740,8 @@ func TestProvisionFromSnapshot(t *testing.T) {
PVName: "test-name",
PVC: &v1.PersistentVolumeClaim{
ObjectMeta: metav1.ObjectMeta{
UID: "testid",
UID: "testid",
Annotations: driverNameAnnotation,
},
Spec: v1.PersistentVolumeClaimSpec{
Selector: nil,
Expand Down Expand Up @@ -1714,7 +1770,8 @@ func TestProvisionFromSnapshot(t *testing.T) {
PVName: "test-name",
PVC: &v1.PersistentVolumeClaim{
ObjectMeta: metav1.ObjectMeta{
UID: "testid",
UID: "testid",
Annotations: driverNameAnnotation,
},
Spec: v1.PersistentVolumeClaimSpec{
Selector: nil,
Expand Down Expand Up @@ -1743,7 +1800,8 @@ func TestProvisionFromSnapshot(t *testing.T) {
PVName: "test-name",
PVC: &v1.PersistentVolumeClaim{
ObjectMeta: metav1.ObjectMeta{
UID: "testid",
UID: "testid",
Annotations: driverNameAnnotation,
},
Spec: v1.PersistentVolumeClaimSpec{
Selector: nil,
Expand Down Expand Up @@ -2265,7 +2323,8 @@ func TestProvisionFromPVC(t *testing.T) {
PVName: "test-name",
PVC: &v1.PersistentVolumeClaim{
ObjectMeta: metav1.ObjectMeta{
UID: "testid",
UID: "testid",
Annotations: driverNameAnnotation,
},
Spec: v1.PersistentVolumeClaimSpec{
StorageClassName: &fakeSc1,
Expand Down Expand Up @@ -2311,7 +2370,8 @@ func TestProvisionFromPVC(t *testing.T) {
PVName: "test-name",
PVC: &v1.PersistentVolumeClaim{
ObjectMeta: metav1.ObjectMeta{
UID: "testid",
UID: "testid",
Annotations: map[string]string{annStorageProvisioner: driverName},
},
Spec: v1.PersistentVolumeClaimSpec{
StorageClassName: &fakeSc1,
Expand Down Expand Up @@ -2343,7 +2403,8 @@ func TestProvisionFromPVC(t *testing.T) {
PVName: "test-name",
PVC: &v1.PersistentVolumeClaim{
ObjectMeta: metav1.ObjectMeta{
UID: "testid",
UID: "testid",
Annotations: map[string]string{annStorageProvisioner: driverName},
},
Spec: v1.PersistentVolumeClaimSpec{
StorageClassName: &fakeSc2,
Expand Down Expand Up @@ -2375,7 +2436,8 @@ func TestProvisionFromPVC(t *testing.T) {
PVName: "test-name",
PVC: &v1.PersistentVolumeClaim{
ObjectMeta: metav1.ObjectMeta{
UID: "testid",
UID: "testid",
Annotations: map[string]string{annStorageProvisioner: driverName},
},
Spec: v1.PersistentVolumeClaimSpec{
StorageClassName: &fakeSc1,
Expand Down Expand Up @@ -2408,7 +2470,8 @@ func TestProvisionFromPVC(t *testing.T) {
PVName: pvName,
PVC: &v1.PersistentVolumeClaim{
ObjectMeta: metav1.ObjectMeta{
UID: "testid",
UID: "testid",
Annotations: driverNameAnnotation,
},
Spec: v1.PersistentVolumeClaimSpec{
StorageClassName: &fakeSc1,
Expand Down Expand Up @@ -2441,7 +2504,8 @@ func TestProvisionFromPVC(t *testing.T) {
PVName: pvName,
PVC: &v1.PersistentVolumeClaim{
ObjectMeta: metav1.ObjectMeta{
UID: "testid",
UID: "testid",
Annotations: driverNameAnnotation,
},
Spec: v1.PersistentVolumeClaimSpec{
StorageClassName: &fakeSc1,
Expand Down Expand Up @@ -2473,7 +2537,8 @@ func TestProvisionFromPVC(t *testing.T) {
PVName: "test-name",
PVC: &v1.PersistentVolumeClaim{
ObjectMeta: metav1.ObjectMeta{
UID: "testid",
UID: "testid",
Annotations: driverNameAnnotation,
},
Spec: v1.PersistentVolumeClaimSpec{
Selector: nil,
Expand Down Expand Up @@ -2505,7 +2570,8 @@ func TestProvisionFromPVC(t *testing.T) {
PVName: pvName,
PVC: &v1.PersistentVolumeClaim{
ObjectMeta: metav1.ObjectMeta{
UID: "testid",
UID: "testid",
Annotations: driverNameAnnotation,
},
Spec: v1.PersistentVolumeClaimSpec{
Selector: nil,
Expand Down Expand Up @@ -2536,7 +2602,8 @@ func TestProvisionFromPVC(t *testing.T) {
PVName: "invalid-pv",
PVC: &v1.PersistentVolumeClaim{
ObjectMeta: metav1.ObjectMeta{
UID: "testid",
UID: "testid",
Annotations: driverNameAnnotation,
},
Spec: v1.PersistentVolumeClaimSpec{
StorageClassName: &fakeSc1,
Expand Down

0 comments on commit 9bccfb1

Please sign in to comment.