Skip to content

Commit ecc8d0a

Browse files
authored
Merge pull request kubernetes#132337 from hakuna-matatah/automated-cherry-pick-of-#132244-upstream-release-1.33
Automated cherry pick of kubernetes#132244: 1.33 regression - Consistent paginated lists serve from cache
2 parents 54a0cea + d4a4a1d commit ecc8d0a

File tree

2 files changed

+134
-5
lines changed

2 files changed

+134
-5
lines changed

staging/src/k8s.io/apiserver/pkg/storage/cacher/cacher_whitebox_test.go

Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -662,6 +662,7 @@ func TestMatchExactResourceVersionFallback(t *testing.T) {
662662
}
663663
for _, tc := range tcs {
664664
t.Run(tc.name, func(t *testing.T) {
665+
featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.ListFromCacheSnapshot, true)
665666
backingStorage := &dummyStorage{}
666667
expectStoreRequests := 0
667668
backingStorage.getListFn = func(_ context.Context, key string, opts storage.ListOptions, listObj runtime.Object) error {
@@ -759,6 +760,125 @@ func TestGetListNonRecursiveCacheBypass(t *testing.T) {
759760
}
760761
}
761762

763+
func TestGetListNonRecursiveCacheWithConsistentListFromCache(t *testing.T) {
764+
// Set feature gates once at the beginning since we only care about ConsistentListFromCache=true and ListFromCacheSnapshot=false
765+
featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.ConsistentListFromCache, true)
766+
featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.ListFromCacheSnapshot, false)
767+
forceRequestWatchProgressSupport(t)
768+
769+
tests := []struct {
770+
name string
771+
consistentListFromCache bool
772+
expectGetListCallCount int
773+
expectGetCurrentRV bool
774+
injectRVError bool
775+
expectedError error
776+
}{
777+
{
778+
name: "ConsistentListFromCache enabled - served from cache",
779+
consistentListFromCache: true,
780+
expectGetListCallCount: 1,
781+
expectGetCurrentRV: true,
782+
injectRVError: false,
783+
expectedError: nil,
784+
},
785+
}
786+
787+
for _, tc := range tests {
788+
t.Run(tc.name, func(t *testing.T) {
789+
var getListCount, getCurrentRVCount int
790+
backingStorage := &dummyStorage{}
791+
792+
backingStorage.getListFn = func(ctx context.Context, key string, opts storage.ListOptions, listObj runtime.Object) error {
793+
getListCount++
794+
if tc.injectRVError {
795+
return errDummy
796+
}
797+
podList := listObj.(*example.PodList)
798+
podList.ListMeta = metav1.ListMeta{ResourceVersion: "100"}
799+
return nil
800+
}
801+
802+
backingStorage.getRVFn = func(ctx context.Context) (uint64, error) {
803+
getCurrentRVCount++
804+
rv := uint64(100)
805+
err := error(nil)
806+
if tc.injectRVError {
807+
err = errDummy
808+
return 0, err
809+
}
810+
return rv, nil
811+
}
812+
813+
cacher, v, err := newTestCacher(backingStorage)
814+
if err != nil {
815+
t.Fatalf("Couldn't create cacher: %v", err)
816+
}
817+
defer cacher.Stop()
818+
819+
// Wait for cacher to be ready before injecting errors
820+
if err := cacher.ready.wait(context.Background()); err != nil {
821+
t.Fatalf("unexpected error waiting for the cache to be ready: %v", err)
822+
}
823+
delegator := NewCacheDelegator(cacher, backingStorage)
824+
defer delegator.Stop()
825+
826+
// Setup test object
827+
key := "pods/ns"
828+
input := &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "ns"}}
829+
if err := v.UpdateObject(input, 100); err != nil {
830+
t.Fatalf("Unexpected error: %v", err)
831+
}
832+
833+
// Put object into the store
834+
if err := cacher.watchCache.Add(input); err != nil {
835+
t.Fatalf("Unexpected error: %v", err)
836+
}
837+
838+
pred := storage.SelectionPredicate{
839+
Label: labels.Everything(),
840+
Field: fields.Everything(),
841+
Limit: 500,
842+
}
843+
result := &example.PodList{}
844+
845+
// Make the list call with empty RV - delegator will get current RV and use it
846+
err = delegator.GetList(context.TODO(), key, storage.ListOptions{
847+
ResourceVersion: "",
848+
Predicate: pred,
849+
Recursive: true,
850+
}, result)
851+
852+
// Verify error matches expectation
853+
if !errors.Is(err, tc.expectedError) {
854+
t.Errorf("Expected error %v, got: %v", tc.expectedError, err)
855+
}
856+
857+
// Verify the correct storage method was called
858+
if getListCount != tc.expectGetListCallCount {
859+
t.Errorf("Expected GetList to be called %d times, but it was called %d times", tc.expectGetListCallCount, getListCount)
860+
}
861+
if tc.expectGetCurrentRV && getCurrentRVCount == 0 {
862+
t.Error("Expected GetCurrentResourceVersion to be called, but it wasn't")
863+
}
864+
if !tc.expectGetCurrentRV && getCurrentRVCount > 0 {
865+
t.Errorf("Expected GetCurrentResourceVersion not to be called, but it was called %d times", getCurrentRVCount)
866+
}
867+
868+
// For successful cache reads, verify the resource version
869+
if err == nil {
870+
resultRV, err := cacher.versioner.ParseResourceVersion(result.ResourceVersion)
871+
if err != nil {
872+
t.Fatalf("Failed to parse result resource version: %v", err)
873+
}
874+
expectedRV := uint64(100)
875+
if resultRV != expectedRV {
876+
t.Errorf("Expected RV %d but got %d", expectedRV, resultRV)
877+
}
878+
}
879+
})
880+
}
881+
}
762882
func TestGetCacheBypass(t *testing.T) {
763883
backingStorage := &dummyStorage{}
764884
cacher, _, err := newTestCacher(backingStorage)

staging/src/k8s.io/apiserver/pkg/storage/cacher/delegator.go

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -206,27 +206,36 @@ func (c *CacheDelegator) GetList(ctx context.Context, key string, opts storage.L
206206
return c.storage.GetList(ctx, key, opts, listObj)
207207
}
208208
}
209+
fallbackOpts := opts
209210
if result.ConsistentRead {
210211
listRV, err = c.storage.GetCurrentResourceVersion(ctx)
211212
if err != nil {
212213
return err
213214
}
214215
// Setting resource version for consistent read in cache based on current ResourceVersion in etcd.
215216
opts.ResourceVersion = strconv.FormatInt(int64(listRV), 10)
217+
// If continue is not set, we need to set the resource version match to ResourceVersionMatchNotOlderThan to serve latest from cache
218+
if opts.Predicate.Continue == "" {
219+
opts.ResourceVersionMatch = metav1.ResourceVersionMatchNotOlderThan
220+
}
216221
}
217222
err = c.cacher.GetList(ctx, key, opts, listObj)
218223
success := "true"
219224
fallback := "false"
220225
if err != nil {
221-
if errors.IsResourceExpired(err) {
222-
return c.storage.GetList(ctx, key, opts, listObj)
226+
// ResourceExpired error occurs when attempting to list from cache with a specific resourceVersion
227+
// that is no longer available in the cache. With ListFromCacheSnapshot feature (1.34+), we can
228+
// serve exact resourceVersion requests from cache if available, falling back to storage only when
229+
// the requested version is expired.
230+
if errors.IsResourceExpired(err) && utilfeature.DefaultFeatureGate.Enabled(features.ListFromCacheSnapshot) {
231+
return c.storage.GetList(ctx, key, fallbackOpts, listObj)
223232
}
224233
if result.ConsistentRead {
234+
// IsTooLargeResourceVersion occurs when the requested RV is higher than cache's current RV
235+
// and cache hasn't caught up within the timeout period. Fall back to etcd.
225236
if storage.IsTooLargeResourceVersion(err) {
226237
fallback = "true"
227-
// Reset resourceVersion during fallback from consistent read.
228-
opts.ResourceVersion = ""
229-
err = c.storage.GetList(ctx, key, opts, listObj)
238+
err = c.storage.GetList(ctx, key, fallbackOpts, listObj)
230239
}
231240
if err != nil {
232241
success = "false"

0 commit comments

Comments
 (0)