Conversation
vivekr-splunk
requested review from
Igor-splunk,
Copilot,
kasiakoziol,
kubabuczak and
rlieberman-splunk
There was a problem hiding this comment.
Pull request overview
This PR introduces a way to perform live (non-cached) reads of Splunk Enterprise CRs right before status updates, reducing the chance of updating status based on stale cached objects while still retaining the existing cache-convergence loop after status writes.
Changes:
- Added a
client.Clientwrapper that exposes the controller-runtimeAPIReader, plus a helper to resolve it when available. - Updated CR status refresh logic to use the resolved live reader for the pre-status-update re-fetch.
- Wired the new wrapped client into the manager setup so enterprise reconcilers receive an API-reader-aware client.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| pkg/splunk/enterprise/util.go | Uses a resolved live client.Reader for the “fresh read” before status updates while keeping cached reads for convergence checks. |
| pkg/splunk/common/client.go | Adds APIAwareClient wrapper + ResolveAPIReader helper to expose/resolve a live API reader. |
| cmd/main.go | Wraps the manager client with the API reader and passes it into enterprise reconcilers. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // APIReaderProvider exposes a direct API reader alongside a controller client. | ||
| type APIReaderProvider interface { | ||
| GetAPIReader() client.Reader | ||
| } | ||
|
|
||
| // APIAwareClient keeps the standard controller client behavior while exposing a | ||
| // direct API reader for code paths that need a live read before a write. | ||
| type APIAwareClient struct { | ||
| client.Client | ||
| apiReader client.Reader | ||
| } | ||
|
|
||
| // NewAPIAwareClient returns a client wrapper that exposes the provided API reader. | ||
| func NewAPIAwareClient(baseClient client.Client, apiReader client.Reader) client.Client { | ||
| if baseClient == nil || apiReader == nil { | ||
| return baseClient | ||
| } | ||
|
|
||
| return &APIAwareClient{ | ||
| Client: baseClient, | ||
| apiReader: apiReader, | ||
| } | ||
| } | ||
|
|
||
| // GetAPIReader returns the live API reader associated with the wrapped client. | ||
| func (c *APIAwareClient) GetAPIReader() client.Reader { | ||
| if c.apiReader != nil { | ||
| return c.apiReader | ||
| } | ||
|
|
||
| return c.Client | ||
| } | ||
|
|
||
| // ResolveAPIReader returns a live API reader when one is available, otherwise | ||
| // it falls back to the provided client. | ||
| func ResolveAPIReader(baseClient client.Client) client.Reader { | ||
| if provider, ok := baseClient.(APIReaderProvider); ok { | ||
| if apiReader := provider.GetAPIReader(); apiReader != nil { | ||
| return apiReader | ||
| } | ||
| } | ||
|
|
||
| return baseClient | ||
| } |
There was a problem hiding this comment.
New exported API-reader wrapper helpers are introduced here, but there are no unit tests validating the key behaviors (e.g., ResolveAPIReader returns the provided API reader when the client implements APIReaderProvider, and falls back to the base client when it does not / when nils are provided). Adding a small client_test.go would help prevent regressions since this code changes read consistency around status updates.
vivekr-splunk
requested review from
gabrielm-splunk,
minjieqiu,
patrykw-splunk and
qingw-splunk
Summary
Testing