From 4d6d9817b0ec6afd9b2fadaf9dae336436a6695f Mon Sep 17 00:00:00 2001 From: pospispa Date: Wed, 22 Nov 2017 10:06:49 +0100 Subject: [PATCH] PVC Being Deleted Checks in kubelet Kubelet must not start pods that use PVCs that are being deleted. --- pkg/kubelet/volumemanager/populator/BUILD | 1 + .../desired_state_of_world_populator.go | 21 ++++++- pkg/volume/util/BUILD | 2 + pkg/volume/util/finalizer.go | 28 +++++++++ pkg/volume/util/finalizer_test.go | 58 +++++++++++++++++++ 5 files changed, 109 insertions(+), 1 deletion(-) create mode 100644 pkg/volume/util/finalizer.go create mode 100644 pkg/volume/util/finalizer_test.go diff --git a/pkg/kubelet/volumemanager/populator/BUILD b/pkg/kubelet/volumemanager/populator/BUILD index f8938c1ce3..a3bd36cc05 100644 --- a/pkg/kubelet/volumemanager/populator/BUILD +++ b/pkg/kubelet/volumemanager/populator/BUILD @@ -19,6 +19,7 @@ go_library( "//pkg/kubelet/util/format:go_default_library", "//pkg/kubelet/volumemanager/cache:go_default_library", "//pkg/volume:go_default_library", + "//pkg/volume/util:go_default_library", "//pkg/volume/util/types:go_default_library", "//pkg/volume/util/volumehelper:go_default_library", "//vendor/github.com/golang/glog:go_default_library", diff --git a/pkg/kubelet/volumemanager/populator/desired_state_of_world_populator.go b/pkg/kubelet/volumemanager/populator/desired_state_of_world_populator.go index a9ecaaad69..ac762a040f 100644 --- a/pkg/kubelet/volumemanager/populator/desired_state_of_world_populator.go +++ b/pkg/kubelet/volumemanager/populator/desired_state_of_world_populator.go @@ -41,6 +41,7 @@ import ( "k8s.io/kubernetes/pkg/kubelet/util/format" "k8s.io/kubernetes/pkg/kubelet/volumemanager/cache" "k8s.io/kubernetes/pkg/volume" + volumeutil "k8s.io/kubernetes/pkg/volume/util" volumetypes "k8s.io/kubernetes/pkg/volume/util/types" "k8s.io/kubernetes/pkg/volume/util/volumehelper" ) @@ -419,7 +420,8 @@ func (dswp *desiredStateOfWorldPopulator) createVolumeSpec( } // getPVCExtractPV fetches the PVC object with the given namespace and name from -// the API server extracts the name of the PV it is pointing to and returns it. +// the API server, checks whether PVC is being deleted, extracts the name of the PV +// it is pointing to and returns it. // An error is returned if the PVC object's phase is not "Bound". func (dswp *desiredStateOfWorldPopulator) getPVCExtractPV( namespace string, claimName string) (string, types.UID, error) { @@ -433,6 +435,23 @@ func (dswp *desiredStateOfWorldPopulator) getPVCExtractPV( err) } + if utilfeature.DefaultFeatureGate.Enabled(features.PVCProtection) { + // Pods that uses a PVC that is being deleted must not be started. + // + // In case an old kubelet is running without this check or some kubelets + // have this feature disabled, the worst that can happen is that such + // pod is scheduled. This was the default behavior in 1.8 and earlier + // and users should not be that surprised. + // It should happen only in very rare case when scheduler schedules + // a pod and user deletes a PVC that's used by it at the same time. + if volumeutil.IsPVCBeingDeleted(pvc) { + return "", "", fmt.Errorf( + "can't start pod because PVC %s/%s is being deleted", + namespace, + claimName) + } + } + if pvc.Status.Phase != v1.ClaimBound || pvc.Spec.VolumeName == "" { return "", "", fmt.Errorf( diff --git a/pkg/volume/util/BUILD b/pkg/volume/util/BUILD index 3c94453fd4..2ceb17c6d3 100644 --- a/pkg/volume/util/BUILD +++ b/pkg/volume/util/BUILD @@ -14,6 +14,7 @@ go_library( "device_util_unsupported.go", "doc.go", "error.go", + "finalizer.go", "fs_unsupported.go", "io_util.go", "metrics.go", @@ -61,6 +62,7 @@ go_library( go_test( name = "go_default_test", srcs = [ + "finalizer_test.go", "util_test.go", ] + select({ "@io_bazel_rules_go//go/platform:linux_amd64": [ diff --git a/pkg/volume/util/finalizer.go b/pkg/volume/util/finalizer.go new file mode 100644 index 0000000000..db22ff2578 --- /dev/null +++ b/pkg/volume/util/finalizer.go @@ -0,0 +1,28 @@ +/* +Copyright 2017 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package util + +import ( + "k8s.io/api/core/v1" +) + +// IsPVCBeingDeleted returns: +// true: in case PVC is being deleted, i.e. ObjectMeta.DeletionTimestamp is set +// false: in case PVC is not being deleted, i.e. ObjectMeta.DeletionTimestamp is nil +func IsPVCBeingDeleted(pvc *v1.PersistentVolumeClaim) bool { + return pvc.ObjectMeta.DeletionTimestamp != nil +} diff --git a/pkg/volume/util/finalizer_test.go b/pkg/volume/util/finalizer_test.go new file mode 100644 index 0000000000..16bef4c7ba --- /dev/null +++ b/pkg/volume/util/finalizer_test.go @@ -0,0 +1,58 @@ +/* +Copyright 2017 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package util + +import ( + "testing" + "time" + + "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +var ( + arbitraryTime = metav1.Date(2017, 11, 1, 14, 28, 47, 0, time.FixedZone("CET", 0)) +) + +func TestIsPVCBeingDeleted(t *testing.T) { + tests := []struct { + pvc *v1.PersistentVolumeClaim + want bool + }{ + { + pvc: &v1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + DeletionTimestamp: nil, + }, + }, + want: false, + }, + { + pvc: &v1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + DeletionTimestamp: &arbitraryTime, + }, + }, + want: true, + }, + } + for _, tt := range tests { + if got := IsPVCBeingDeleted(tt.pvc); got != tt.want { + t.Errorf("IsPVCBeingDeleted(%v) = %v WANT %v", tt.pvc, got, tt.want) + } + } +}