fix(operator): Only deploy an online store when it is declared#6591
Open
nikolauspschuetz wants to merge 2 commits into
Open
fix(operator): Only deploy an online store when it is declared#6591nikolauspschuetz wants to merge 2 commits into
nikolauspschuetz wants to merge 2 commits into
Conversation
ApplyDefaultsToStatus created an online store service (including a serving pod) whenever spec.services.onlineStore was omitted, so the operator always deployed an online store even for a standalone registry or offline store. This contradicts the documented federated / shared-registry pattern and differs from how registry and offlineStore behave (both are only defaulted when declared). Gate the online-store defaulting behind `services.OnlineStore != nil`, mirroring registry and offlineStore. A declared online store still receives all of its defaults. Adds unit coverage for both cases. Closes feast-dev#6586 Signed-off-by: Nikolaus Schuetz <nikolauspschuetz@gmail.com>
…on the implicit default The operator no longer creates an online store when a FeatureStore omits spec.services.onlineStore (feast-dev#6586). These existing tests built CRs that declared other services (or nothing) and relied on the online store being created implicitly, so they now either fail the "at least one local server" guard or find no online container/service. Declare the online store explicitly where the test intent is to exercise an online store, and assert its absence in the registry+offlineStore case so it also serves as a regression check for feast-dev#6586. Signed-off-by: Nikolaus Schuetz <nikolauspschuetz@gmail.com>
Contributor
Author
|
The red `unit-test-python (3.11, macos-14)` leg is an unrelated CI flake, not caused by this change (which is Go-only, scoped to `infra/feast-operator/`): ``` A `feast apply` subprocess timed out after 120s on the macOS-14 runner. The macOS-3.12 leg and all Ubuntu legs (3.10/3.11/3.12) passed on this same commit, and `operator-test` is green. I do not have permission to re-run the job from the fork — a maintainer re-run of that single leg should clear it. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #6586.
Problem
The Feast operator always deployed an online store service (including a serving pod), even when the
FeatureStoreCR did not declarespec.services.onlineStore. This makes it impossible to run a standalone registry or offline store (e.g. the documented federated / shared-registry pattern), and is inconsistent with howregistryandofflineStorebehave — both are only defaulted when declared.Cause
In
ApplyDefaultsToStatus(infra/feast-operator/internal/controller/services/util.go), the online-store block force-created the service when nil:whereas
registryandofflineStoreare gated behindif services.X != nil.Fix
Gate the online-store defaulting behind
services.OnlineStore != nil, mirroring the two sibling services. A declared online store still receives all of its defaults (persistence, path, PVC, server, container configs).Tests
Added
util_test.gowith two specs forApplyDefaultsToStatus:Status.Applied.Services.OnlineStorestaysnil(fails onmaster, passes with this change),Verified with
KUBEBUILDER_ASSETS=... go test ./internal/controller/services/(envtest 1.31.0);go build ./...,go vet, andgofmtare clean. The existing controller tests are unaffected because they declareonlineStoreexplicitly.