-
Notifications
You must be signed in to change notification settings - Fork 47
feat: patch kagenti-ui-config ConfigMap with MLflow dashboard URL at startup #411
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,105 @@ | ||
| /* | ||
| Copyright 2026. | ||
|
|
||
| Licensed under the Apache License, Version 2.0 (the "License"); | ||
| you may not use this file except in compliance with the License. | ||
| You may obtain a copy of the License at | ||
|
|
||
| http://www.apache.org/licenses/LICENSE-2.0 | ||
|
|
||
| Unless required by applicable law or agreed to in writing, software | ||
| distributed under the License is distributed on an "AS IS" BASIS, | ||
| WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| See the License for the specific language governing permissions and | ||
| limitations under the License. | ||
| */ | ||
|
|
||
| package bootstrap | ||
|
|
||
| import ( | ||
| "context" | ||
| "strings" | ||
|
|
||
| "github.com/go-logr/logr" | ||
| corev1 "k8s.io/api/core/v1" | ||
| "k8s.io/apimachinery/pkg/api/errors" | ||
| "k8s.io/apimachinery/pkg/api/meta" | ||
| "k8s.io/apimachinery/pkg/types" | ||
| "sigs.k8s.io/controller-runtime/pkg/client" | ||
|
|
||
| "github.com/kagenti/operator/internal/mlflow" | ||
| ) | ||
|
|
||
| const ( | ||
| uiConfigMapName = "kagenti-ui-config" | ||
| mlflowDashboardURLKey = "MLFLOW_DASHBOARD_URL" | ||
| ) | ||
|
|
||
| // UIConfigBootstrapRunnable patches the kagenti-ui-config ConfigMap with the | ||
| // MLflow dashboard URL. Runs once at startup; skips gracefully if MLflow or | ||
| // the UI ConfigMap are not present. | ||
| type UIConfigBootstrapRunnable struct { | ||
| Client client.Client | ||
| APIReader client.Reader | ||
| Namespace string | ||
| Log logr.Logger | ||
| } | ||
|
|
||
| func (r *UIConfigBootstrapRunnable) Start(ctx context.Context) error { | ||
| log := r.Log.WithName("ui-config-bootstrap") | ||
|
|
||
| dashboardURL := r.discoverDashboardURL(ctx, log) | ||
| if dashboardURL == "" { | ||
| return nil | ||
| } | ||
|
|
||
| cm := &corev1.ConfigMap{} | ||
| key := types.NamespacedName{Name: uiConfigMapName, Namespace: r.Namespace} | ||
| if err := r.APIReader.Get(ctx, key, cm); err != nil { | ||
| if errors.IsNotFound(err) { | ||
| log.V(1).Info("UI ConfigMap not found, skipping (UI likely not deployed)") | ||
| return nil | ||
| } | ||
| log.Error(err, "Failed to read UI ConfigMap") | ||
| return nil | ||
| } | ||
|
|
||
| if cm.Data != nil && cm.Data[mlflowDashboardURLKey] == dashboardURL { | ||
| log.V(1).Info("MLflow dashboard URL already set") | ||
| return nil | ||
| } | ||
|
|
||
| if cm.Data == nil { | ||
| cm.Data = make(map[string]string) | ||
| } | ||
| cm.Data[mlflowDashboardURLKey] = dashboardURL | ||
| if err := r.Client.Update(ctx, cm); err != nil { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion: The PR title says "patch" but this is a read-modify-write
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Switched to |
||
| log.Error(err, "Failed to patch UI ConfigMap with MLflow dashboard URL") | ||
| return nil | ||
| } | ||
|
|
||
| log.Info("Patched UI ConfigMap with MLflow dashboard URL", "url", dashboardURL) | ||
| return nil | ||
| } | ||
|
|
||
| func (r *UIConfigBootstrapRunnable) NeedLeaderElection() bool { | ||
| return true | ||
| } | ||
|
|
||
| func (r *UIConfigBootstrapRunnable) discoverDashboardURL(ctx context.Context, log logr.Logger) string { | ||
| list := &mlflow.MLflowList{} | ||
| if err := r.APIReader.List(ctx, list); err != nil { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit:
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Documented the single-instance assumption — Kagenti deploys one MLflow per platform, so cluster-wide list is intentional and the first Available CR is always the correct one. |
||
| log.V(1).Info("Could not list MLflow CRs, skipping", "error", err) | ||
| return "" | ||
| } | ||
|
|
||
| for i := range list.Items { | ||
| cr := &list.Items[i] | ||
| if meta.IsStatusConditionTrue(cr.Status.Conditions, "Available") && cr.Status.URL != "" { | ||
| return strings.TrimRight(cr.Status.URL, "/") + "/" | ||
| } | ||
| } | ||
|
|
||
| log.V(1).Info("No available MLflow CR with external URL found") | ||
| return "" | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,130 @@ | ||
| /* | ||
| Copyright 2026. | ||
|
|
||
| Licensed under the Apache License, Version 2.0 (the "License"); | ||
| you may not use this file except in compliance with the License. | ||
| You may obtain a copy of the License at | ||
|
|
||
| http://www.apache.org/licenses/LICENSE-2.0 | ||
|
|
||
| Unless required by applicable law or agreed to in writing, software | ||
| distributed under the License is distributed on an "AS IS" BASIS, | ||
| WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| See the License for the specific language governing permissions and | ||
| limitations under the License. | ||
| */ | ||
|
|
||
| package bootstrap | ||
|
|
||
| import ( | ||
| "context" | ||
| "testing" | ||
|
|
||
| corev1 "k8s.io/api/core/v1" | ||
| metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" | ||
| "k8s.io/apimachinery/pkg/types" | ||
| "sigs.k8s.io/controller-runtime/pkg/client" | ||
| "sigs.k8s.io/controller-runtime/pkg/client/fake" | ||
| ) | ||
|
|
||
| func newUIConfigRunnable(objs ...client.Object) *UIConfigBootstrapRunnable { | ||
| scheme := testScheme() | ||
| cl := fake.NewClientBuilder().WithScheme(scheme).WithObjects(objs...).Build() | ||
| return &UIConfigBootstrapRunnable{ | ||
| Client: cl, | ||
| APIReader: cl, | ||
| Namespace: testNamespace, | ||
| Log: testLogger(), | ||
| } | ||
| } | ||
|
|
||
| func TestUIConfig_PatchesConfigMap(t *testing.T) { | ||
| cr := mlflowCR("mlflow", "", true, "https://mlflow-gateway.apps.example.com") | ||
| cm := &corev1.ConfigMap{ | ||
| ObjectMeta: metav1.ObjectMeta{Name: uiConfigMapName, Namespace: testNamespace}, | ||
| Data: map[string]string{"DOMAIN_NAME": "example.com"}, | ||
| } | ||
|
|
||
| r := newUIConfigRunnable(cr, cm) | ||
| if err := r.Start(context.Background()); err != nil { | ||
| t.Fatalf("Start() failed: %v", err) | ||
| } | ||
|
|
||
| got := &corev1.ConfigMap{} | ||
| _ = r.Client.Get(context.Background(), types.NamespacedName{Name: uiConfigMapName, Namespace: testNamespace}, got) | ||
|
|
||
| if got.Data[mlflowDashboardURLKey] != "https://mlflow-gateway.apps.example.com/" { | ||
| t.Errorf("got %q, want %q", got.Data[mlflowDashboardURLKey], "https://mlflow-gateway.apps.example.com/") | ||
| } | ||
| if got.Data["DOMAIN_NAME"] != "example.com" { | ||
| t.Error("existing keys were modified") | ||
| } | ||
| } | ||
|
|
||
| func TestUIConfig_Idempotent(t *testing.T) { | ||
| cr := mlflowCR("mlflow", "", true, "https://mlflow.apps.cluster.com/mlflow") | ||
| cm := &corev1.ConfigMap{ | ||
| ObjectMeta: metav1.ObjectMeta{Name: uiConfigMapName, Namespace: testNamespace}, | ||
| Data: map[string]string{mlflowDashboardURLKey: "https://mlflow.apps.cluster.com/mlflow/"}, | ||
| } | ||
|
|
||
| r := newUIConfigRunnable(cr, cm) | ||
| if err := r.Start(context.Background()); err != nil { | ||
| t.Fatalf("Start() failed: %v", err) | ||
| } | ||
|
|
||
| got := &corev1.ConfigMap{} | ||
| _ = r.Client.Get(context.Background(), types.NamespacedName{Name: uiConfigMapName, Namespace: testNamespace}, got) | ||
|
|
||
| if got.Data[mlflowDashboardURLKey] != "https://mlflow.apps.cluster.com/mlflow/" { | ||
| t.Error("URL was unexpectedly modified") | ||
| } | ||
| } | ||
|
|
||
| func TestUIConfig_SkipsWhenNoMLflow(t *testing.T) { | ||
| cm := &corev1.ConfigMap{ | ||
| ObjectMeta: metav1.ObjectMeta{Name: uiConfigMapName, Namespace: testNamespace}, | ||
| Data: map[string]string{}, | ||
| } | ||
|
|
||
| r := newUIConfigRunnable(cm) | ||
| if err := r.Start(context.Background()); err != nil { | ||
| t.Fatalf("Start() failed: %v", err) | ||
| } | ||
|
|
||
| got := &corev1.ConfigMap{} | ||
| _ = r.Client.Get(context.Background(), types.NamespacedName{Name: uiConfigMapName, Namespace: testNamespace}, got) | ||
|
|
||
| if _, ok := got.Data[mlflowDashboardURLKey]; ok { | ||
| t.Error("should not set URL when no MLflow CR exists") | ||
| } | ||
| } | ||
|
|
||
| func TestUIConfig_SkipsWhenNoConfigMap(t *testing.T) { | ||
| cr := mlflowCR("mlflow", "", true, "https://mlflow.apps.cluster.com") | ||
|
|
||
| r := newUIConfigRunnable(cr) | ||
| if err := r.Start(context.Background()); err != nil { | ||
| t.Fatalf("Start() failed: %v", err) | ||
| } | ||
| } | ||
|
|
||
| func TestUIConfig_NormalizesTrailingSlash(t *testing.T) { | ||
| cr := mlflowCR("mlflow", "", true, "https://mlflow.apps.cluster.com/mlflow") | ||
| cm := &corev1.ConfigMap{ | ||
| ObjectMeta: metav1.ObjectMeta{Name: uiConfigMapName, Namespace: testNamespace}, | ||
| Data: map[string]string{}, | ||
| } | ||
|
|
||
| r := newUIConfigRunnable(cr, cm) | ||
| if err := r.Start(context.Background()); err != nil { | ||
| t.Fatalf("Start() failed: %v", err) | ||
| } | ||
|
|
||
| got := &corev1.ConfigMap{} | ||
| _ = r.Client.Get(context.Background(), types.NamespacedName{Name: uiConfigMapName, Namespace: testNamespace}, got) | ||
|
|
||
| if got.Data[mlflowDashboardURLKey] != "https://mlflow.apps.cluster.com/mlflow/" { | ||
| t.Errorf("got %q, expected trailing slash to be added", got.Data[mlflowDashboardURLKey]) | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion: One-shot bootstrap silently gives up if no MLflow CR is
Availableyet.Start()runs once; on a fresh install the MLflow Route is typically admitted after the operator boots, sodiscoverDashboardURLreturns"", this returnsnil, and the ConfigMap is never patched until the operator pod restarts. This couples correct behavior to startup ordering. Consider a watch/requeue on MLflow status (or a periodic retry) so the patch is applied whenever MLflow becomes ready — the operator handling ordering rather than depending on it.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the suggestion, I added a retry loop with backoff (~4 min window), previously the kagenti setup script would wait 2mins. A full controller/watch felt heavyweight for a one-shot ConfigMap patch, the retry covers the typical MLflow startup lag. If MLflow takes significantly longer, a pod restart or manual reconciliation could handle this.