Skip to content

Commit ab6153a

Browse files
jyejarejyejare
committed
Dynamic controller namespace
Signed-off-by: jyejare <jyejare@redhat.com>
1 parent dedf2cc commit ab6153a

File tree

4 files changed

+68
-70
lines changed

4 files changed

+68
-70
lines changed

‎infra/feast-operator/docs/namespace-registry.md‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,6 @@ The Feast Namespace Registry is a feature that automatically creates and maintai
66

77
## Implementation Details
88

9-
### RHOAIENG-33698 Requirements
10-
11-
This implementation addresses the following requirements from [RHOAIENG-33698](https://issues.redhat.com/browse/RHOAIENG-33698):
12-
139
1. **ConfigMap Creation**: The operator creates a ConfigMap in the appropriate namespace:
1410
- **OpenShift AI**: `redhat-ods-applications` namespace (or DSCi configured namespace)
1511
- **Kubernetes**: `feast-operator-system` namespace
@@ -22,7 +18,7 @@ This implementation addresses the following requirements from [RHOAIENG-33698](h
2218

2319
### ConfigMap Structure
2420

25-
The namespace registry ConfigMap (`feast-namespace-registry`) contains the following data:
21+
The namespace registry ConfigMap (`feast-configs-registry`) contains the following data:
2622

2723
```json
2824
{
@@ -44,10 +40,10 @@ External applications can discover Feast instances by:
4440
1. Reading the ConfigMap from the appropriate namespace:
4541
```bash
4642
# For OpenShift
47-
kubectl get configmap feast-namespace-registry -n redhat-ods-applications -o jsonpath='{.data.namespaces}'
43+
kubectl get configmap feast-configs-registry -n redhat-ods-applications -o jsonpath='{.data.namespaces}'
4844

4945
# For Kubernetes
50-
kubectl get configmap feast-namespace-registry -n feast-operator-system -o jsonpath='{.data.namespaces}'
46+
kubectl get configmap feast-configs-registry -n feast-operator-system -o jsonpath='{.data.namespaces}'
5147
```
5248

5349
### Lifecycle Management

‎infra/feast-operator/internal/controller/featurestore_controller_namespace_registry_test.go‎

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -450,15 +450,4 @@ var _ = Describe("FeatureStore Controller - Namespace Registry", func() {
450450
Expect(data.Namespaces).To(HaveLen(1))
451451
})
452452
})
453-
454-
Context("When testing namespace determination logic", func() {
455-
It("should return correct namespace for different environments", func() {
456-
By("Testing namespace determination constants")
457-
458-
// Test the constants are properly defined
459-
Expect(services.DefaultOpenShiftNamespace).NotTo(BeEmpty())
460-
Expect(services.DefaultKubernetesNamespace).NotTo(BeEmpty())
461-
Expect(services.DefaultOpenShiftNamespace).NotTo(Equal(services.DefaultKubernetesNamespace))
462-
})
463-
})
464453
})

‎infra/feast-operator/internal/controller/services/namespace_registry.go‎

Lines changed: 64 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ package services
1919
import (
2020
"encoding/json"
2121
"fmt"
22+
"os"
2223

2324
corev1 "k8s.io/api/core/v1"
2425
rbacv1 "k8s.io/api/rbac/v1"
@@ -51,7 +52,10 @@ func (feast *FeastServices) createNamespaceRegistryConfigMap() error {
5152
logger := log.FromContext(feast.Handler.Context)
5253

5354
// Determine the target namespace based on platform
54-
targetNamespace := feast.getNamespaceRegistryNamespace()
55+
targetNamespace, err := feast.getNamespaceRegistryNamespace()
56+
if err != nil {
57+
return fmt.Errorf("failed to get namespace registry namespace: %w", err)
58+
}
5559

5660
cm := &corev1.ConfigMap{
5761
ObjectMeta: metav1.ObjectMeta{
@@ -133,7 +137,10 @@ func (feast *FeastServices) setNamespaceRegistryConfigMap(cm *corev1.ConfigMap)
133137
func (feast *FeastServices) createNamespaceRegistryRoleBinding() error {
134138
logger := log.FromContext(feast.Handler.Context)
135139

136-
targetNamespace := feast.getNamespaceRegistryNamespace()
140+
targetNamespace, err := feast.getNamespaceRegistryNamespace()
141+
if err != nil {
142+
return fmt.Errorf("failed to get namespace registry namespace: %w", err)
143+
}
137144

138145
roleBinding := &rbacv1.RoleBinding{
139146
ObjectMeta: metav1.ObjectMeta{
@@ -182,7 +189,6 @@ func (feast *FeastServices) setNamespaceRegistryRoleBinding(rb *rbacv1.RoleBindi
182189
Verbs: []string{"get", "list"},
183190
},
184191
}
185-
role.Labels = feast.getLabels()
186192
return nil
187193
})); err != nil {
188194
return err
@@ -203,37 +209,40 @@ func (feast *FeastServices) setNamespaceRegistryRoleBinding(rb *rbacv1.RoleBindi
203209
},
204210
}
205211

206-
rb.Labels = feast.getLabels()
207-
208212
return nil
209213
}
210214

211215
// getNamespaceRegistryNamespace determines the target namespace for the namespace registry ConfigMap
212-
func (feast *FeastServices) getNamespaceRegistryNamespace() string {
216+
func (feast *FeastServices) getNamespaceRegistryNamespace() (string, error) {
213217
// Check if we're running on OpenShift
214-
if IsOpenShiftForNamespaceRegistry() {
215-
// For OpenShift, use redhat-ods-applications or check for DSCi configuration
216-
// For now, we'll use the default OpenShift namespace
218+
logger := log.FromContext(feast.Handler.Context)
219+
if isOpenShift {
217220
// TODO: Add support for reading DSCi configuration
218-
return DefaultOpenShiftNamespace
221+
if data, err := os.ReadFile("/var/run/secrets/kubernetes.io/serviceaccount/namespace"); err == nil {
222+
if ns := string(data); len(ns) > 0 {
223+
logger.V(1).Info("Using OpenShift namespace", "namespace", ns)
224+
return ns, nil
225+
}
226+
}
227+
// This is what notebook controller team is doing, we are following them
228+
// They are not defaulting to redhat-ods-applications namespace
229+
return "", fmt.Errorf("unable to determine the namespace")
219230
}
220231

221-
return DefaultKubernetesNamespace
222-
}
223-
224-
// IsOpenShiftForNamespaceRegistry returns true if the operator is running on OpenShift
225-
func IsOpenShiftForNamespaceRegistry() bool {
226-
return isOpenShift
232+
return DefaultKubernetesNamespace, nil
227233
}
228234

229-
// RemoveFromNamespaceRegistry removes a feature store instance from the namespace registry
235+
// AddToNamespaceRegistry adds a feature store instance to the namespace registry
230236
func (feast *FeastServices) AddToNamespaceRegistry() error {
231237
logger := log.FromContext(feast.Handler.Context)
232-
targetNamespace := feast.getNamespaceRegistryNamespace()
238+
targetNamespace, err := feast.getNamespaceRegistryNamespace()
239+
if err != nil {
240+
return fmt.Errorf("failed to get namespace registry namespace: %w", err)
241+
}
233242

234243
// Get the existing ConfigMap
235244
cm := &corev1.ConfigMap{}
236-
err := feast.Handler.Client.Get(feast.Handler.Context, types.NamespacedName{
245+
err = feast.Handler.Client.Get(feast.Handler.Context, types.NamespacedName{
237246
Name: NamespaceRegistryConfigMapName,
238247
Namespace: targetNamespace,
239248
}, cm)
@@ -308,19 +317,22 @@ func (feast *FeastServices) AddToNamespaceRegistry() error {
308317
return nil
309318
}
310319

320+
// RemoveFromNamespaceRegistry removes a feature store instance from the namespace registry
311321
func (feast *FeastServices) RemoveFromNamespaceRegistry() error {
312322
logger := log.FromContext(feast.Handler.Context)
313323

314324
// Determine the target namespace based on platform
315-
targetNamespace := feast.getNamespaceRegistryNamespace()
325+
targetNamespace, err := feast.getNamespaceRegistryNamespace()
326+
if err != nil {
327+
return fmt.Errorf("failed to get namespace registry namespace: %w", err)
328+
}
316329

317330
// Get the existing ConfigMap
318331
cm := &corev1.ConfigMap{}
319-
err := feast.Handler.Client.Get(feast.Handler.Context, client.ObjectKey{
332+
err = feast.Handler.Client.Get(feast.Handler.Context, client.ObjectKey{
320333
Name: NamespaceRegistryConfigMapName,
321334
Namespace: targetNamespace,
322335
}, cm)
323-
324336
if err != nil {
325337
if apierrors.IsNotFound(err) {
326338
// ConfigMap doesn't exist, nothing to clean up
@@ -346,42 +358,45 @@ func (feast *FeastServices) RemoveFromNamespaceRegistry() error {
346358
// Remove current feature store instance from the registry
347359
featureStoreNamespace := feast.Handler.FeatureStore.Namespace
348360
clientConfigName := feast.Handler.FeatureStore.Status.ClientConfigMap
361+
featureStoreName := feast.Handler.FeatureStore.Name
362+
363+
// Generate expected client config name using the same logic as creation
364+
expectedClientConfigName := "feast-" + featureStoreName + "-client"
365+
366+
logger.Info("Removing feature store from registry",
367+
"featureStoreName", featureStoreName,
368+
"featureStoreNamespace", featureStoreNamespace,
369+
"clientConfigName", clientConfigName,
370+
"expectedClientConfigName", expectedClientConfigName)
349371

350372
if existingData.Namespaces[featureStoreNamespace] != nil {
351-
if clientConfigName != "" {
352-
// Remove the specific client config from the list
353-
var updatedConfigs []string
354-
for _, config := range existingData.Namespaces[featureStoreNamespace] {
355-
if config != clientConfigName {
356-
updatedConfigs = append(updatedConfigs, config)
357-
}
358-
}
359-
existingData.Namespaces[featureStoreNamespace] = updatedConfigs
360-
} else {
361-
// If we don't have the client config name, try to find and remove the config
362-
// that was created for this FeatureStore (it should follow the pattern: feast-{name}-client)
363-
featureStoreName := feast.Handler.FeatureStore.Name
364-
expectedClientConfigName := "feast-" + featureStoreName + "-client"
365-
logger.Info("Attempting to remove config by name pattern",
366-
"featureStoreName", featureStoreName,
367-
"expectedClientConfigName", expectedClientConfigName,
368-
"existingConfigs", existingData.Namespaces[featureStoreNamespace])
369-
var updatedConfigs []string
370-
for _, config := range existingData.Namespaces[featureStoreNamespace] {
371-
// Remove configs that match the FeatureStore name pattern
372-
if config != expectedClientConfigName {
373-
updatedConfigs = append(updatedConfigs, config)
374-
} else {
375-
logger.Info("Removing config from registry", "config", config)
376-
}
373+
var updatedConfigs []string
374+
removed := false
375+
376+
for _, config := range existingData.Namespaces[featureStoreNamespace] {
377+
// Remove if it matches the client config name or the expected pattern
378+
if config == clientConfigName || config == expectedClientConfigName {
379+
logger.Info("Removing config from registry", "config", config)
380+
removed = true
381+
} else {
382+
updatedConfigs = append(updatedConfigs, config)
377383
}
378-
existingData.Namespaces[featureStoreNamespace] = updatedConfigs
379384
}
380385

386+
existingData.Namespaces[featureStoreNamespace] = updatedConfigs
387+
381388
// If no configs left for this namespace, remove the namespace entry
382389
if len(existingData.Namespaces[featureStoreNamespace]) == 0 {
383390
delete(existingData.Namespaces, featureStoreNamespace)
391+
logger.Info("Removed empty namespace entry from registry", "namespace", featureStoreNamespace)
392+
}
393+
394+
if !removed {
395+
logger.V(1).Info("No matching config found to remove from registry",
396+
"existingConfigs", existingData.Namespaces[featureStoreNamespace])
384397
}
398+
} else {
399+
logger.V(1).Info("Namespace not found in registry", "namespace", featureStoreNamespace)
385400
}
386401

387402
// Marshal the updated data back to JSON

‎infra/feast-operator/internal/controller/services/services_types.go‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,8 @@ const (
3636
svcDomain = ".svc.cluster.local"
3737

3838
// Namespace registry ConfigMap constants
39-
NamespaceRegistryConfigMapName = "feast-namespace-registry"
39+
NamespaceRegistryConfigMapName = "feast-configs-registry"
4040
NamespaceRegistryDataKey = "namespaces"
41-
DefaultOpenShiftNamespace = "redhat-ods-applications" // TODO: make this configurable
4241
DefaultKubernetesNamespace = "feast-operator-system"
4342

4443
HttpPort = 80
@@ -64,7 +63,6 @@ const (
6463
ClientFeastType FeastServiceType = "client"
6564
ClientCaFeastType FeastServiceType = "client-ca"
6665
CronJobFeastType FeastServiceType = "cronjob"
67-
NamespaceRegistryFeastType FeastServiceType = "namespace-registry"
6866

6967
OfflineRemoteConfigType OfflineConfigType = "remote"
7068
OfflineFilePersistenceDaskConfigType OfflineConfigType = "dask"

0 commit comments

Comments
 (0)