From 9cfd96dbb5efedaac920490122e29f9b32982699 Mon Sep 17 00:00:00 2001 From: Roman Bednar Date: Wed, 13 Sep 2023 11:31:23 +0200 Subject: [PATCH 1/4] graduate PersistentVolumeLastPhaseTransitionTime to beta --- pkg/features/kube_features.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/features/kube_features.go b/pkg/features/kube_features.go index 7f03686aeb6..f4c9f7ba450 100644 --- a/pkg/features/kube_features.go +++ b/pkg/features/kube_features.go @@ -1044,7 +1044,7 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS PDBUnhealthyPodEvictionPolicy: {Default: true, PreRelease: featuregate.Beta}, - PersistentVolumeLastPhaseTransitionTime: {Default: false, PreRelease: featuregate.Alpha}, + PersistentVolumeLastPhaseTransitionTime: {Default: true, PreRelease: featuregate.Beta}, PodAndContainerStatsFromCRI: {Default: false, PreRelease: featuregate.Alpha}, From 071a67d8c29a53d2c3ea9c1504bbb2cdb2b8a46e Mon Sep 17 00:00:00 2001 From: Roman Bednar Date: Wed, 13 Sep 2023 11:32:56 +0200 Subject: [PATCH 2/4] export time function NowFunc() has to be exported so it is available for testing in other packages. --- pkg/registry/core/persistentvolume/strategy.go | 6 +++--- pkg/registry/core/persistentvolume/strategy_test.go | 8 ++++---- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/pkg/registry/core/persistentvolume/strategy.go b/pkg/registry/core/persistentvolume/strategy.go index d33a3f0a925..515a9b76869 100644 --- a/pkg/registry/core/persistentvolume/strategy.go +++ b/pkg/registry/core/persistentvolume/strategy.go @@ -71,7 +71,7 @@ func (persistentvolumeStrategy) PrepareForCreate(ctx context.Context, obj runtim if utilfeature.DefaultFeatureGate.Enabled(features.PersistentVolumeLastPhaseTransitionTime) { pv.Status.Phase = api.VolumePending - now := nowFunc() + now := NowFunc() pv.Status.LastPhaseTransitionTime = &now } @@ -143,7 +143,7 @@ func (persistentvolumeStatusStrategy) GetResetFields() map[fieldpath.APIVersion] return fields } -var nowFunc = metav1.Now +var NowFunc = metav1.Now // PrepareForUpdate sets the Spec field which is not allowed to be changed when updating a PV's Status func (persistentvolumeStatusStrategy) PrepareForUpdate(ctx context.Context, obj, old runtime.Object) { @@ -159,7 +159,7 @@ func (persistentvolumeStatusStrategy) PrepareForUpdate(ctx context.Context, obj, case oldPv.Status.Phase != newPv.Status.Phase && (newPv.Status.LastPhaseTransitionTime == nil || newPv.Status.LastPhaseTransitionTime.Equal(oldPv.Status.LastPhaseTransitionTime)): // phase changed and client didn't set or didn't change the transition time - now := nowFunc() + now := NowFunc() newPv.Status.LastPhaseTransitionTime = &now } } diff --git a/pkg/registry/core/persistentvolume/strategy_test.go b/pkg/registry/core/persistentvolume/strategy_test.go index e7d9794b5fe..c5ca9d07110 100644 --- a/pkg/registry/core/persistentvolume/strategy_test.go +++ b/pkg/registry/core/persistentvolume/strategy_test.go @@ -46,9 +46,9 @@ func TestStatusUpdate(t *testing.T) { now := metav1.Now() origin := metav1.NewTime(now.Add(time.Hour)) later := metav1.NewTime(now.Add(time.Hour * 2)) - nowFunc = func() metav1.Time { return now } + NowFunc = func() metav1.Time { return now } defer func() { - nowFunc = metav1.Now + NowFunc = metav1.Now }() tests := []struct { name string @@ -376,9 +376,9 @@ func TestStatusUpdate(t *testing.T) { func TestStatusCreate(t *testing.T) { now := metav1.Now() - nowFunc = func() metav1.Time { return now } + NowFunc = func() metav1.Time { return now } defer func() { - nowFunc = metav1.Now + NowFunc = metav1.Now }() tests := []struct { name string From 53339894a1c93f4760e332ee61b9cf1fef46048e Mon Sep 17 00:00:00 2001 From: Roman Bednar Date: Thu, 26 Oct 2023 11:44:57 +0200 Subject: [PATCH 3/4] enable PersistentVolumeLastPhaseTransitionTime feature gate for status update test --- pkg/registry/core/persistentvolume/storage/storage_test.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pkg/registry/core/persistentvolume/storage/storage_test.go b/pkg/registry/core/persistentvolume/storage/storage_test.go index 271a459d2a1..e93757677a7 100644 --- a/pkg/registry/core/persistentvolume/storage/storage_test.go +++ b/pkg/registry/core/persistentvolume/storage/storage_test.go @@ -17,6 +17,9 @@ limitations under the License. package storage import ( + utilfeature "k8s.io/apiserver/pkg/util/feature" + featuregatetesting "k8s.io/component-base/featuregate/testing" + "k8s.io/kubernetes/pkg/features" "testing" "github.com/google/go-cmp/cmp" @@ -166,6 +169,7 @@ func TestWatch(t *testing.T) { } func TestUpdateStatus(t *testing.T) { + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.PersistentVolumeLastPhaseTransitionTime, true)() storage, statusStorage, server := newStorage(t) defer server.Terminate(t) defer storage.Store.DestroyFunc() From fb872e863843fc0b78f9608e3232714580dd11d4 Mon Sep 17 00:00:00 2001 From: Roman Bednar Date: Wed, 13 Sep 2023 11:36:50 +0200 Subject: [PATCH 4/4] test: fix storage status update test After enabling PersistentVolumeLastPhaseTransitionTime feature, any test that compares PV objects that transitioned phase needs to handle timestamp values correctly. Either the tests should avoid phase transitions if not needed or the test needs to set the same timestamp on new PV object so it's not changed and can be checked for equality later, the latter is used in this commit. --- .../persistentvolume/storage/storage_test.go | 25 ++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/pkg/registry/core/persistentvolume/storage/storage_test.go b/pkg/registry/core/persistentvolume/storage/storage_test.go index e93757677a7..2570ebba358 100644 --- a/pkg/registry/core/persistentvolume/storage/storage_test.go +++ b/pkg/registry/core/persistentvolume/storage/storage_test.go @@ -22,6 +22,7 @@ import ( "k8s.io/kubernetes/pkg/features" "testing" + "context" "github.com/google/go-cmp/cmp" apiequality "k8s.io/apimachinery/pkg/api/equality" "k8s.io/apimachinery/pkg/api/resource" @@ -35,6 +36,7 @@ import ( "k8s.io/apiserver/pkg/registry/rest" etcd3testing "k8s.io/apiserver/pkg/storage/etcd3/testing" api "k8s.io/kubernetes/pkg/apis/core" + "k8s.io/kubernetes/pkg/registry/core/persistentvolume" "k8s.io/kubernetes/pkg/registry/registrytest" ) @@ -60,6 +62,7 @@ func newHostPathType(pathType string) *api.HostPathType { } func validNewPersistentVolume(name string) *api.PersistentVolume { + now := persistentvolume.NowFunc() pv := &api.PersistentVolume{ ObjectMeta: metav1.ObjectMeta{ Name: name, @@ -75,9 +78,10 @@ func validNewPersistentVolume(name string) *api.PersistentVolume { PersistentVolumeReclaimPolicy: api.PersistentVolumeReclaimRetain, }, Status: api.PersistentVolumeStatus{ - Phase: api.VolumePending, - Message: "bar", - Reason: "foo", + Phase: api.VolumePending, + Message: "bar", + Reason: "foo", + LastPhaseTransitionTime: &now, }, } return pv @@ -181,12 +185,19 @@ func TestUpdateStatus(t *testing.T) { t.Errorf("unexpected error: %v", err) } + pvStartTimestamp, err := getPhaseTranstitionTime(ctx, pvStart.Name, storage) + if err != nil { + t.Errorf("unexpected error: %v", err) + } + pvIn := &api.PersistentVolume{ ObjectMeta: metav1.ObjectMeta{ Name: "foo", }, Status: api.PersistentVolumeStatus{ Phase: api.VolumeBound, + // Set the same timestamp as original PV so this won't get updated on phase change breaking DeepEqual() later in test. + LastPhaseTransitionTime: pvStartTimestamp, }, } @@ -205,6 +216,14 @@ func TestUpdateStatus(t *testing.T) { } } +func getPhaseTranstitionTime(ctx context.Context, pvName string, storage *REST) (*metav1.Time, error) { + obj, err := storage.Get(ctx, pvName, &metav1.GetOptions{}) + if err != nil { + return nil, err + } + return obj.(*api.PersistentVolume).Status.LastPhaseTransitionTime, nil +} + func TestShortNames(t *testing.T) { storage, _, server := newStorage(t) defer server.Terminate(t)