Skip to content

Commit 62bb203

Browse files
alroscaCopilot
andauthored
APIGOV-32372 cache rebuild optimizations (#1023)
* APIGOV-32372 rebuild only failed kinds * APIGOV-32372 fix for listener/cache rebuild race condition * APIGOV-32372 merge main * APIGOV-32372 remove else statement Co-authored-by: Copilot <copilot@github.com> * APIGOV-32327 wait for clients to be ready * APIGOV-32372 improvements * APIGOV-32372 fix for unlock on unlocked mutex error * APIGOV-32372 guard against listener potential data race --------- Co-authored-by: Copilot <copilot@github.com>
1 parent 0547f88 commit 62bb203

19 files changed

Lines changed: 1250 additions & 273 deletions

pkg/agent/cache/cachevalidation.go

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,17 @@ func (c *cacheManager) getCacheForKind(kind string) cache.Cache {
6666
}
6767
}
6868

69+
// FlushKind empties the dedicated cache for the given resource kind.
70+
// Kinds without a dedicated cache are silently ignored.
71+
func (c *cacheManager) FlushKind(kind string) {
72+
c.ApplyResourceReadLock()
73+
defer c.ReleaseResourceReadLock()
74+
75+
if resourceCache := c.getCacheForKind(kind); resourceCache != nil {
76+
resourceCache.Flush()
77+
}
78+
}
79+
6980
// ResourceCacheKey builds a unique cache key from kind, scope name, and resource name.
7081
func ResourceCacheKey(kind, scopeName, name string) string {
7182
return kind + "/" + scopeName + "/" + name

pkg/agent/cache/cachevalidation_test.go

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -219,3 +219,77 @@ func TestGetCachedResourcesByKind(t *testing.T) {
219219
})
220220
}
221221
}
222+
223+
func TestFlushKind(t *testing.T) {
224+
modTime := time.Date(2026, 3, 12, 10, 0, 0, 0, time.UTC)
225+
226+
tests := map[string]struct {
227+
setup func(Manager)
228+
flushKind string
229+
// after flush, verify these kinds still have data
230+
expectNonEmpty []struct{ group, kind string }
231+
// after flush, verify these kinds are empty
232+
expectEmpty []struct{ group, kind string }
233+
}{
234+
"flush APIService leaves other caches intact": {
235+
setup: func(cm Manager) {
236+
cm.AddAPIService(makeAPIServiceRI("env1", "svc1", "ext-id-1", modTime))
237+
cm.AddAPIServiceInstance(makeRI("management", management.APIServiceInstanceGVK().Kind, "Environment", "env1", "inst1", "id1", modTime))
238+
},
239+
flushKind: management.APIServiceGVK().Kind,
240+
expectEmpty: []struct{ group, kind string }{
241+
{"management", management.APIServiceGVK().Kind},
242+
},
243+
expectNonEmpty: []struct{ group, kind string }{
244+
{"management", management.APIServiceInstanceGVK().Kind},
245+
},
246+
},
247+
"flush APIServiceInstance": {
248+
setup: func(cm Manager) {
249+
cm.AddAPIServiceInstance(makeRI("management", management.APIServiceInstanceGVK().Kind, "Environment", "env1", "inst1", "id1", modTime))
250+
},
251+
flushKind: management.APIServiceInstanceGVK().Kind,
252+
expectEmpty: []struct{ group, kind string }{
253+
{"management", management.APIServiceInstanceGVK().Kind},
254+
},
255+
},
256+
"flush ManagedApplication": {
257+
setup: func(cm Manager) {
258+
cm.AddManagedApplication(makeRI("management", management.ManagedApplicationGVK().Kind, "Environment", "env1", "app1", "id1", modTime))
259+
},
260+
flushKind: management.ManagedApplicationGVK().Kind,
261+
expectEmpty: []struct{ group, kind string }{
262+
{"management", management.ManagedApplicationGVK().Kind},
263+
},
264+
},
265+
"flush unknown kind is a no-op": {
266+
setup: func(cm Manager) {
267+
cm.AddAPIService(makeAPIServiceRI("env1", "svc1", "ext-id-1", modTime))
268+
},
269+
flushKind: "UnknownKind",
270+
expectNonEmpty: []struct{ group, kind string }{
271+
{"management", management.APIServiceGVK().Kind},
272+
},
273+
},
274+
}
275+
276+
for name, tc := range tests {
277+
t.Run(name, func(t *testing.T) {
278+
cm := NewAgentCacheManager(&config.CentralConfiguration{}, false)
279+
if tc.setup != nil {
280+
tc.setup(cm)
281+
}
282+
283+
cm.FlushKind(tc.flushKind)
284+
285+
for _, e := range tc.expectEmpty {
286+
result := cm.GetCachedResourcesByKind(e.group, e.kind, "")
287+
assert.Empty(t, result, "expected %s cache to be empty after flush", e.kind)
288+
}
289+
for _, e := range tc.expectNonEmpty {
290+
result := cm.GetCachedResourcesByKind(e.group, e.kind, "")
291+
assert.NotEmpty(t, result, "expected %s cache to still have data", e.kind)
292+
}
293+
})
294+
}
295+
}

pkg/agent/cache/manager.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ type Manager interface {
4242
HasLoadedPersistedCache() bool
4343
SaveCache()
4444
Flush()
45+
FlushKind(kind string)
4546

4647
// API Service cache related methods
4748
AddAPIService(resource *v1.ResourceInstance) error

pkg/agent/cachevalidationjob.go

Lines changed: 29 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package agent
22

33
import (
44
"fmt"
5+
"sync"
56
"time"
67

78
agentcache "github.com/Axway/agent-sdk/pkg/agent/cache"
@@ -41,17 +42,39 @@ func newCacheValidator(
4142
}
4243
}
4344

44-
func (cv *cacheValidator) Execute() error {
45+
// Execute validates each filter in the watch topic against the API server.
46+
// It returns the slice of filters whose cache is out of sync, plus errCacheOutOfSync
47+
// if any failed, or a nil slice and nil error if all filters are in sync.
48+
func (cv *cacheValidator) Execute() ([]management.WatchTopicSpecFilters, error) {
4549
cv.logger.Debug("executing cache validation")
4650

47-
for _, filter := range cv.watchTopic.Spec.Filters {
48-
if !cv.validateKind(filter) {
49-
return errCacheOutOfSync
50-
}
51+
filters := cv.watchTopic.Spec.Filters
52+
ch := make(chan management.WatchTopicSpecFilters, len(filters))
53+
54+
var wg sync.WaitGroup
55+
for _, filter := range filters {
56+
wg.Add(1)
57+
go func(f management.WatchTopicSpecFilters) {
58+
defer wg.Done()
59+
if !cv.validateKind(f) {
60+
ch <- f
61+
}
62+
}(filter)
63+
}
64+
wg.Wait()
65+
close(ch)
66+
67+
var failed []management.WatchTopicSpecFilters
68+
for f := range ch {
69+
failed = append(failed, f)
70+
}
71+
72+
if len(failed) > 0 {
73+
return failed, errCacheOutOfSync
5174
}
5275

5376
cv.logger.Debug("cache validation passed")
54-
return nil
77+
return nil, nil
5578
}
5679

5780
func (cv *cacheValidator) validateKind(filter management.WatchTopicSpecFilters) bool {

pkg/agent/cachevalidationjob_test.go

Lines changed: 69 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
apiv1 "github.com/Axway/agent-sdk/pkg/apic/apiserver/models/api/v1"
1111
management "github.com/Axway/agent-sdk/pkg/apic/apiserver/models/management/v1alpha1"
1212
"github.com/stretchr/testify/assert"
13+
"github.com/stretchr/testify/require"
1314
)
1415

1516
// mockCacheGetter implements cacheGetter for tests
@@ -123,9 +124,10 @@ func TestCacheValidator_Execute(t *testing.T) {
123124
}
124125

125126
type testCase struct {
126-
setup func(*mockResourceClient, *mockCacheGetter)
127-
watchTopic *management.WatchTopic
128-
expectErr error
127+
setup func(*mockResourceClient, *mockCacheGetter)
128+
watchTopic *management.WatchTopic
129+
expectErr error
130+
expectedFailedKinds []string
129131
}
130132

131133
tests := map[string]testCase{
@@ -137,7 +139,8 @@ func TestCacheValidator_Execute(t *testing.T) {
137139
},
138140
},
139141
},
140-
expectErr: nil,
142+
expectErr: nil,
143+
expectedFailedKinds: nil,
141144
},
142145
"in sync": {
143146
setup: func(client *mockResourceClient, cacheMan *mockCacheGetter) {
@@ -146,8 +149,9 @@ func TestCacheValidator_Execute(t *testing.T) {
146149
agentcache.ResourceCacheKey(svcGVK.Kind, scopeName, "svc1"): modTime,
147150
})
148151
},
149-
watchTopic: singleScopeWatchTopic(svcGVK, scopeName),
150-
expectErr: nil,
152+
watchTopic: singleScopeWatchTopic(svcGVK, scopeName),
153+
expectErr: nil,
154+
expectedFailedKinds: nil,
151155
},
152156
"count mismatch": {
153157
setup: func(client *mockResourceClient, cacheMan *mockCacheGetter) {
@@ -156,8 +160,9 @@ func TestCacheValidator_Execute(t *testing.T) {
156160
agentcache.ResourceCacheKey(svcGVK.Kind, scopeName, "svc1"): modTime,
157161
})
158162
},
159-
watchTopic: singleScopeWatchTopic(svcGVK, scopeName),
160-
expectErr: errCacheOutOfSync,
163+
watchTopic: singleScopeWatchTopic(svcGVK, scopeName),
164+
expectErr: errCacheOutOfSync,
165+
expectedFailedKinds: []string{svcGVK.Kind},
161166
},
162167
"resource missing from cache": {
163168
setup: func(client *mockResourceClient, cacheMan *mockCacheGetter) {
@@ -166,8 +171,9 @@ func TestCacheValidator_Execute(t *testing.T) {
166171
agentcache.ResourceCacheKey(svcGVK.Kind, scopeName, "svc-other"): modTime,
167172
})
168173
},
169-
watchTopic: singleScopeWatchTopic(svcGVK, scopeName),
170-
expectErr: errCacheOutOfSync,
174+
watchTopic: singleScopeWatchTopic(svcGVK, scopeName),
175+
expectErr: errCacheOutOfSync,
176+
expectedFailedKinds: []string{svcGVK.Kind},
171177
},
172178
"modify time mismatch": {
173179
setup: func(client *mockResourceClient, cacheMan *mockCacheGetter) {
@@ -177,8 +183,9 @@ func TestCacheValidator_Execute(t *testing.T) {
177183
agentcache.ResourceCacheKey(svcGVK.Kind, scopeName, "svc1"): modTime,
178184
})
179185
},
180-
watchTopic: singleScopeWatchTopic(svcGVK, scopeName),
181-
expectErr: errCacheOutOfSync,
186+
watchTopic: singleScopeWatchTopic(svcGVK, scopeName),
187+
expectErr: errCacheOutOfSync,
188+
expectedFailedKinds: []string{svcGVK.Kind},
182189
},
183190
"zero timestamps are ignored": {
184191
setup: func(client *mockResourceClient, cacheMan *mockCacheGetter) {
@@ -188,8 +195,9 @@ func TestCacheValidator_Execute(t *testing.T) {
188195
agentcache.ResourceCacheKey(svcGVK.Kind, scopeName, "svc1"): time.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC),
189196
})
190197
},
191-
watchTopic: singleScopeWatchTopic(svcGVK, scopeName),
192-
expectErr: nil,
198+
watchTopic: singleScopeWatchTopic(svcGVK, scopeName),
199+
expectErr: nil,
200+
expectedFailedKinds: nil,
193201
},
194202
"extra resource in cache": {
195203
setup: func(client *mockResourceClient, cacheMan *mockCacheGetter) {
@@ -199,15 +207,17 @@ func TestCacheValidator_Execute(t *testing.T) {
199207
agentcache.ResourceCacheKey(svcGVK.Kind, scopeName, "svc3"): modTime,
200208
})
201209
},
202-
watchTopic: singleScopeWatchTopic(svcGVK, scopeName),
203-
expectErr: errCacheOutOfSync,
210+
watchTopic: singleScopeWatchTopic(svcGVK, scopeName),
211+
expectErr: errCacheOutOfSync,
212+
expectedFailedKinds: []string{svcGVK.Kind},
204213
},
205214
"fetch error": {
206215
setup: func(client *mockResourceClient, _ *mockCacheGetter) {
207216
client.err = fmt.Errorf("network error")
208217
},
209-
watchTopic: singleScopeWatchTopic(svcGVK, scopeName),
210-
expectErr: errCacheOutOfSync,
218+
watchTopic: singleScopeWatchTopic(svcGVK, scopeName),
219+
expectErr: errCacheOutOfSync,
220+
expectedFailedKinds: []string{svcGVK.Kind},
211221
},
212222
"APIServiceInstances from multiple scopes are each validated independently": {
213223
setup: func(client *mockResourceClient, cacheMan *mockCacheGetter) {
@@ -228,7 +238,8 @@ func TestCacheValidator_Execute(t *testing.T) {
228238
},
229239
},
230240
},
231-
expectErr: nil,
241+
expectErr: nil,
242+
expectedFailedKinds: nil,
232243
},
233244
"multiple kinds in sync": {
234245
setup: func(client *mockResourceClient, cacheMan *mockCacheGetter) {
@@ -249,7 +260,8 @@ func TestCacheValidator_Execute(t *testing.T) {
249260
},
250261
},
251262
},
252-
expectErr: nil,
263+
expectErr: nil,
264+
expectedFailedKinds: nil,
253265
},
254266
"second kind out of sync": {
255267
setup: func(client *mockResourceClient, cacheMan *mockCacheGetter) {
@@ -268,7 +280,25 @@ func TestCacheValidator_Execute(t *testing.T) {
268280
},
269281
},
270282
},
271-
expectErr: errCacheOutOfSync,
283+
expectErr: errCacheOutOfSync,
284+
expectedFailedKinds: []string{instGVK.Kind},
285+
},
286+
"both kinds out of sync": {
287+
setup: func(client *mockResourceClient, cacheMan *mockCacheGetter) {
288+
client.resources[svc1.GetKindLink()] = []*apiv1.ResourceInstance{svc1}
289+
client.resources[inst1.GetKindLink()] = []*apiv1.ResourceInstance{inst1}
290+
// both empty in cache
291+
},
292+
watchTopic: &management.WatchTopic{
293+
Spec: management.WatchTopicSpec{
294+
Filters: []management.WatchTopicSpecFilters{
295+
{Group: svcGVK.Group, Kind: svcGVK.Kind, Name: "*", Scope: &management.WatchTopicSpecScope{Kind: "Environment", Name: scopeName}},
296+
{Group: instGVK.Group, Kind: instGVK.Kind, Name: "*", Scope: &management.WatchTopicSpecScope{Kind: "Environment", Name: scopeName}},
297+
},
298+
},
299+
},
300+
expectErr: errCacheOutOfSync,
301+
expectedFailedKinds: []string{svcGVK.Kind, instGVK.Kind},
272302
},
273303
"empty server and cache": {
274304
setup: func(client *mockResourceClient, _ *mockCacheGetter) {
@@ -285,15 +315,17 @@ func TestCacheValidator_Execute(t *testing.T) {
285315
}
286316
client.resources[ri.GetKindLink()] = []*apiv1.ResourceInstance{}
287317
},
288-
watchTopic: singleScopeWatchTopic(svcGVK, scopeName),
289-
expectErr: nil,
318+
watchTopic: singleScopeWatchTopic(svcGVK, scopeName),
319+
expectErr: nil,
320+
expectedFailedKinds: nil,
290321
},
291322
"ComplianceRuntimeResult out of sync is detected": {
292323
setup: func(client *mockResourceClient, _ *mockCacheGetter) {
293324
client.resources[crr1.GetKindLink()] = []*apiv1.ResourceInstance{crr1}
294325
},
295-
watchTopic: singleScopeWatchTopic(crrGVK, scopeName),
296-
expectErr: errCacheOutOfSync,
326+
watchTopic: singleScopeWatchTopic(crrGVK, scopeName),
327+
expectErr: errCacheOutOfSync,
328+
expectedFailedKinds: []string{crrGVK.Kind},
297329
},
298330
}
299331

@@ -305,8 +337,19 @@ func TestCacheValidator_Execute(t *testing.T) {
305337
tc.setup(client, cacheMan)
306338
}
307339
cv := newCacheValidator(client, tc.watchTopic, cacheMan)
308-
err := cv.Execute()
340+
failedFilters, err := cv.Execute()
309341
assert.Equal(t, tc.expectErr, err)
342+
343+
if len(tc.expectedFailedKinds) == 0 {
344+
assert.Empty(t, failedFilters)
345+
} else {
346+
require.Len(t, failedFilters, len(tc.expectedFailedKinds))
347+
actualKinds := make([]string, len(failedFilters))
348+
for i, f := range failedFilters {
349+
actualKinds[i] = f.Kind
350+
}
351+
assert.ElementsMatch(t, tc.expectedFailedKinds, actualKinds)
352+
}
310353
})
311354
}
312355
}

0 commit comments

Comments
 (0)