From d13fe474f9c9e42ac88e22985c503ab7217329db Mon Sep 17 00:00:00 2001 From: Haowei Cai Date: Wed, 2 Dec 2020 14:32:34 -0800 Subject: [PATCH 1/2] storage-version: update conditions --- .../storageversiongc/gc_controller.go | 22 +-- .../apiserver/pkg/storageversion/updater.go | 95 ++++++++-- .../pkg/storageversion/updater_test.go | 173 +++++++++++++++++- 3 files changed, 247 insertions(+), 43 deletions(-) diff --git a/pkg/controller/storageversiongc/gc_controller.go b/pkg/controller/storageversiongc/gc_controller.go index 0de4e94f7d2..2be00f79c2f 100644 --- a/pkg/controller/storageversiongc/gc_controller.go +++ b/pkg/controller/storageversiongc/gc_controller.go @@ -28,6 +28,7 @@ import ( utilerrors "k8s.io/apimachinery/pkg/util/errors" utilruntime "k8s.io/apimachinery/pkg/util/runtime" "k8s.io/apimachinery/pkg/util/wait" + "k8s.io/apiserver/pkg/storageversion" apiserverinternalinformers "k8s.io/client-go/informers/apiserverinternal/v1alpha1" coordinformers "k8s.io/client-go/informers/coordination/v1" "k8s.io/client-go/kubernetes" @@ -265,32 +266,13 @@ func (c *Controller) enqueueLease(obj *coordinationv1.Lease) { c.leaseQueue.Add(obj.Name) } -func setCommonEncodingVersion(sv *apiserverinternalv1alpha1.StorageVersion) { - if len(sv.Status.StorageVersions) == 0 { - return - } - firstVersion := sv.Status.StorageVersions[0].EncodingVersion - agreed := true - for _, ssv := range sv.Status.StorageVersions { - if ssv.EncodingVersion != firstVersion { - agreed = false - break - } - } - if agreed { - sv.Status.CommonEncodingVersion = &firstVersion - } else { - sv.Status.CommonEncodingVersion = nil - } -} - func (c *Controller) updateOrDeleteStorageVersion(sv *apiserverinternalv1alpha1.StorageVersion, serverStorageVersions []apiserverinternalv1alpha1.ServerStorageVersion) error { if len(serverStorageVersions) == 0 { return c.kubeclientset.InternalV1alpha1().StorageVersions().Delete( context.TODO(), sv.Name, metav1.DeleteOptions{}) } sv.Status.StorageVersions = serverStorageVersions - setCommonEncodingVersion(sv) + storageversion.SetCommonEncodingVersion(sv) _, err := c.kubeclientset.InternalV1alpha1().StorageVersions().UpdateStatus( context.TODO(), sv, metav1.UpdateOptions{}) return err diff --git a/staging/src/k8s.io/apiserver/pkg/storageversion/updater.go b/staging/src/k8s.io/apiserver/pkg/storageversion/updater.go index 10927fb0f03..ddd8dfbe632 100644 --- a/staging/src/k8s.io/apiserver/pkg/storageversion/updater.go +++ b/staging/src/k8s.io/apiserver/pkg/storageversion/updater.go @@ -35,23 +35,90 @@ type Client interface { Get(context.Context, string, metav1.GetOptions) (*v1alpha1.StorageVersion, error) } -func setCommonEncodingVersion(sv *v1alpha1.StorageVersion) { - if len(sv.Status.StorageVersions) == 0 { - return +// SetCommonEncodingVersion updates the CommonEncodingVersion and the AllEncodingVersionsEqual +// condition based on the StorageVersions. +func SetCommonEncodingVersion(sv *v1alpha1.StorageVersion) { + var oldCommonEncodingVersion *string + if sv.Status.CommonEncodingVersion != nil { + version := *sv.Status.CommonEncodingVersion + oldCommonEncodingVersion = &version } - firstVersion := sv.Status.StorageVersions[0].EncodingVersion - agreed := true - for _, ssv := range sv.Status.StorageVersions { - if ssv.EncodingVersion != firstVersion { - agreed = false - break + sv.Status.CommonEncodingVersion = nil + if len(sv.Status.StorageVersions) != 0 { + firstVersion := sv.Status.StorageVersions[0].EncodingVersion + agreed := true + for _, ssv := range sv.Status.StorageVersions { + if ssv.EncodingVersion != firstVersion { + agreed = false + break + } + } + if agreed { + sv.Status.CommonEncodingVersion = &firstVersion } } - if agreed { - sv.Status.CommonEncodingVersion = &firstVersion - } else { - sv.Status.CommonEncodingVersion = nil + + condition := v1alpha1.StorageVersionCondition{ + Type: v1alpha1.AllEncodingVersionsEqual, + Status: v1alpha1.ConditionFalse, + ObservedGeneration: sv.Generation, + LastTransitionTime: metav1.NewTime(time.Now()), + Reason: "CommonEncodingVersionUnset", + Message: "Common encoding version unset", } + if sv.Status.CommonEncodingVersion != nil { + condition.Status = v1alpha1.ConditionTrue + condition.Reason = "CommonEncodingVersionSet" + condition.Message = "Common encoding version set" + } + forceTransition := false + if oldCommonEncodingVersion != nil && sv.Status.CommonEncodingVersion != nil && + *oldCommonEncodingVersion != *sv.Status.CommonEncodingVersion { + forceTransition = true + } + setStatusCondition(&sv.Status.Conditions, condition, forceTransition) +} + +func findStatusCondition(conditions []v1alpha1.StorageVersionCondition, + conditionType v1alpha1.StorageVersionConditionType) *v1alpha1.StorageVersionCondition { + for i := range conditions { + if conditions[i].Type == conditionType { + return &conditions[i] + } + } + return nil +} + +// setStatusCondition sets the corresponding condition in conditions to newCondition. +// conditions must be non-nil. +// 1. if the condition of the specified type already exists: all fields of the existing condition are updated to +// newCondition, LastTransitionTime is set to now if the new status differs from the old status +// 2. if a condition of the specified type does not exist: LastTransitionTime is set to now() if unset, +// and newCondition is appended +// NOTE: forceTransition allows overwriting LastTransitionTime even when the status doesn't change. +func setStatusCondition(conditions *[]v1alpha1.StorageVersionCondition, newCondition v1alpha1.StorageVersionCondition, + forceTransition bool) { + if conditions == nil { + return + } + + if newCondition.LastTransitionTime.IsZero() { + newCondition.LastTransitionTime = metav1.NewTime(time.Now()) + } + existingCondition := findStatusCondition(*conditions, newCondition.Type) + if existingCondition == nil { + *conditions = append(*conditions, newCondition) + return + } + + statusChanged := existingCondition.Status != newCondition.Status + if statusChanged || forceTransition { + existingCondition.LastTransitionTime = newCondition.LastTransitionTime + } + existingCondition.Status = newCondition.Status + existingCondition.Reason = newCondition.Reason + existingCondition.Message = newCondition.Message + existingCondition.ObservedGeneration = newCondition.ObservedGeneration } // updateStorageVersionFor updates the storage version object for the resource. @@ -123,6 +190,6 @@ func localUpdateStorageVersion(sv *v1alpha1.StorageVersion, apiserverID, encodin if !foundSSV { sv.Status.StorageVersions = append(sv.Status.StorageVersions, newSSV) } - setCommonEncodingVersion(sv) + SetCommonEncodingVersion(sv) return sv } diff --git a/staging/src/k8s.io/apiserver/pkg/storageversion/updater_test.go b/staging/src/k8s.io/apiserver/pkg/storageversion/updater_test.go index be899a280f1..4356a19d956 100644 --- a/staging/src/k8s.io/apiserver/pkg/storageversion/updater_test.go +++ b/staging/src/k8s.io/apiserver/pkg/storageversion/updater_test.go @@ -22,35 +22,46 @@ import ( "github.com/google/go-cmp/cmp" "k8s.io/api/apiserverinternal/v1alpha1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) -func TestLocalUpdateStorageVersion(t *testing.T) { - v1 := "v1" - ssv1 := v1alpha1.ServerStorageVersion{ +var ( + v1 = "v1" + v2 = "v2" + ssv1 = v1alpha1.ServerStorageVersion{ APIServerID: "1", EncodingVersion: "v1", DecodableVersions: []string{"v1", "v2"}, } - ssv2 := v1alpha1.ServerStorageVersion{ + ssv2 = v1alpha1.ServerStorageVersion{ APIServerID: "2", EncodingVersion: "v1", DecodableVersions: []string{"v1", "v2"}, } // ssv3 has a different encoding version - ssv3 := v1alpha1.ServerStorageVersion{ + ssv3 = v1alpha1.ServerStorageVersion{ APIServerID: "3", EncodingVersion: "v2", DecodableVersions: []string{"v1", "v2"}, } - ssv4 := v1alpha1.ServerStorageVersion{ + ssv4 = v1alpha1.ServerStorageVersion{ APIServerID: "4", EncodingVersion: "v1", DecodableVersions: []string{"v1", "v2", "v4"}, } + ssv5 = v1alpha1.ServerStorageVersion{ + APIServerID: "5", + EncodingVersion: "v2", + DecodableVersions: []string{"v1", "v2", "v4"}, + } +) + +func TestLocalUpdateStorageVersion(t *testing.T) { tests := []struct { - old v1alpha1.StorageVersionStatus - newSSV v1alpha1.ServerStorageVersion - expected v1alpha1.StorageVersionStatus + old v1alpha1.StorageVersionStatus + newSSV v1alpha1.ServerStorageVersion + expected v1alpha1.StorageVersionStatus + expectLastTransitionTimeUpdate bool }{ { old: v1alpha1.StorageVersionStatus{}, @@ -58,36 +69,45 @@ func TestLocalUpdateStorageVersion(t *testing.T) { expected: v1alpha1.StorageVersionStatus{ StorageVersions: []v1alpha1.ServerStorageVersion{ssv1}, CommonEncodingVersion: &v1, + Conditions: commonVersionTrueCondition(), }, + expectLastTransitionTimeUpdate: true, }, { old: v1alpha1.StorageVersionStatus{ StorageVersions: []v1alpha1.ServerStorageVersion{ssv1, ssv2}, CommonEncodingVersion: &v1, + Conditions: commonVersionTrueCondition(), }, newSSV: ssv3, expected: v1alpha1.StorageVersionStatus{ StorageVersions: []v1alpha1.ServerStorageVersion{ssv1, ssv2, ssv3}, + Conditions: commonVersionFalseCondition(), }, + expectLastTransitionTimeUpdate: true, }, { old: v1alpha1.StorageVersionStatus{ StorageVersions: []v1alpha1.ServerStorageVersion{ssv1, ssv2}, CommonEncodingVersion: &v1, + Conditions: commonVersionTrueCondition(), }, newSSV: ssv4, expected: v1alpha1.StorageVersionStatus{ StorageVersions: []v1alpha1.ServerStorageVersion{ssv1, ssv2, ssv4}, CommonEncodingVersion: &v1, + Conditions: commonVersionTrueCondition(), }, }, { old: v1alpha1.StorageVersionStatus{ StorageVersions: []v1alpha1.ServerStorageVersion{ssv1, ssv2, ssv3}, + Conditions: commonVersionFalseCondition(), }, newSSV: ssv4, expected: v1alpha1.StorageVersionStatus{ StorageVersions: []v1alpha1.ServerStorageVersion{ssv1, ssv2, ssv3, ssv4}, + Conditions: commonVersionFalseCondition(), }, }, } @@ -95,8 +115,143 @@ func TestLocalUpdateStorageVersion(t *testing.T) { for _, tc := range tests { sv := &v1alpha1.StorageVersion{Status: tc.old} updated := localUpdateStorageVersion(sv, tc.newSSV.APIServerID, tc.newSSV.EncodingVersion, tc.newSSV.DecodableVersions) + if tc.expectLastTransitionTimeUpdate == updated.Status.Conditions[0].LastTransitionTime.IsZero() { + t.Errorf("unexpected LastTransitionTime, expected update: %v, got: %v", + tc.expectLastTransitionTimeUpdate, updated.Status.Conditions[0].LastTransitionTime) + } + updated.Status.Conditions[0].LastTransitionTime = metav1.Time{} if e, a := tc.expected, updated.Status; !reflect.DeepEqual(e, a) { t.Errorf("unexpected: %v", cmp.Diff(e, a)) } } } + +func commonVersionTrueCondition() []v1alpha1.StorageVersionCondition { + return []v1alpha1.StorageVersionCondition{{ + Type: v1alpha1.AllEncodingVersionsEqual, + Status: v1alpha1.ConditionTrue, + Reason: "CommonEncodingVersionSet", + Message: "Common encoding version set", + }} +} +func commonVersionFalseCondition() []v1alpha1.StorageVersionCondition { + return []v1alpha1.StorageVersionCondition{{ + Type: v1alpha1.AllEncodingVersionsEqual, + Status: v1alpha1.ConditionFalse, + Reason: "CommonEncodingVersionUnset", + Message: "Common encoding version unset", + }} +} + +func TestSetCommonEncodingVersion(t *testing.T) { + tests := []struct { + name string + old v1alpha1.StorageVersionStatus + expected v1alpha1.StorageVersionStatus + expectLastTransitionTimeUpdate bool + }{ + { + name: "no-common_init", + old: v1alpha1.StorageVersionStatus{ + StorageVersions: []v1alpha1.ServerStorageVersion{ssv1, ssv2, ssv3}, + }, + expected: v1alpha1.StorageVersionStatus{ + StorageVersions: []v1alpha1.ServerStorageVersion{ssv1, ssv2, ssv3}, + Conditions: commonVersionFalseCondition(), + }, + expectLastTransitionTimeUpdate: true, + }, + { + name: "no-common_transition", + old: v1alpha1.StorageVersionStatus{ + StorageVersions: []v1alpha1.ServerStorageVersion{ssv1, ssv2, ssv3}, + CommonEncodingVersion: &v1, + Conditions: commonVersionTrueCondition(), + }, + expected: v1alpha1.StorageVersionStatus{ + StorageVersions: []v1alpha1.ServerStorageVersion{ssv1, ssv2, ssv3}, + Conditions: commonVersionFalseCondition(), + }, + expectLastTransitionTimeUpdate: true, + }, + { + name: "no-common_no-transition", + old: v1alpha1.StorageVersionStatus{ + StorageVersions: []v1alpha1.ServerStorageVersion{ssv1, ssv2, ssv3}, + Conditions: commonVersionFalseCondition(), + }, + expected: v1alpha1.StorageVersionStatus{ + StorageVersions: []v1alpha1.ServerStorageVersion{ssv1, ssv2, ssv3}, + Conditions: commonVersionFalseCondition(), + }, + }, + { + name: "common_init", + old: v1alpha1.StorageVersionStatus{ + StorageVersions: []v1alpha1.ServerStorageVersion{ssv1, ssv2}, + }, + expected: v1alpha1.StorageVersionStatus{ + StorageVersions: []v1alpha1.ServerStorageVersion{ssv1, ssv2}, + CommonEncodingVersion: &v1, + Conditions: commonVersionTrueCondition(), + }, + expectLastTransitionTimeUpdate: true, + }, + { + name: "common_no-transition", + old: v1alpha1.StorageVersionStatus{ + StorageVersions: []v1alpha1.ServerStorageVersion{ssv1, ssv2}, + CommonEncodingVersion: &v1, + Conditions: commonVersionTrueCondition(), + }, + expected: v1alpha1.StorageVersionStatus{ + StorageVersions: []v1alpha1.ServerStorageVersion{ssv1, ssv2}, + CommonEncodingVersion: &v1, + Conditions: commonVersionTrueCondition(), + }, + }, + { + name: "common_transition", + old: v1alpha1.StorageVersionStatus{ + StorageVersions: []v1alpha1.ServerStorageVersion{ssv1, ssv2}, + Conditions: commonVersionFalseCondition(), + }, + expected: v1alpha1.StorageVersionStatus{ + StorageVersions: []v1alpha1.ServerStorageVersion{ssv1, ssv2}, + CommonEncodingVersion: &v1, + Conditions: commonVersionTrueCondition(), + }, + expectLastTransitionTimeUpdate: true, + }, + { + name: "common_version-changed", + old: v1alpha1.StorageVersionStatus{ + StorageVersions: []v1alpha1.ServerStorageVersion{ssv3, ssv5}, + CommonEncodingVersion: &v1, + Conditions: commonVersionTrueCondition(), + }, + expected: v1alpha1.StorageVersionStatus{ + StorageVersions: []v1alpha1.ServerStorageVersion{ssv3, ssv5}, + CommonEncodingVersion: &v2, + Conditions: commonVersionTrueCondition(), + }, + expectLastTransitionTimeUpdate: true, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + sv := &v1alpha1.StorageVersion{Status: tc.old} + SetCommonEncodingVersion(sv) + if (tc.expectLastTransitionTimeUpdate && sv.Status.Conditions[0].LastTransitionTime.IsZero()) || + (!tc.expectLastTransitionTimeUpdate && !sv.Status.Conditions[0].LastTransitionTime.IsZero()) { + t.Errorf("unexpected LastTransitionTime, expected update: %v, got: %v", + tc.expectLastTransitionTimeUpdate, sv.Status.Conditions[0].LastTransitionTime) + } + sv.Status.Conditions[0].LastTransitionTime = metav1.Time{} + if e, a := tc.expected, sv.Status; !reflect.DeepEqual(e, a) { + t.Errorf("unexpected: %v", cmp.Diff(e, a)) + } + }) + } +} From 7a28ca419e9c3c246937d1a3c1cdc7581ab06060 Mon Sep 17 00:00:00 2001 From: Haowei Cai Date: Wed, 2 Dec 2020 14:32:50 -0800 Subject: [PATCH 2/2] generated --- pkg/controller/storageversiongc/BUILD | 1 + staging/src/k8s.io/apiserver/pkg/storageversion/BUILD | 1 + 2 files changed, 2 insertions(+) diff --git a/pkg/controller/storageversiongc/BUILD b/pkg/controller/storageversiongc/BUILD index 85d9f5f8332..1c458720cb7 100644 --- a/pkg/controller/storageversiongc/BUILD +++ b/pkg/controller/storageversiongc/BUILD @@ -14,6 +14,7 @@ go_library( "//staging/src/k8s.io/apimachinery/pkg/util/errors:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/runtime:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/wait:go_default_library", + "//staging/src/k8s.io/apiserver/pkg/storageversion:go_default_library", "//staging/src/k8s.io/client-go/informers/apiserverinternal/v1alpha1:go_default_library", "//staging/src/k8s.io/client-go/informers/coordination/v1:go_default_library", "//staging/src/k8s.io/client-go/kubernetes:go_default_library", diff --git a/staging/src/k8s.io/apiserver/pkg/storageversion/BUILD b/staging/src/k8s.io/apiserver/pkg/storageversion/BUILD index 247a5156958..3d681b50b6f 100644 --- a/staging/src/k8s.io/apiserver/pkg/storageversion/BUILD +++ b/staging/src/k8s.io/apiserver/pkg/storageversion/BUILD @@ -32,6 +32,7 @@ go_test( embed = [":go_default_library"], deps = [ "//staging/src/k8s.io/api/apiserverinternal/v1alpha1:go_default_library", + "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/runtime/schema:go_default_library", "//vendor/github.com/google/go-cmp/cmp:go_default_library", ],