From 272dfc9d7e61481dfcdc4d6a021385d9cd85ba5f Mon Sep 17 00:00:00 2001 From: Lukasz Szaszkiewicz Date: Mon, 27 May 2024 11:10:43 +0200 Subject: [PATCH 1/4] move client-go/tools/cache/reflector_data_consistency_detector to client-go/util/consistencydetector --- .../consistencydetector/data_consistency_detector.go} | 0 .../consistencydetector/data_consistency_detector_test.go} | 0 2 files changed, 0 insertions(+), 0 deletions(-) rename staging/src/k8s.io/client-go/{tools/cache/reflector_data_consistency_detector.go => util/consistencydetector/data_consistency_detector.go} (100%) rename staging/src/k8s.io/client-go/{tools/cache/reflector_data_consistency_detector_test.go => util/consistencydetector/data_consistency_detector_test.go} (100%) diff --git a/staging/src/k8s.io/client-go/tools/cache/reflector_data_consistency_detector.go b/staging/src/k8s.io/client-go/util/consistencydetector/data_consistency_detector.go similarity index 100% rename from staging/src/k8s.io/client-go/tools/cache/reflector_data_consistency_detector.go rename to staging/src/k8s.io/client-go/util/consistencydetector/data_consistency_detector.go diff --git a/staging/src/k8s.io/client-go/tools/cache/reflector_data_consistency_detector_test.go b/staging/src/k8s.io/client-go/util/consistencydetector/data_consistency_detector_test.go similarity index 100% rename from staging/src/k8s.io/client-go/tools/cache/reflector_data_consistency_detector_test.go rename to staging/src/k8s.io/client-go/util/consistencydetector/data_consistency_detector_test.go From faf5110c8a2394f9d098da6e5097ce6deed1b18b Mon Sep 17 00:00:00 2001 From: Lukasz Szaszkiewicz Date: Mon, 27 May 2024 11:13:18 +0200 Subject: [PATCH 2/4] client-go/util/consistencydetector: update after moving to the new package --- .../util/consistencydetector/data_consistency_detector.go | 2 +- .../consistencydetector/data_consistency_detector_test.go | 7 ++++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/staging/src/k8s.io/client-go/util/consistencydetector/data_consistency_detector.go b/staging/src/k8s.io/client-go/util/consistencydetector/data_consistency_detector.go index 0aacee4a067..ce5348381f9 100644 --- a/staging/src/k8s.io/client-go/util/consistencydetector/data_consistency_detector.go +++ b/staging/src/k8s.io/client-go/util/consistencydetector/data_consistency_detector.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package cache +package consistencydetector import ( "context" diff --git a/staging/src/k8s.io/client-go/util/consistencydetector/data_consistency_detector_test.go b/staging/src/k8s.io/client-go/util/consistencydetector/data_consistency_detector_test.go index 81affb75d76..8ce0e312766 100644 --- a/staging/src/k8s.io/client-go/util/consistencydetector/data_consistency_detector_test.go +++ b/staging/src/k8s.io/client-go/util/consistencydetector/data_consistency_detector_test.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package cache +package consistencydetector import ( "context" @@ -26,6 +26,7 @@ import ( v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" "k8s.io/utils/ptr" ) @@ -158,3 +159,7 @@ func (lw *listWrapper) List(_ context.Context, opts metav1.ListOptions) (*v1.Pod lw.requestOptions = append(lw.requestOptions, opts) return lw.response, nil } + +func makePod(name, rv string) *v1.Pod { + return &v1.Pod{ObjectMeta: metav1.ObjectMeta{Name: name, ResourceVersion: rv, UID: types.UID(name)}} +} From e421046f64c90b58577a79f92dd463ab03479d79 Mon Sep 17 00:00:00 2001 From: Lukasz Szaszkiewicz Date: Mon, 27 May 2024 11:16:17 +0200 Subject: [PATCH 3/4] client-go/util/consistencydetector: make the detector public --- .../consistencydetector/data_consistency_detector.go | 12 ++++++------ .../data_consistency_detector_test.go | 6 +++--- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/staging/src/k8s.io/client-go/util/consistencydetector/data_consistency_detector.go b/staging/src/k8s.io/client-go/util/consistencydetector/data_consistency_detector.go index ce5348381f9..084f13226f6 100644 --- a/staging/src/k8s.io/client-go/util/consistencydetector/data_consistency_detector.go +++ b/staging/src/k8s.io/client-go/util/consistencydetector/data_consistency_detector.go @@ -39,9 +39,9 @@ func init() { dataConsistencyDetectionForWatchListEnabled, _ = strconv.ParseBool(os.Getenv("KUBE_WATCHLIST_INCONSISTENCY_DETECTOR")) } -type retrieveItemsFunc[U any] func() []U +type RetrieveItemsFunc[U any] func() []U -type listFunc[T runtime.Object] func(ctx context.Context, options metav1.ListOptions) (T, error) +type ListFunc[T runtime.Object] func(ctx context.Context, options metav1.ListOptions) (T, error) // checkWatchListDataConsistencyIfRequested performs a data consistency check only when // the KUBE_WATCHLIST_INCONSISTENCY_DETECTOR environment variable was set during a binary startup. @@ -52,21 +52,21 @@ type listFunc[T runtime.Object] func(ctx context.Context, options metav1.ListOpt // // Note that this function will panic when data inconsistency is detected. // This is intentional because we want to catch it in the CI. -func checkWatchListDataConsistencyIfRequested[T runtime.Object, U any](ctx context.Context, identity string, lastSyncedResourceVersion string, listFn listFunc[T], retrieveItemsFn retrieveItemsFunc[U]) { +func checkWatchListDataConsistencyIfRequested[T runtime.Object, U any](ctx context.Context, identity string, lastSyncedResourceVersion string, listFn ListFunc[T], retrieveItemsFn RetrieveItemsFunc[U]) { if !dataConsistencyDetectionForWatchListEnabled { return } // for informers we pass an empty ListOptions because // listFn might be wrapped for filtering during informer construction. - checkDataConsistency(ctx, identity, lastSyncedResourceVersion, listFn, metav1.ListOptions{}, retrieveItemsFn) + CheckDataConsistency(ctx, identity, lastSyncedResourceVersion, listFn, metav1.ListOptions{}, retrieveItemsFn) } -// checkDataConsistency exists solely for testing purposes. +// CheckDataConsistency exists solely for testing purposes. // we cannot use checkWatchListDataConsistencyIfRequested because // it is guarded by an environmental variable. // we cannot manipulate the environmental variable because // it will affect other tests in this package. -func checkDataConsistency[T runtime.Object, U any](ctx context.Context, identity string, lastSyncedResourceVersion string, listFn listFunc[T], listOptions metav1.ListOptions, retrieveItemsFn retrieveItemsFunc[U]) { +func CheckDataConsistency[T runtime.Object, U any](ctx context.Context, identity string, lastSyncedResourceVersion string, listFn ListFunc[T], listOptions metav1.ListOptions, retrieveItemsFn RetrieveItemsFunc[U]) { klog.Warningf("data consistency check for %s is enabled, this will result in an additional call to the API server.", identity) listOptions.ResourceVersion = lastSyncedResourceVersion listOptions.ResourceVersionMatch = metav1.ResourceVersionMatchExact diff --git a/staging/src/k8s.io/client-go/util/consistencydetector/data_consistency_detector_test.go b/staging/src/k8s.io/client-go/util/consistencydetector/data_consistency_detector_test.go index 8ce0e312766..9cbe036eaa1 100644 --- a/staging/src/k8s.io/client-go/util/consistencydetector/data_consistency_detector_test.go +++ b/staging/src/k8s.io/client-go/util/consistencydetector/data_consistency_detector_test.go @@ -106,10 +106,10 @@ func TestDataConsistencyChecker(t *testing.T) { if scenario.expectPanic { require.Panics(t, func() { - checkDataConsistency(ctx, "", scenario.listResponse.ResourceVersion, fakeLister.List, scenario.requestOptions, retrievedItemsFunc) + CheckDataConsistency(ctx, "", scenario.listResponse.ResourceVersion, fakeLister.List, scenario.requestOptions, retrievedItemsFunc) }) } else { - checkDataConsistency(ctx, "", scenario.listResponse.ResourceVersion, fakeLister.List, scenario.requestOptions, retrievedItemsFunc) + CheckDataConsistency(ctx, "", scenario.listResponse.ResourceVersion, fakeLister.List, scenario.requestOptions, retrievedItemsFunc) } require.Equal(t, fakeLister.counter, scenario.expectedListRequests) @@ -131,7 +131,7 @@ func TestDataConsistencyCheckerRetry(t *testing.T) { stopListErrorAfter := 5 fakeErrLister := &errorLister{stopErrorAfter: stopListErrorAfter} - checkDataConsistency(ctx, "", "", fakeErrLister.List, metav1.ListOptions{}, retrievedItemsFunc) + CheckDataConsistency(ctx, "", "", fakeErrLister.List, metav1.ListOptions{}, retrievedItemsFunc) require.Equal(t, fakeErrLister.listCounter, fakeErrLister.stopErrorAfter) } From cb44f83b3d500bb2ed29f7634095bc64c9b21729 Mon Sep 17 00:00:00 2001 From: Lukasz Szaszkiewicz Date: Mon, 27 May 2024 11:22:28 +0200 Subject: [PATCH 4/4] move checkWatchListDataConsistencyIfRequested back to client-go/tools/cache --- .../reflector_data_consistency_detector.go | 51 +++++++++++++++++++ ...eflector_data_consistency_detector_test.go | 29 +++++++++++ .../data_consistency_detector.go | 26 ---------- .../data_consistency_detector_test.go | 5 -- 4 files changed, 80 insertions(+), 31 deletions(-) create mode 100644 staging/src/k8s.io/client-go/tools/cache/reflector_data_consistency_detector.go create mode 100644 staging/src/k8s.io/client-go/tools/cache/reflector_data_consistency_detector_test.go diff --git a/staging/src/k8s.io/client-go/tools/cache/reflector_data_consistency_detector.go b/staging/src/k8s.io/client-go/tools/cache/reflector_data_consistency_detector.go new file mode 100644 index 00000000000..bd857de7a51 --- /dev/null +++ b/staging/src/k8s.io/client-go/tools/cache/reflector_data_consistency_detector.go @@ -0,0 +1,51 @@ +/* +Copyright 2024 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 cache + +import ( + "context" + "os" + "strconv" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/client-go/util/consistencydetector" +) + +var dataConsistencyDetectionForWatchListEnabled = false + +func init() { + dataConsistencyDetectionForWatchListEnabled, _ = strconv.ParseBool(os.Getenv("KUBE_WATCHLIST_INCONSISTENCY_DETECTOR")) +} + +// checkWatchListDataConsistencyIfRequested performs a data consistency check only when +// the KUBE_WATCHLIST_INCONSISTENCY_DETECTOR environment variable was set during a binary startup. +// +// The consistency check is meant to be enforced only in the CI, not in production. +// The check ensures that data retrieved by the watch-list api call +// is exactly the same as data received by the standard list api call against etcd. +// +// Note that this function will panic when data inconsistency is detected. +// This is intentional because we want to catch it in the CI. +func checkWatchListDataConsistencyIfRequested[T runtime.Object, U any](ctx context.Context, identity string, lastSyncedResourceVersion string, listFn consistencydetector.ListFunc[T], retrieveItemsFn consistencydetector.RetrieveItemsFunc[U]) { + if !dataConsistencyDetectionForWatchListEnabled { + return + } + // for informers we pass an empty ListOptions because + // listFn might be wrapped for filtering during informer construction. + consistencydetector.CheckDataConsistency(ctx, identity, lastSyncedResourceVersion, listFn, metav1.ListOptions{}, retrieveItemsFn) +} diff --git a/staging/src/k8s.io/client-go/tools/cache/reflector_data_consistency_detector_test.go b/staging/src/k8s.io/client-go/tools/cache/reflector_data_consistency_detector_test.go new file mode 100644 index 00000000000..7d0d8bb624d --- /dev/null +++ b/staging/src/k8s.io/client-go/tools/cache/reflector_data_consistency_detector_test.go @@ -0,0 +1,29 @@ +/* +Copyright 2024 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 cache + +import ( + "context" + "testing" + + "k8s.io/apimachinery/pkg/runtime" +) + +func TestDriveWatchLisConsistencyIfRequired(t *testing.T) { + ctx := context.TODO() + checkWatchListDataConsistencyIfRequested[runtime.Object, runtime.Object](ctx, "", "", nil, nil) +} diff --git a/staging/src/k8s.io/client-go/util/consistencydetector/data_consistency_detector.go b/staging/src/k8s.io/client-go/util/consistencydetector/data_consistency_detector.go index 084f13226f6..64288eb865f 100644 --- a/staging/src/k8s.io/client-go/util/consistencydetector/data_consistency_detector.go +++ b/staging/src/k8s.io/client-go/util/consistencydetector/data_consistency_detector.go @@ -19,9 +19,7 @@ package consistencydetector import ( "context" "fmt" - "os" "sort" - "strconv" "time" "github.com/google/go-cmp/cmp" @@ -33,34 +31,10 @@ import ( "k8s.io/klog/v2" ) -var dataConsistencyDetectionForWatchListEnabled = false - -func init() { - dataConsistencyDetectionForWatchListEnabled, _ = strconv.ParseBool(os.Getenv("KUBE_WATCHLIST_INCONSISTENCY_DETECTOR")) -} - type RetrieveItemsFunc[U any] func() []U type ListFunc[T runtime.Object] func(ctx context.Context, options metav1.ListOptions) (T, error) -// checkWatchListDataConsistencyIfRequested performs a data consistency check only when -// the KUBE_WATCHLIST_INCONSISTENCY_DETECTOR environment variable was set during a binary startup. -// -// The consistency check is meant to be enforced only in the CI, not in production. -// The check ensures that data retrieved by the watch-list api call -// is exactly the same as data received by the standard list api call against etcd. -// -// Note that this function will panic when data inconsistency is detected. -// This is intentional because we want to catch it in the CI. -func checkWatchListDataConsistencyIfRequested[T runtime.Object, U any](ctx context.Context, identity string, lastSyncedResourceVersion string, listFn ListFunc[T], retrieveItemsFn RetrieveItemsFunc[U]) { - if !dataConsistencyDetectionForWatchListEnabled { - return - } - // for informers we pass an empty ListOptions because - // listFn might be wrapped for filtering during informer construction. - CheckDataConsistency(ctx, identity, lastSyncedResourceVersion, listFn, metav1.ListOptions{}, retrieveItemsFn) -} - // CheckDataConsistency exists solely for testing purposes. // we cannot use checkWatchListDataConsistencyIfRequested because // it is guarded by an environmental variable. diff --git a/staging/src/k8s.io/client-go/util/consistencydetector/data_consistency_detector_test.go b/staging/src/k8s.io/client-go/util/consistencydetector/data_consistency_detector_test.go index 9cbe036eaa1..c6d21754e0d 100644 --- a/staging/src/k8s.io/client-go/util/consistencydetector/data_consistency_detector_test.go +++ b/staging/src/k8s.io/client-go/util/consistencydetector/data_consistency_detector_test.go @@ -118,11 +118,6 @@ func TestDataConsistencyChecker(t *testing.T) { } } -func TestDriveWatchLisConsistencyIfRequired(t *testing.T) { - ctx := context.TODO() - checkWatchListDataConsistencyIfRequested[runtime.Object, runtime.Object](ctx, "", "", nil, nil) -} - func TestDataConsistencyCheckerRetry(t *testing.T) { ctx := context.TODO() retrievedItemsFunc := func() []*v1.Pod {