Skip to content

fix: Restore namespace discovery label on FeatureStore reconcile - #347

Merged
jyejare merged 1 commit into
red-hat-data-services:mainfrom
patelchaitany:fix/cherry-pick-pr-341-namespace-label
Sep 3, 2026
Merged

jyejare merged 1 commit into
red-hat-data-services:mainfrom
patelchaitany:fix/cherry-pick-pr-341-namespace-label

Conversation

@patelchaitany

Copy link
Copy Markdown

The operator is documented to add opendatahub.io/feast: "true" to any namespace holding a deployed FeatureStore, so dashboards can discover Feast namespaces cluster-wide via that label selector. The reconcile wiring for it was dropped during an upstream merge, leaving access.EnsureNamespaceLabel and access.RemoveNamespaceLabelIfLast orphaned while the manager-role kept the now-unused namespaces: update permission.

Wire both back into Reconcile:

  • Add the label in the post-deploy block, gated on the Ready condition, so it lands only after the FeatureStore is actually deployed.
  • Remove it on both deletion paths (CR-not-found and DeletionTimestamp set), and only when no other live FeatureStore remains in the namespace.
  • Run the removal before the namespace-registry and OpenLineage cleanups, which return early on error and would otherwise strand the label.

countOtherFeatureStoresInNamespace returns (int, error) so a List failure keeps the label in place, rather than the -1 sentinel used elsewhere which fails the > 0 check and unlabels the namespace on any transient API error.

Regenerated with make manifests generate bundle, which produced no manifest changes: the marker already granted core/namespaces: get;list;watch;update.

What this PR does / why we need it:

Which issue(s) this PR fixes:

Checks

  • I've made sure the tests are passing.
  • My commits are signed off (git commit -s)
  • My PR title follows conventional commits format

Testing Strategy

  • Unit tests
  • Integration tests
  • Manual tests
  • Testing is not required for this change

Misc

The operator is documented to add `opendatahub.io/feast: "true"` to any
namespace holding a deployed FeatureStore, so dashboards can discover Feast
namespaces cluster-wide via that label selector. The reconcile wiring for it
was dropped during an upstream merge, leaving access.EnsureNamespaceLabel and
access.RemoveNamespaceLabelIfLast orphaned while the manager-role kept the
now-unused `namespaces: update` permission.

Wire both back into Reconcile:

- Add the label in the post-deploy block, gated on the Ready condition, so it
  lands only after the FeatureStore is actually deployed.
- Remove it on both deletion paths (CR-not-found and DeletionTimestamp set),
  and only when no other live FeatureStore remains in the namespace.
- Run the removal before the namespace-registry and OpenLineage cleanups,
  which return early on error and would otherwise strand the label.

countOtherFeatureStoresInNamespace returns (int, error) so a List failure
keeps the label in place, rather than the -1 sentinel used elsewhere which
fails the `> 0` check and unlabels the namespace on any transient API error.

Regenerated with `make manifests generate bundle`, which produced no manifest
changes: the marker already granted `core/namespaces: get;list;watch;update`.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Signed-off-by: Chaitany Patel <patelchaitany93@gmail.com>
@jyejare
jyejare merged commit f033577 into red-hat-data-services:main Sep 3, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants