From 9b2908ea3b98edbb1a8bcaf4e3e429283b2debfc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Wojciech=20Tyczy=C5=84ski?= Date: Wed, 19 Jan 2022 21:13:25 +0100 Subject: [PATCH] Cleanup apiserver storage selflink references where possible --- .../pkg/storage/cacher/caching_object.go | 3 +- .../pkg/storage/cacher/caching_object_test.go | 29 ++++++++++--------- .../pkg/storage/etcd3/api_object_versioner.go | 2 +- .../apiserver/pkg/storage/etcd3/store_test.go | 10 +++---- 4 files changed, 23 insertions(+), 21 deletions(-) diff --git a/staging/src/k8s.io/apiserver/pkg/storage/cacher/caching_object.go b/staging/src/k8s.io/apiserver/pkg/storage/cacher/caching_object.go index 752a28714c3..1ce675a1ed5 100644 --- a/staging/src/k8s.io/apiserver/pkg/storage/cacher/caching_object.go +++ b/staging/src/k8s.io/apiserver/pkg/storage/cacher/caching_object.go @@ -60,8 +60,7 @@ type serializationsCache map[runtime.Identifier]*serializationResult // so that each of those is computed exactly once. // // cachingObject implements the metav1.Object interface (accessors for -// all metadata fields). However, setters for all fields except from -// SelfLink (which is set lately in the path) are ignored. +// all metadata fields). type cachingObject struct { lock sync.RWMutex diff --git a/staging/src/k8s.io/apiserver/pkg/storage/cacher/caching_object_test.go b/staging/src/k8s.io/apiserver/pkg/storage/cacher/caching_object_test.go index 14adcda65c2..f992573bab3 100644 --- a/staging/src/k8s.io/apiserver/pkg/storage/cacher/caching_object_test.go +++ b/staging/src/k8s.io/apiserver/pkg/storage/cacher/caching_object_test.go @@ -83,36 +83,39 @@ func TestCachingObject(t *testing.T) { } } -func TestSelfLink(t *testing.T) { +func TestCachingObjectFieldAccessors(t *testing.T) { object, err := newCachingObject(&v1.Pod{}) if err != nil { t.Fatalf("couldn't create cachingObject: %v", err) } - selfLink := "selfLink" - object.SetSelfLink(selfLink) - encodeSelfLink := func(obj runtime.Object, w io.Writer) error { + // Given accessors for all fields implement the same logic, + // we are choosing an arbitrary one to test. + clusterName := "clusterName" + object.SetClusterName(clusterName) + + encodeClusterName := func(obj runtime.Object, w io.Writer) error { accessor, err := meta.Accessor(obj) if err != nil { t.Fatalf("failed to get accessor for %#v: %v", obj, err) } - _, err = w.Write([]byte(accessor.GetSelfLink())) + _, err = w.Write([]byte(accessor.GetClusterName())) return err } buffer := bytes.NewBuffer(nil) - if err := object.CacheEncode("", encodeSelfLink, buffer); err != nil { + if err := object.CacheEncode("", encodeClusterName, buffer); err != nil { t.Errorf("unexpected error: %v", err) } - if a, e := buffer.String(), selfLink; a != e { + if a, e := buffer.String(), clusterName; a != e { t.Errorf("unexpected serialization: %s, expected: %s", a, e) } - // GetObject should also set selfLink. + // GetObject should also set clusterName. buffer.Reset() - if err := encodeSelfLink(object.GetObject(), buffer); err != nil { + if err := encodeClusterName(object.GetObject(), buffer); err != nil { t.Errorf("unexpected error: %v", err) } - if a, e := buffer.String(), selfLink; a != e { + if a, e := buffer.String(), clusterName; a != e { t.Errorf("unexpected serialization: %s, expected: %s", a, e) } } @@ -136,7 +139,7 @@ func TestCachingObjectRaces(t *testing.T) { for i := 0; i < numWorkers; i++ { go func() { defer wg.Done() - object.SetSelfLink("selfLink") + object.SetClusterName("clusterName") buffer := bytes.NewBuffer(nil) for _, encoder := range encoders { buffer.Reset() @@ -152,8 +155,8 @@ func TestCachingObjectRaces(t *testing.T) { t.Errorf("failed to get accessor: %v", err) return } - if selfLink := accessor.GetSelfLink(); selfLink != "selfLink" { - t.Errorf("unexpected selfLink: %s", selfLink) + if clusterName := accessor.GetClusterName(); clusterName != "clusterName" { + t.Errorf("unexpected clusterName: %s", clusterName) } }() } diff --git a/staging/src/k8s.io/apiserver/pkg/storage/etcd3/api_object_versioner.go b/staging/src/k8s.io/apiserver/pkg/storage/etcd3/api_object_versioner.go index c42fc6e08ea..a5e88fd01c5 100644 --- a/staging/src/k8s.io/apiserver/pkg/storage/etcd3/api_object_versioner.go +++ b/staging/src/k8s.io/apiserver/pkg/storage/etcd3/api_object_versioner.go @@ -60,7 +60,7 @@ func (a APIObjectVersioner) UpdateList(obj runtime.Object, resourceVersion uint6 return nil } -// PrepareObjectForStorage clears resource version and self link prior to writing to etcd. +// PrepareObjectForStorage clears resourceVersion and selfLink prior to writing to etcd. func (a APIObjectVersioner) PrepareObjectForStorage(obj runtime.Object) error { accessor, err := meta.Accessor(obj) if err != nil { diff --git a/staging/src/k8s.io/apiserver/pkg/storage/etcd3/store_test.go b/staging/src/k8s.io/apiserver/pkg/storage/etcd3/store_test.go index 2a5de51255f..e22228534dc 100644 --- a/staging/src/k8s.io/apiserver/pkg/storage/etcd3/store_test.go +++ b/staging/src/k8s.io/apiserver/pkg/storage/etcd3/store_test.go @@ -135,7 +135,7 @@ func TestCreate(t *testing.T) { t.Errorf("output should have non-empty resource version") } if out.SelfLink != "" { - t.Errorf("output should have empty self link") + t.Errorf("output should have empty selfLink") } checkStorageInvariants(ctx, t, etcdClient, store, key) @@ -158,7 +158,7 @@ func checkStorageInvariants(ctx context.Context, t *testing.T, etcdClient *clien t.Errorf("stored object should have empty resource version") } if obj.SelfLink != "" { - t.Errorf("stored output should have empty self link") + t.Errorf("stored output should have empty selfLink") } } @@ -686,7 +686,7 @@ func TestGuaranteedUpdate(t *testing.T) { expectNotFoundErr: false, expectInvalidObjErr: false, expectNoUpdate: true, - }, { // GuaranteedUpdate with same data AND a self link + }, { // GuaranteedUpdate with same data AND a selfLink key: key, ignoreNotFound: false, precondition: nil, @@ -768,7 +768,7 @@ func TestGuaranteedUpdate(t *testing.T) { t.Errorf("#%d: pod name want=%s, get=%s", i, name, out.ObjectMeta.Name) } if out.SelfLink != "" { - t.Errorf("#%d: selflink should not be set", i) + t.Errorf("#%d: selfLink should not be set", i) } // verify that kv pair is not empty after set and that the underlying data matches expectations @@ -2359,7 +2359,7 @@ func TestLeaseMaxObjectCount(t *testing.T) { }) ctx := context.Background() - obj := &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: "foo", SelfLink: "testlink"}} + obj := &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: "foo"}} out := &example.Pod{} testCases := []struct {