From 1bae0775be1992769ff8e90a32a15594d69b4b01 Mon Sep 17 00:00:00 2001 From: Aleksei Sviridkin Date: Thu, 9 Apr 2026 20:25:20 +0300 Subject: [PATCH 1/4] feat(application): add WorkloadsReady condition and Events tab Add two features to improve application observability in the dashboard: 1. WorkloadsReady condition on Application status - Query WorkloadMonitor resources to determine if all pods are running - Expose WorkloadsReady as a separate condition alongside Ready - Ready continues to reflect HelmRelease state only (no override) to preserve backward compatibility with tooling and avoid false-negatives during normal startup windows when pods are still coming up - Handle three states: operational, not operational, unknown (pending) - Fail-open on WorkloadMonitor query errors - Integrate with Application Watch to emit MODIFIED events on WorkloadMonitor changes (with initial-events-end safety) - Register cozystack.io/v1alpha1 types in API server scheme 2. Events tab in dashboard - Show Kubernetes Events scoped to application namespace - Use status.namespace for Tenant applications - Include both lastTimestamp and eventTime columns for K8s compat 3. Bug fix: WorkloadMonitor Operational status persistence - Operational was written to stale 'monitor' instead of 'fresh' inside RetryOnConflict, so it was never persisted to the cluster Closes #2359 Closes #2360 Assisted-By: Claude Signed-off-by: Aleksei Sviridkin --- .gitignore | 1 + internal/controller/dashboard/factory.go | 32 ++ internal/controller/dashboard/factory_test.go | 60 +++ .../controller/dashboard/static_refactored.go | 11 + .../controller/workloadmonitor_controller.go | 4 +- .../workloadmonitor_controller_test.go | 180 +++++++++ pkg/apiserver/apiserver.go | 7 + pkg/registry/apps/application/rest.go | 186 ++++++++++ .../apps/application/rest_conditions_test.go | 331 +++++++++++++++++ .../apps/application/rest_watch_test.go | 223 ++++++++++++ .../apps/application/rest_workloads_test.go | 342 ++++++++++++++++++ 11 files changed, 1375 insertions(+), 2 deletions(-) create mode 100644 internal/controller/dashboard/factory_test.go create mode 100644 internal/controller/workloadmonitor_controller_test.go create mode 100644 pkg/registry/apps/application/rest_conditions_test.go create mode 100644 pkg/registry/apps/application/rest_watch_test.go create mode 100644 pkg/registry/apps/application/rest_workloads_test.go diff --git a/.gitignore b/.gitignore index 4e1e3119..61003de5 100644 --- a/.gitignore +++ b/.gitignore @@ -83,3 +83,4 @@ tmp/ # build revision marker (generated by make image-packages) packages/core/platform/.build-revision +.claude/ diff --git a/internal/controller/dashboard/factory.go b/internal/controller/dashboard/factory.go index 539e503b..fe0cadfd 100644 --- a/internal/controller/dashboard/factory.go +++ b/internal/controller/dashboard/factory.go @@ -47,6 +47,7 @@ func (m *Manager) ensureFactory(ctx context.Context, crd *cozyv1alpha1.Applicati if prefix, ok := vncTabPrefix(kind); ok { tabs = append(tabs, vncTab(prefix)) } + tabs = append(tabs, eventsTab(kind)) tabs = append(tabs, yamlTab(g, v, plural)) // Use unified factory creation @@ -358,6 +359,37 @@ func secretsTab(kind string) map[string]any { } } +// eventsTab shows Kubernetes Events scoped to the application's namespace. +// Events are namespace-scoped because Kubernetes Events don't carry application +// labels and cannot be filtered by label selector. In Cozystack's multi-tenancy +// model, each tenant namespace maps to a single application scope, so namespace +// filtering provides the correct event scope. +// For Tenant applications, events are fetched from status.namespace (the tenant's +// own namespace) instead of the parent namespace where the Tenant object lives. +func eventsTab(kind string) map[string]any { + nsPlaceholder := "{3}" + if kind == "Tenant" { + nsPlaceholder = "{reqsJsonPath[0]['.status.namespace']}" + } + return map[string]any{ + "key": "events", + "label": "Events", + "children": []any{ + map[string]any{ + "type": "EnrichedTable", + "data": map[string]any{ + "id": "events-table", + "fetchUrl": "/api/clusters/{2}/k8s/api/v1/namespaces/" + nsPlaceholder + "/events", + "cluster": "{2}", + "baseprefix": "/openapi-ui", + "customizationId": "factory-details-events", + "pathToItems": []any{"items"}, + }, + }, + }, + } +} + func yamlTab(group, version, plural string) map[string]any { return map[string]any{ "key": "yaml", diff --git a/internal/controller/dashboard/factory_test.go b/internal/controller/dashboard/factory_test.go new file mode 100644 index 00000000..b3742564 --- /dev/null +++ b/internal/controller/dashboard/factory_test.go @@ -0,0 +1,60 @@ +package dashboard + +import ( + "testing" +) + +func TestEventsTab_Structure(t *testing.T) { + tab := eventsTab("PostgreSQL") + + if tab["key"] != "events" { + t.Errorf("expected key=events, got %v", tab["key"]) + } + if tab["label"] != "Events" { + t.Errorf("expected label=Events, got %v", tab["label"]) + } + + children, ok := tab["children"].([]any) + if !ok || len(children) != 1 { + t.Fatal("expected exactly 1 child in events tab") + } + + table, ok := children[0].(map[string]any) + if !ok { + t.Fatal("child is not a map") + } + if table["type"] != "EnrichedTable" { + t.Errorf("expected type=EnrichedTable, got %v", table["type"]) + } + + data, ok := table["data"].(map[string]any) + if !ok { + t.Fatal("table data is not a map") + } + if data["id"] != "events-table" { + t.Errorf("expected id=events-table, got %v", data["id"]) + } + if data["fetchUrl"] != "/api/clusters/{2}/k8s/api/v1/namespaces/{3}/events" { + t.Errorf("unexpected fetchUrl for non-Tenant: %v", data["fetchUrl"]) + } + if data["customizationId"] != "factory-details-events" { + t.Errorf("expected customizationId=factory-details-events, got %v", data["customizationId"]) + } + + pathToItems, ok := data["pathToItems"].([]any) + if !ok || len(pathToItems) != 1 || pathToItems[0] != "items" { + t.Errorf("expected pathToItems=[items], got %v", data["pathToItems"]) + } +} + +func TestEventsTab_TenantUsesStatusNamespace(t *testing.T) { + tab := eventsTab("Tenant") + children := tab["children"].([]any) + table := children[0].(map[string]any) + data := table["data"].(map[string]any) + + expectedURL := "/api/clusters/{2}/k8s/api/v1/namespaces/{reqsJsonPath[0]['.status.namespace']}/events" + if data["fetchUrl"] != expectedURL { + t.Errorf("expected Tenant fetchUrl to use status.namespace, got %v", data["fetchUrl"]) + } +} diff --git a/internal/controller/dashboard/static_refactored.go b/internal/controller/dashboard/static_refactored.go index 06c4239f..a73026bc 100644 --- a/internal/controller/dashboard/static_refactored.go +++ b/internal/controller/dashboard/static_refactored.go @@ -224,6 +224,17 @@ func CreateAllCustomColumnsOverrides() []*dashboardv1alpha1.CustomColumnsOverrid createStringColumn("Name", ".name"), }), + // Factory details events + createCustomColumnsOverride("factory-details-events", []any{ + createTimestampColumn("Last Seen", ".lastTimestamp"), + createTimestampColumn("Event Time", ".eventTime"), + createStringColumn("Type", ".type"), + createStringColumn("Reason", ".reason"), + createStringColumn("Object", ".involvedObject.kind"), + createStringColumn("Name", ".involvedObject.name"), + createStringColumn("Message", ".message"), + }), + // Factory status conditions createCustomColumnsOverride("factory-status-conditions", []any{ createStringColumn("Type", ".type"), diff --git a/internal/controller/workloadmonitor_controller.go b/internal/controller/workloadmonitor_controller.go index a684df9b..0cd3103e 100644 --- a/internal/controller/workloadmonitor_controller.go +++ b/internal/controller/workloadmonitor_controller.go @@ -389,9 +389,9 @@ func (r *WorkloadMonitorReconciler) Reconcile(ctx context.Context, req ctrl.Requ fresh.Status.AvailableReplicas = availableReplicas // Default to operational = true, but check MinReplicas if set - monitor.Status.Operational = pointer.Bool(true) + fresh.Status.Operational = pointer.Bool(true) if monitor.Spec.MinReplicas != nil && availableReplicas < *monitor.Spec.MinReplicas { - monitor.Status.Operational = pointer.Bool(false) + fresh.Status.Operational = pointer.Bool(false) } return r.Status().Update(ctx, fresh) }) diff --git a/internal/controller/workloadmonitor_controller_test.go b/internal/controller/workloadmonitor_controller_test.go new file mode 100644 index 00000000..e1ed0c0d --- /dev/null +++ b/internal/controller/workloadmonitor_controller_test.go @@ -0,0 +1,180 @@ +package controller + +import ( + "context" + "testing" + + cozyv1alpha1 "github.com/cozystack/cozystack/api/v1alpha1" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/reconcile" +) + +func TestReconcile_OperationalStatusPersisted(t *testing.T) { + scheme := runtime.NewScheme() + _ = cozyv1alpha1.AddToScheme(scheme) + _ = corev1.AddToScheme(scheme) + + minReplicas := int32(2) + monitor := &cozyv1alpha1.WorkloadMonitor{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-monitor", + Namespace: "default", + }, + Spec: cozyv1alpha1.WorkloadMonitorSpec{ + Selector: map[string]string{"app": "test"}, + MinReplicas: &minReplicas, + }, + } + + // Create one pod that is ready — availableReplicas=1 < minReplicas=2, so Operational should be false + pod := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pod-1", + Namespace: "default", + Labels: map[string]string{"app": "test"}, + }, + Status: corev1.PodStatus{ + Conditions: []corev1.PodCondition{ + {Type: corev1.PodReady, Status: corev1.ConditionTrue}, + }, + }, + } + + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(monitor, pod). + WithStatusSubresource(monitor). + Build() + + reconciler := &WorkloadMonitorReconciler{Client: fakeClient, Scheme: scheme} + req := reconcile.Request{NamespacedName: types.NamespacedName{Name: "test-monitor", Namespace: "default"}} + + _, err := reconciler.Reconcile(context.TODO(), req) + if err != nil { + t.Fatalf("Reconcile returned error: %v", err) + } + + // Fetch the monitor back from fake client and check Operational is persisted + updated := &cozyv1alpha1.WorkloadMonitor{} + if err := fakeClient.Get(context.TODO(), req.NamespacedName, updated); err != nil { + t.Fatalf("Failed to get updated WorkloadMonitor: %v", err) + } + + if updated.Status.Operational == nil { + t.Fatal("Expected Operational to be set, got nil") + } + if *updated.Status.Operational { + t.Error("Expected Operational=false (1 available < 2 minReplicas), got true") + } + if updated.Status.ObservedReplicas != 1 { + t.Errorf("Expected ObservedReplicas=1, got %d", updated.Status.ObservedReplicas) + } + if updated.Status.AvailableReplicas != 1 { + t.Errorf("Expected AvailableReplicas=1, got %d", updated.Status.AvailableReplicas) + } +} + +func TestReconcile_OperationalTrue_WhenEnoughReplicas(t *testing.T) { + scheme := runtime.NewScheme() + _ = cozyv1alpha1.AddToScheme(scheme) + _ = corev1.AddToScheme(scheme) + + minReplicas := int32(1) + monitor := &cozyv1alpha1.WorkloadMonitor{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-monitor", + Namespace: "default", + }, + Spec: cozyv1alpha1.WorkloadMonitorSpec{ + Selector: map[string]string{"app": "test"}, + MinReplicas: &minReplicas, + }, + } + + pod := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pod-1", + Namespace: "default", + Labels: map[string]string{"app": "test"}, + }, + Status: corev1.PodStatus{ + Conditions: []corev1.PodCondition{ + {Type: corev1.PodReady, Status: corev1.ConditionTrue}, + }, + }, + } + + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(monitor, pod). + WithStatusSubresource(monitor). + Build() + + reconciler := &WorkloadMonitorReconciler{Client: fakeClient, Scheme: scheme} + req := reconcile.Request{NamespacedName: types.NamespacedName{Name: "test-monitor", Namespace: "default"}} + + _, err := reconciler.Reconcile(context.TODO(), req) + if err != nil { + t.Fatalf("Reconcile returned error: %v", err) + } + + updated := &cozyv1alpha1.WorkloadMonitor{} + if err := fakeClient.Get(context.TODO(), req.NamespacedName, updated); err != nil { + t.Fatalf("Failed to get updated WorkloadMonitor: %v", err) + } + + if updated.Status.Operational == nil { + t.Fatal("Expected Operational to be set, got nil") + } + if !*updated.Status.Operational { + t.Error("Expected Operational=true (1 available >= 1 minReplicas), got false") + } +} + +func TestReconcile_OperationalTrue_WhenNoMinReplicas(t *testing.T) { + scheme := runtime.NewScheme() + _ = cozyv1alpha1.AddToScheme(scheme) + _ = corev1.AddToScheme(scheme) + + monitor := &cozyv1alpha1.WorkloadMonitor{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-monitor", + Namespace: "default", + }, + Spec: cozyv1alpha1.WorkloadMonitorSpec{ + Selector: map[string]string{"app": "test"}, + // No MinReplicas — should default to operational=true + }, + } + + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(monitor). + WithStatusSubresource(monitor). + Build() + + reconciler := &WorkloadMonitorReconciler{Client: fakeClient, Scheme: scheme} + req := reconcile.Request{NamespacedName: types.NamespacedName{Name: "test-monitor", Namespace: "default"}} + + _, err := reconciler.Reconcile(context.TODO(), req) + if err != nil { + t.Fatalf("Reconcile returned error: %v", err) + } + + updated := &cozyv1alpha1.WorkloadMonitor{} + if err := fakeClient.Get(context.TODO(), req.NamespacedName, updated); err != nil { + t.Fatalf("Failed to get updated WorkloadMonitor: %v", err) + } + + if updated.Status.Operational == nil { + t.Fatal("Expected Operational to be set, got nil") + } + if !*updated.Status.Operational { + t.Error("Expected Operational=true (no MinReplicas constraint), got false") + } +} + diff --git a/pkg/apiserver/apiserver.go b/pkg/apiserver/apiserver.go index bf02e642..005fc640 100644 --- a/pkg/apiserver/apiserver.go +++ b/pkg/apiserver/apiserver.go @@ -21,6 +21,7 @@ import ( "fmt" "time" + cozyv1alpha1 "github.com/cozystack/cozystack/api/v1alpha1" helmv2 "github.com/fluxcd/helm-controller/api/v2" corev1 "k8s.io/api/core/v1" rbacv1 "k8s.io/api/rbac/v1" @@ -79,6 +80,11 @@ func init() { if err := rbacv1.AddToScheme(mgrScheme); err != nil { panic(fmt.Errorf("Failed to add RBAC types to scheme: %w", err)) } + + // Register Cozystack types for WorkloadMonitor queries. + if err := cozyv1alpha1.AddToScheme(mgrScheme); err != nil { + panic(fmt.Errorf("failed to add Cozystack types to scheme: %w", err)) + } // Add unversioned types. metav1.AddToGroupVersion(Scheme, schema.GroupVersion{Version: "v1"}) @@ -168,6 +174,7 @@ func (c completedConfig) New() (*CozyServer, error) { &corev1.Namespace{}, &corev1.Service{}, &rbacv1.RoleBinding{}, + &cozyv1alpha1.WorkloadMonitor{}, ); err != nil { return nil, fmt.Errorf("failed to get informers: %w", err) } diff --git a/pkg/registry/apps/application/rest.go b/pkg/registry/apps/application/rest.go index 470da560..583a4a8c 100644 --- a/pkg/registry/apps/application/rest.go +++ b/pkg/registry/apps/application/rest.go @@ -26,6 +26,7 @@ import ( "sync" "time" + cozyv1alpha1 "github.com/cozystack/cozystack/api/v1alpha1" helmv2 "github.com/fluxcd/helm-controller/api/v2" corev1 "k8s.io/api/core/v1" metainternalversion "k8s.io/apimachinery/pkg/apis/meta/internalversion" @@ -703,9 +704,28 @@ func (r *REST) Watch(ctx context.Context, options *metainternalversion.ListOptio } customW.underlying = helmWatcher + // Start watch on WorkloadMonitor to detect pod readiness changes + wmLabelSelector := labels.NewSelector().Add(*appKindReq, *appGroupReq) + wmList := &cozyv1alpha1.WorkloadMonitorList{} + wmWatcher, err := r.w.Watch(ctx, wmList, &client.ListOptions{ + Namespace: namespace, + LabelSelector: wmLabelSelector, + }) + if err != nil { + klog.Warningf("Failed to set up WorkloadMonitor watch, workload status changes won't trigger events: %v", err) + // Non-fatal: proceed without WorkloadMonitor watch + wmWatcher = nil + } + go func() { + // Capture wmWatcher for cleanup; the variable may be set to nil + // inside the loop when the channel closes, so defer must use this copy. + wmWatcherForCleanup := wmWatcher defer close(customW.resultChan) defer customW.underlying.Stop() + if wmWatcherForCleanup != nil { + defer wmWatcherForCleanup.Stop() + } // Track whether we've sent the initial-events-end bookmark initialEventsEndSent := !sendInitialEvents // If not sendInitialEvents, consider it already sent @@ -879,6 +899,77 @@ func (r *REST) Watch(ctx context.Context, options *metainternalversion.ListOptio return } + case wmEvent, ok := <-wmResultChan(wmWatcher): + if !ok { + klog.V(4).Info("WorkloadMonitor watcher closed") + wmWatcher = nil + continue + } + if wmEvent.Type == watch.Bookmark || wmEvent.Type == watch.Error { + if wmEvent.Type == watch.Error { + klog.V(4).Infof("WorkloadMonitor watch error event: %v", wmEvent.Object) + } + continue + } + // Don't emit WM-triggered events until the initial snapshot is + // complete — the watch-list contract requires all ADDED events + // followed by the initial-events-end bookmark before any live updates. + if !initialEventsEndSent { + continue + } + wm, ok := wmEvent.Object.(*cozyv1alpha1.WorkloadMonitor) + if !ok { + continue + } + // All WM event types (Added/Modified/Deleted) produce a Modified + // Application event because the Application itself is what changed + // from the client's perspective. + wmAppName, hasLabel := wm.Labels[ApplicationNameLabel] + if !hasLabel { + continue + } + // Filter: skip WorkloadMonitor events for applications not matching + // the watch scope (single-resource or field-selector filtered watches) + hrName := r.releaseConfig.Prefix + wmAppName + if filterByName != "" && hrName != filterByName { + continue + } + if resourceName != "" && wmAppName != resourceName { + continue + } + hr := &helmv2.HelmRelease{} + if err := r.c.Get(ctx, client.ObjectKey{Namespace: wm.Namespace, Name: hrName}, hr); err != nil { + klog.V(4).Infof("Cannot find HelmRelease %s/%s for WorkloadMonitor event: %v", wm.Namespace, hrName, err) + continue + } + app, err := r.ConvertHelmReleaseToApplication(ctx, hr) + if err != nil { + klog.V(4).Infof("Error converting HelmRelease for WorkloadMonitor event: %v", err) + continue + } + // Apply label selector filtering (same as HelmRelease event path) + if options.LabelSelector != nil { + sel, err := labels.Parse(options.LabelSelector.String()) + if err != nil { + klog.Errorf("Invalid label selector: %v", err) + continue + } + if !sel.Matches(labels.Set(app.Labels)) { + continue + } + } + // Use the WorkloadMonitor's ResourceVersion for the emitted event + // so clients see a monotonically increasing RV and don't skip this update. + app.SetResourceVersion(wm.GetResourceVersion()) + lastResourceVersion = wm.GetResourceVersion() + select { + case customW.resultChan <- watch.Event{Type: watch.Modified, Object: &app}: + case <-customW.stopChan: + return + case <-ctx.Done(): + return + } + case <-customW.stopChan: return case <-ctx.Done(): @@ -891,6 +982,15 @@ func (r *REST) Watch(ctx context.Context, options *metainternalversion.ListOptio return customW, nil } +// wmResultChan returns the result channel of a WorkloadMonitor watcher, or a nil +// channel (which blocks forever in select) if the watcher is nil. +func wmResultChan(w watch.Interface) <-chan watch.Event { + if w == nil { + return nil + } + return w.ResultChan() +} + // customWatcher wraps the original watcher and filters/converts events type customWatcher struct { resultChan chan watch.Event @@ -1150,6 +1250,56 @@ func (r *REST) convertHelmReleaseToApplication(ctx context.Context, hr *helmv2.H }) } } + // Enrich conditions with WorkloadMonitor operational status + ws, wsErr := r.getWorkloadsOperational(ctx, hr.Namespace, app.Name) + if wsErr != nil { + // Fail-open: if we can't query WorkloadMonitors (e.g., informer cache not ready), + // don't override Ready. Prefer operational availability over safety. + // The WorkloadsReady=Unknown condition still signals the issue to the user. + klog.Warningf("Failed to check workload monitors for %s/%s: %v", hr.Namespace, app.Name, wsErr) + conditions = append(conditions, metav1.Condition{ + Type: "WorkloadsReady", + Status: metav1.ConditionUnknown, + LastTransitionTime: metav1.Now(), + Reason: "Error", + Message: fmt.Sprintf("Failed to check workload status: %v", wsErr), + }) + } else if ws.found { + // LastTransitionTime is set to the current time because the Application + // resource is virtual (computed on-the-fly from HelmRelease). There is no + // persistent condition state to track actual transitions. This is consistent + // with how computed/virtual API resources work in Kubernetes. + workloadsCondition := metav1.Condition{ + Type: "WorkloadsReady", + LastTransitionTime: metav1.Now(), + Reason: "WorkloadMonitorCheck", + } + switch { + case !ws.operational: + // Concrete failure takes priority over unknown/pending state + workloadsCondition.Status = metav1.ConditionFalse + workloadsCondition.Message = "One or more workloads are not operational" + case ws.unknown: + workloadsCondition.Status = metav1.ConditionUnknown + workloadsCondition.Reason = "Pending" + workloadsCondition.Message = "One or more workloads have not been reconciled yet" + default: + workloadsCondition.Status = metav1.ConditionTrue + workloadsCondition.Message = "All workloads are operational" + } + conditions = append(conditions, workloadsCondition) + + // Intentionally do NOT override the Ready condition based on WorkloadsReady. + // Ready continues to reflect HelmRelease state only, which: + // - preserves backward compatibility with existing tooling (kubectl wait, + // GitOps health checks) that expect Ready to match HelmRelease + // - avoids false-negative Ready=False during normal startup windows where + // pods are still coming up but WorkloadMonitor has already reported + // Operational=false due to availableReplicas < MinReplicas + // WorkloadsReady is a separate condition that surfaces workload health + // independently — users and dashboards can observe it for operational visibility. + } + app.SetConditions(conditions) // Add namespace field for Tenant applications @@ -1166,6 +1316,42 @@ func (r *REST) convertHelmReleaseToApplication(ctx context.Context, hr *helmv2.H return app, nil } +// workloadsStatus holds the aggregated operational status of WorkloadMonitors. +type workloadsStatus struct { + operational bool + found bool + unknown bool // true when at least one monitor has nil Operational (not yet reconciled) +} + +// getWorkloadsOperational checks WorkloadMonitor resources for an application and returns +// aggregated operational status. If no monitors exist, returns found=false. +func (r *REST) getWorkloadsOperational(ctx context.Context, namespace, appName string) (workloadsStatus, error) { + monitors := &cozyv1alpha1.WorkloadMonitorList{} + if err := r.c.List(ctx, monitors, + client.InNamespace(namespace), + client.MatchingLabels{ + appsv1alpha1.ApplicationKindLabel: r.kindName, + appsv1alpha1.ApplicationGroupLabel: r.gvk.Group, + appsv1alpha1.ApplicationNameLabel: appName, + }, + ); err != nil { + return workloadsStatus{}, err + } + if len(monitors.Items) == 0 { + return workloadsStatus{operational: true, found: false}, nil + } + operational := true + unknown := false + for _, m := range monitors.Items { + if m.Status.Operational == nil { + unknown = true + } else if !*m.Status.Operational { + operational = false + } + } + return workloadsStatus{operational: operational, found: true, unknown: unknown}, nil +} + // convertApplicationToHelmRelease implements the actual conversion logic func (r *REST) convertApplicationToHelmRelease(app *appsv1alpha1.Application) (*helmv2.HelmRelease, error) { helmRelease := &helmv2.HelmRelease{ diff --git a/pkg/registry/apps/application/rest_conditions_test.go b/pkg/registry/apps/application/rest_conditions_test.go new file mode 100644 index 00000000..dbb99d52 --- /dev/null +++ b/pkg/registry/apps/application/rest_conditions_test.go @@ -0,0 +1,331 @@ +package application + +import ( + "context" + "testing" + + cozyv1alpha1 "github.com/cozystack/cozystack/api/v1alpha1" + helmv2 "github.com/fluxcd/helm-controller/api/v2" + appsv1alpha1 "github.com/cozystack/cozystack/pkg/apis/apps/v1alpha1" + "github.com/cozystack/cozystack/pkg/config" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/utils/ptr" + "sigs.k8s.io/controller-runtime/pkg/client/fake" +) + +func findCondition(conditions []metav1.Condition, condType string) *metav1.Condition { + for i := range conditions { + if conditions[i].Type == condType { + return &conditions[i] + } + } + return nil +} + +func makeHelmRelease(name, namespace string) *helmv2.HelmRelease { + hr := &helmv2.HelmRelease{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + Labels: map[string]string{ + ApplicationKindLabel: "PostgreSQL", + ApplicationGroupLabel: "apps.cozystack.io", + ApplicationNameLabel: "mydb", + }, + }, + } + return hr +} + +func TestConvertConditions_WorkloadsReadyAdded(t *testing.T) { + monitor := &cozyv1alpha1.WorkloadMonitor{ + ObjectMeta: metav1.ObjectMeta{ + Name: "mon-1", + Namespace: "default", + Labels: map[string]string{ + appsv1alpha1.ApplicationKindLabel: "PostgreSQL", + appsv1alpha1.ApplicationGroupLabel: "apps.cozystack.io", + appsv1alpha1.ApplicationNameLabel: "mydb", + }, + }, + Status: cozyv1alpha1.WorkloadMonitorStatus{ + Operational: ptr.To(true), + }, + } + + r := newTestRESTWithSchemes(monitor) + hr := makeHelmRelease("postgresql-mydb", "default") + hr.Status.Conditions = []metav1.Condition{ + {Type: "Ready", Status: metav1.ConditionTrue, Reason: "Succeeded", Message: "Release applied"}, + {Type: "Released", Status: metav1.ConditionTrue, Reason: "Succeeded", Message: "Released"}, + } + + app, err := r.convertHelmReleaseToApplication(context.TODO(), hr) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + wc := findCondition(app.GetConditions(), "WorkloadsReady") + if wc == nil { + t.Fatal("expected WorkloadsReady condition to be present") + } + if wc.Status != metav1.ConditionTrue { + t.Errorf("expected WorkloadsReady=True, got %s", wc.Status) + } + + rc := findCondition(app.GetConditions(), "Ready") + if rc == nil { + t.Fatal("expected Ready condition to be present") + } + if rc.Status != metav1.ConditionTrue { + t.Errorf("expected Ready=True, got %s", rc.Status) + } +} + +func TestConvertConditions_ReadyNotOverriddenWhenWorkloadsNotReady(t *testing.T) { + // Ready must reflect HelmRelease state only. WorkloadsReady is a separate + // signal that users/dashboards can observe independently. + monitor := &cozyv1alpha1.WorkloadMonitor{ + ObjectMeta: metav1.ObjectMeta{ + Name: "mon-1", + Namespace: "default", + Labels: map[string]string{ + appsv1alpha1.ApplicationKindLabel: "PostgreSQL", + appsv1alpha1.ApplicationGroupLabel: "apps.cozystack.io", + appsv1alpha1.ApplicationNameLabel: "mydb", + }, + }, + Status: cozyv1alpha1.WorkloadMonitorStatus{ + Operational: ptr.To(false), + }, + } + + r := newTestRESTWithSchemes(monitor) + hr := makeHelmRelease("postgresql-mydb", "default") + hr.Status.Conditions = []metav1.Condition{ + {Type: "Ready", Status: metav1.ConditionTrue, Reason: "Succeeded", Message: "Release applied"}, + } + + app, err := r.convertHelmReleaseToApplication(context.TODO(), hr) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + wc := findCondition(app.GetConditions(), "WorkloadsReady") + if wc == nil { + t.Fatal("expected WorkloadsReady condition to be present") + } + if wc.Status != metav1.ConditionFalse { + t.Errorf("expected WorkloadsReady=False, got %s", wc.Status) + } + + rc := findCondition(app.GetConditions(), "Ready") + if rc == nil { + t.Fatal("expected Ready condition to be present") + } + if rc.Status != metav1.ConditionTrue { + t.Errorf("expected Ready=True (reflects HelmRelease only), got %s", rc.Status) + } + if rc.Reason != "Succeeded" { + t.Errorf("expected Ready.Reason=Succeeded (unchanged), got %s", rc.Reason) + } +} + +func TestConvertConditions_NoOverrideWhenNoMonitors(t *testing.T) { + r := newTestRESTWithSchemes() + hr := makeHelmRelease("postgresql-mydb", "default") + hr.Status.Conditions = []metav1.Condition{ + {Type: "Ready", Status: metav1.ConditionTrue, Reason: "Succeeded", Message: "Release applied"}, + } + + app, err := r.convertHelmReleaseToApplication(context.TODO(), hr) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + wc := findCondition(app.GetConditions(), "WorkloadsReady") + if wc != nil { + t.Error("expected no WorkloadsReady condition when no monitors exist") + } + + rc := findCondition(app.GetConditions(), "Ready") + if rc == nil { + t.Fatal("expected Ready condition to be present") + } + if rc.Status != metav1.ConditionTrue { + t.Errorf("expected Ready=True unchanged, got %s", rc.Status) + } +} + +func TestConvertConditions_ReadyStaysTrue_WhenAllOperational(t *testing.T) { + monitor := &cozyv1alpha1.WorkloadMonitor{ + ObjectMeta: metav1.ObjectMeta{ + Name: "mon-1", + Namespace: "default", + Labels: map[string]string{ + appsv1alpha1.ApplicationKindLabel: "PostgreSQL", + appsv1alpha1.ApplicationGroupLabel: "apps.cozystack.io", + appsv1alpha1.ApplicationNameLabel: "mydb", + }, + }, + Status: cozyv1alpha1.WorkloadMonitorStatus{ + Operational: ptr.To(true), + }, + } + + r := newTestRESTWithSchemes(monitor) + hr := makeHelmRelease("postgresql-mydb", "default") + hr.Status.Conditions = []metav1.Condition{ + {Type: "Ready", Status: metav1.ConditionTrue, Reason: "Succeeded", Message: "Release applied"}, + {Type: "Released", Status: metav1.ConditionTrue, Reason: "Succeeded", Message: "Released"}, + } + + app, err := r.convertHelmReleaseToApplication(context.TODO(), hr) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + rc := findCondition(app.GetConditions(), "Ready") + if rc == nil { + t.Fatal("expected Ready condition") + } + if rc.Status != metav1.ConditionTrue { + t.Errorf("expected Ready=True when all workloads operational, got %s", rc.Status) + } + if rc.Reason != "Succeeded" { + t.Errorf("expected Ready.Reason=Succeeded (unchanged), got %s", rc.Reason) + } +} + +func TestConvertConditions_WorkloadsReadyTimestampIsNonZero(t *testing.T) { + monitor := &cozyv1alpha1.WorkloadMonitor{ + ObjectMeta: metav1.ObjectMeta{ + Name: "mon-1", + Namespace: "default", + Labels: map[string]string{ + appsv1alpha1.ApplicationKindLabel: "PostgreSQL", + appsv1alpha1.ApplicationGroupLabel: "apps.cozystack.io", + appsv1alpha1.ApplicationNameLabel: "mydb", + }, + }, + Status: cozyv1alpha1.WorkloadMonitorStatus{ + Operational: ptr.To(true), + }, + } + + r := newTestRESTWithSchemes(monitor) + hr := makeHelmRelease("postgresql-mydb", "default") + // No Ready condition — HR still being reconciled + hr.Status.Conditions = []metav1.Condition{} + + app, err := r.convertHelmReleaseToApplication(context.TODO(), hr) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + wc := findCondition(app.GetConditions(), "WorkloadsReady") + if wc == nil { + t.Fatal("expected WorkloadsReady condition") + } + if wc.LastTransitionTime.IsZero() { + t.Error("expected non-zero LastTransitionTime") + } +} + +func TestConvertConditions_WorkloadsReadyUnknownWhenNilOperational(t *testing.T) { + monitor := &cozyv1alpha1.WorkloadMonitor{ + ObjectMeta: metav1.ObjectMeta{ + Name: "mon-1", + Namespace: "default", + Labels: map[string]string{ + appsv1alpha1.ApplicationKindLabel: "PostgreSQL", + appsv1alpha1.ApplicationGroupLabel: "apps.cozystack.io", + appsv1alpha1.ApplicationNameLabel: "mydb", + }, + }, + Status: cozyv1alpha1.WorkloadMonitorStatus{ + Operational: nil, // Not yet reconciled + }, + } + + r := newTestRESTWithSchemes(monitor) + hr := makeHelmRelease("postgresql-mydb", "default") + hr.Status.Conditions = []metav1.Condition{ + {Type: "Ready", Status: metav1.ConditionTrue, Reason: "Succeeded", Message: "ok"}, + } + + app, err := r.convertHelmReleaseToApplication(context.TODO(), hr) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + wc := findCondition(app.GetConditions(), "WorkloadsReady") + if wc == nil { + t.Fatal("expected WorkloadsReady condition") + } + if wc.Status != metav1.ConditionUnknown { + t.Errorf("expected WorkloadsReady=Unknown for nil Operational, got %s", wc.Status) + } + + // Ready should NOT be overridden for unknown — prefer availability during startup + rc := findCondition(app.GetConditions(), "Ready") + if rc == nil { + t.Fatal("expected Ready condition") + } + if rc.Status != metav1.ConditionTrue { + t.Errorf("expected Ready=True when workloads unknown (pending), got %s", rc.Status) + } +} + +func TestConvertConditions_WorkloadsReadyUnknownOnError(t *testing.T) { + // Create a client with only HelmRelease scheme — WorkloadMonitor List will fail + scheme := runtime.NewScheme() + _ = helmv2.AddToScheme(scheme) + // Deliberately NOT registering cozyv1alpha1 so that List returns an error + + c := fake.NewClientBuilder().WithScheme(scheme).Build() + r := NewREST(c, nil, &config.Resource{ + Application: config.ApplicationConfig{ + Kind: "PostgreSQL", + Plural: "postgresqls", + Singular: "postgresql", + }, + Release: config.ReleaseConfig{ + Prefix: "postgresql-", + }, + }) + + hr := makeHelmRelease("postgresql-mydb", "default") + hr.CreationTimestamp = metav1.Now() + hr.Status.Conditions = []metav1.Condition{ + {Type: "Ready", Status: metav1.ConditionTrue, Reason: "Succeeded", Message: "ok"}, + } + + app, err := r.convertHelmReleaseToApplication(context.TODO(), hr) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + wc := findCondition(app.GetConditions(), "WorkloadsReady") + if wc == nil { + t.Fatal("expected WorkloadsReady condition with Unknown status on error") + } + if wc.Status != metav1.ConditionUnknown { + t.Errorf("expected WorkloadsReady=Unknown, got %s", wc.Status) + } + if wc.Reason != "Error" { + t.Errorf("expected reason=Error, got %s", wc.Reason) + } + + // Ready should NOT be overridden on error (fail-open: prefer availability) + rc := findCondition(app.GetConditions(), "Ready") + if rc == nil { + t.Fatal("expected Ready condition") + } + if rc.Status != metav1.ConditionTrue { + t.Errorf("expected Ready=True (fail-open on error), got %s", rc.Status) + } +} + + diff --git a/pkg/registry/apps/application/rest_watch_test.go b/pkg/registry/apps/application/rest_watch_test.go new file mode 100644 index 00000000..8fe2a04a --- /dev/null +++ b/pkg/registry/apps/application/rest_watch_test.go @@ -0,0 +1,223 @@ +package application + +import ( + "context" + "testing" + "time" + + cozyv1alpha1 "github.com/cozystack/cozystack/api/v1alpha1" + appsv1alpha1 "github.com/cozystack/cozystack/pkg/apis/apps/v1alpha1" + "github.com/cozystack/cozystack/pkg/config" + helmv2 "github.com/fluxcd/helm-controller/api/v2" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/watch" + "k8s.io/utils/ptr" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" +) + +func TestWmResultChan_NilWatcher(t *testing.T) { + ch := wmResultChan(nil) + if ch != nil { + t.Error("expected nil channel for nil watcher") + } +} + +func TestWmResultChan_ValidWatcher(t *testing.T) { + fw := watch.NewFake() + ch := wmResultChan(fw) + if ch == nil { + t.Error("expected non-nil channel for valid watcher") + } + fw.Stop() +} + +// TestWatchIntegration_WorkloadMonitorTriggersModifiedEvent verifies the +// full path: WM event → label lookup → HelmRelease Get → Application conversion. +func TestWatchIntegration_WorkloadMonitorTriggersModifiedEvent(t *testing.T) { + scheme := runtime.NewScheme() + _ = cozyv1alpha1.AddToScheme(scheme) + _ = helmv2.AddToScheme(scheme) + + hr := &helmv2.HelmRelease{ + ObjectMeta: metav1.ObjectMeta{ + Name: "postgresql-mydb", + Namespace: "default", + Labels: map[string]string{ + ApplicationKindLabel: "PostgreSQL", + ApplicationGroupLabel: "apps.cozystack.io", + ApplicationNameLabel: "mydb", + }, + }, + } + hr.Status.Conditions = []metav1.Condition{ + {Type: "Ready", Status: metav1.ConditionTrue, Reason: "Succeeded", Message: "ok"}, + } + + wm := &cozyv1alpha1.WorkloadMonitor{ + ObjectMeta: metav1.ObjectMeta{ + Name: "mon-1", + Namespace: "default", + Labels: map[string]string{ + appsv1alpha1.ApplicationKindLabel: "PostgreSQL", + appsv1alpha1.ApplicationGroupLabel: "apps.cozystack.io", + appsv1alpha1.ApplicationNameLabel: "mydb", + }, + }, + Status: cozyv1alpha1.WorkloadMonitorStatus{ + Operational: ptr.To(false), + }, + } + + c := fake.NewClientBuilder().WithScheme(scheme).WithRuntimeObjects(hr, wm).Build() + + r := newTestRESTWithSchemesFromClient(c) + + // Simulate the Watch goroutine path: extract app name from WM labels, + // construct HelmRelease name, look up HR, convert to Application + wmAppName := wm.Labels[ApplicationNameLabel] + hrName := r.releaseConfig.Prefix + wmAppName + + foundHR := &helmv2.HelmRelease{} + if err := c.Get(context.TODO(), types.NamespacedName{Namespace: "default", Name: hrName}, foundHR); err != nil { + t.Fatalf("failed to get HelmRelease: %v", err) + } + app, err := r.ConvertHelmReleaseToApplication(context.TODO(), foundHR) + if err != nil { + t.Fatalf("failed to convert: %v", err) + } + if app.Name != "mydb" { + t.Errorf("expected app name 'mydb', got %q", app.Name) + } + + // Verify WorkloadsReady is False due to non-operational WM + wc := findCondition(app.GetConditions(), "WorkloadsReady") + if wc == nil { + t.Fatal("expected WorkloadsReady condition") + } + if wc.Status != metav1.ConditionFalse { + t.Errorf("expected WorkloadsReady=False, got %s", wc.Status) + } +} + +// TestWatchIntegration_MonitorDeletionDropsWorkloadsReady verifies that when a +// WorkloadMonitor is deleted, the Application's WorkloadsReady condition +// disappears. Ready condition always reflects HelmRelease state regardless. +func TestWatchIntegration_MonitorDeletionDropsWorkloadsReady(t *testing.T) { + scheme := runtime.NewScheme() + _ = cozyv1alpha1.AddToScheme(scheme) + _ = helmv2.AddToScheme(scheme) + + hr := &helmv2.HelmRelease{ + ObjectMeta: metav1.ObjectMeta{ + Name: "postgresql-mydb", + Namespace: "default", + Labels: map[string]string{ + ApplicationKindLabel: "PostgreSQL", + ApplicationGroupLabel: "apps.cozystack.io", + ApplicationNameLabel: "mydb", + }, + }, + } + hr.Status.Conditions = []metav1.Condition{ + {Type: "Ready", Status: metav1.ConditionTrue, Reason: "Succeeded", Message: "ok"}, + } + + nonOpMonitor := &cozyv1alpha1.WorkloadMonitor{ + ObjectMeta: metav1.ObjectMeta{ + Name: "mon-1", + Namespace: "default", + Labels: map[string]string{ + appsv1alpha1.ApplicationKindLabel: "PostgreSQL", + appsv1alpha1.ApplicationGroupLabel: "apps.cozystack.io", + appsv1alpha1.ApplicationNameLabel: "mydb", + }, + }, + Status: cozyv1alpha1.WorkloadMonitorStatus{ + Operational: ptr.To(false), + }, + } + + c := fake.NewClientBuilder().WithScheme(scheme).WithRuntimeObjects(hr, nonOpMonitor).Build() + r := newTestRESTWithSchemesFromClient(c) + + // Step 1: With non-operational monitor, WorkloadsReady=False, Ready=True + app1, err := r.convertHelmReleaseToApplication(context.TODO(), hr) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + wc1 := findCondition(app1.GetConditions(), "WorkloadsReady") + if wc1 == nil || wc1.Status != metav1.ConditionFalse { + t.Fatalf("expected WorkloadsReady=False with non-operational monitor, got %v", wc1) + } + rc1 := findCondition(app1.GetConditions(), "Ready") + if rc1 == nil || rc1.Status != metav1.ConditionTrue { + t.Fatalf("expected Ready=True (reflects HelmRelease), got %v", rc1) + } + + // Step 2: Delete the monitor + if err := c.Delete(context.TODO(), nonOpMonitor); err != nil { + t.Fatalf("failed to delete monitor: %v", err) + } + + // Step 3: WorkloadsReady should disappear, Ready stays True + app2, err := r.convertHelmReleaseToApplication(context.TODO(), hr) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + wc2 := findCondition(app2.GetConditions(), "WorkloadsReady") + if wc2 != nil { + t.Error("expected no WorkloadsReady condition after monitor deletion") + } + rc2 := findCondition(app2.GetConditions(), "Ready") + if rc2 == nil { + t.Fatal("expected Ready condition") + } + if rc2.Status != metav1.ConditionTrue { + t.Errorf("expected Ready=True after monitor deletion, got %s", rc2.Status) + } +} + +// TestWatchIntegration_WMWatcherCloseProducesNilChannel verifies that +// after wmWatcher is set to nil, wmResultChan returns nil channel. +func TestWatchIntegration_WMWatcherCloseProducesNilChannel(t *testing.T) { + fw := watch.NewFake() + ch := wmResultChan(fw) + if ch == nil { + t.Fatal("expected non-nil channel before close") + } + + fw.Stop() + + timeout := time.After(time.Second) + for { + select { + case _, ok := <-ch: + if !ok { + var nilWatcher watch.Interface + nilCh := wmResultChan(nilWatcher) + if nilCh != nil { + t.Error("expected nil channel after watcher set to nil") + } + return + } + case <-timeout: + t.Fatal("timeout waiting for watcher channel to close") + } + } +} + +func newTestRESTWithSchemesFromClient(c client.Client) *REST { + return NewREST(c, nil, &config.Resource{ + Application: config.ApplicationConfig{ + Kind: "PostgreSQL", + Plural: "postgresqls", + Singular: "postgresql", + }, + Release: config.ReleaseConfig{ + Prefix: "postgresql-", + }, + }) +} diff --git a/pkg/registry/apps/application/rest_workloads_test.go b/pkg/registry/apps/application/rest_workloads_test.go new file mode 100644 index 00000000..30dd85c6 --- /dev/null +++ b/pkg/registry/apps/application/rest_workloads_test.go @@ -0,0 +1,342 @@ +package application + +import ( + "context" + "testing" + + cozyv1alpha1 "github.com/cozystack/cozystack/api/v1alpha1" + appsv1alpha1 "github.com/cozystack/cozystack/pkg/apis/apps/v1alpha1" + "github.com/cozystack/cozystack/pkg/config" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/utils/ptr" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + + helmv2 "github.com/fluxcd/helm-controller/api/v2" +) + +func newTestRESTWithSchemes(objs ...runtime.Object) *REST { + scheme := runtime.NewScheme() + _ = cozyv1alpha1.AddToScheme(scheme) + _ = helmv2.AddToScheme(scheme) + + builder := fake.NewClientBuilder().WithScheme(scheme) + for _, obj := range objs { + builder = builder.WithRuntimeObjects(obj) + } + c := builder.Build() + + return NewREST(c, nil, &config.Resource{ + Application: config.ApplicationConfig{ + Kind: "PostgreSQL", + Plural: "postgresqls", + Singular: "postgresql", + }, + Release: config.ReleaseConfig{ + Prefix: "postgresql-", + }, + }) +} + +func TestGetWorkloadsOperational_NoMonitors(t *testing.T) { + r := newTestRESTWithSchemes() + ws, err := r.getWorkloadsOperational(context.TODO(), "default", "mydb") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if ws.found { + t.Error("expected found=false when no monitors exist") + } + if !ws.operational { + t.Error("expected operational=true when no monitors exist") + } +} + +func TestGetWorkloadsOperational_AllOperational(t *testing.T) { + m1 := &cozyv1alpha1.WorkloadMonitor{ + ObjectMeta: metav1.ObjectMeta{ + Name: "mon-1", + Namespace: "default", + Labels: map[string]string{ + appsv1alpha1.ApplicationKindLabel: "PostgreSQL", + appsv1alpha1.ApplicationGroupLabel: "apps.cozystack.io", + appsv1alpha1.ApplicationNameLabel: "mydb", + }, + }, + Status: cozyv1alpha1.WorkloadMonitorStatus{ + Operational: ptr.To(true), + }, + } + m2 := &cozyv1alpha1.WorkloadMonitor{ + ObjectMeta: metav1.ObjectMeta{ + Name: "mon-2", + Namespace: "default", + Labels: map[string]string{ + appsv1alpha1.ApplicationKindLabel: "PostgreSQL", + appsv1alpha1.ApplicationGroupLabel: "apps.cozystack.io", + appsv1alpha1.ApplicationNameLabel: "mydb", + }, + }, + Status: cozyv1alpha1.WorkloadMonitorStatus{ + Operational: ptr.To(true), + }, + } + + r := newTestRESTWithSchemes(m1, m2) + ws, err := r.getWorkloadsOperational(context.TODO(), "default", "mydb") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !ws.found { + t.Error("expected found=true") + } + if !ws.operational { + t.Error("expected operational=true when all monitors are operational") + } +} + +func TestGetWorkloadsOperational_SomeNotOperational(t *testing.T) { + m1 := &cozyv1alpha1.WorkloadMonitor{ + ObjectMeta: metav1.ObjectMeta{ + Name: "mon-1", + Namespace: "default", + Labels: map[string]string{ + appsv1alpha1.ApplicationKindLabel: "PostgreSQL", + appsv1alpha1.ApplicationGroupLabel: "apps.cozystack.io", + appsv1alpha1.ApplicationNameLabel: "mydb", + }, + }, + Status: cozyv1alpha1.WorkloadMonitorStatus{ + Operational: ptr.To(true), + }, + } + m2 := &cozyv1alpha1.WorkloadMonitor{ + ObjectMeta: metav1.ObjectMeta{ + Name: "mon-2", + Namespace: "default", + Labels: map[string]string{ + appsv1alpha1.ApplicationKindLabel: "PostgreSQL", + appsv1alpha1.ApplicationGroupLabel: "apps.cozystack.io", + appsv1alpha1.ApplicationNameLabel: "mydb", + }, + }, + Status: cozyv1alpha1.WorkloadMonitorStatus{ + Operational: ptr.To(false), + }, + } + + r := newTestRESTWithSchemes(m1, m2) + ws, err := r.getWorkloadsOperational(context.TODO(), "default", "mydb") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !ws.found { + t.Error("expected found=true") + } + if ws.operational { + t.Error("expected operational=false when at least one monitor is not operational") + } +} + +func TestGetWorkloadsOperational_OperationalNil(t *testing.T) { + m := &cozyv1alpha1.WorkloadMonitor{ + ObjectMeta: metav1.ObjectMeta{ + Name: "mon-1", + Namespace: "default", + Labels: map[string]string{ + appsv1alpha1.ApplicationKindLabel: "PostgreSQL", + appsv1alpha1.ApplicationGroupLabel: "apps.cozystack.io", + appsv1alpha1.ApplicationNameLabel: "mydb", + }, + }, + Status: cozyv1alpha1.WorkloadMonitorStatus{ + Operational: nil, // Not yet reconciled + }, + } + + r := newTestRESTWithSchemes(m) + ws, err := r.getWorkloadsOperational(context.TODO(), "default", "mydb") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !ws.found { + t.Error("expected found=true") + } + if !ws.unknown { + t.Error("expected unknown=true when Operational is nil") + } +} + +func TestGetWorkloadsOperational_MixedNilAndOperational(t *testing.T) { + m1 := &cozyv1alpha1.WorkloadMonitor{ + ObjectMeta: metav1.ObjectMeta{ + Name: "mon-1", + Namespace: "default", + Labels: map[string]string{ + appsv1alpha1.ApplicationKindLabel: "PostgreSQL", + appsv1alpha1.ApplicationGroupLabel: "apps.cozystack.io", + appsv1alpha1.ApplicationNameLabel: "mydb", + }, + }, + Status: cozyv1alpha1.WorkloadMonitorStatus{ + Operational: ptr.To(true), + }, + } + m2 := &cozyv1alpha1.WorkloadMonitor{ + ObjectMeta: metav1.ObjectMeta{ + Name: "mon-2", + Namespace: "default", + Labels: map[string]string{ + appsv1alpha1.ApplicationKindLabel: "PostgreSQL", + appsv1alpha1.ApplicationGroupLabel: "apps.cozystack.io", + appsv1alpha1.ApplicationNameLabel: "mydb", + }, + }, + Status: cozyv1alpha1.WorkloadMonitorStatus{ + Operational: nil, + }, + } + + r := newTestRESTWithSchemes(m1, m2) + ws, err := r.getWorkloadsOperational(context.TODO(), "default", "mydb") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !ws.unknown { + t.Error("expected unknown=true when at least one monitor has nil Operational") + } +} + +func TestGetWorkloadsOperational_MixedFailedAndPending(t *testing.T) { + m1 := &cozyv1alpha1.WorkloadMonitor{ + ObjectMeta: metav1.ObjectMeta{ + Name: "mon-1", + Namespace: "default", + Labels: map[string]string{ + appsv1alpha1.ApplicationKindLabel: "PostgreSQL", + appsv1alpha1.ApplicationGroupLabel: "apps.cozystack.io", + appsv1alpha1.ApplicationNameLabel: "mydb", + }, + }, + Status: cozyv1alpha1.WorkloadMonitorStatus{ + Operational: ptr.To(false), // Confirmed failure + }, + } + m2 := &cozyv1alpha1.WorkloadMonitor{ + ObjectMeta: metav1.ObjectMeta{ + Name: "mon-2", + Namespace: "default", + Labels: map[string]string{ + appsv1alpha1.ApplicationKindLabel: "PostgreSQL", + appsv1alpha1.ApplicationGroupLabel: "apps.cozystack.io", + appsv1alpha1.ApplicationNameLabel: "mydb", + }, + }, + Status: cozyv1alpha1.WorkloadMonitorStatus{ + Operational: nil, // Not yet reconciled + }, + } + + r := newTestRESTWithSchemes(m1, m2) + ws, err := r.getWorkloadsOperational(context.TODO(), "default", "mydb") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if ws.operational { + t.Error("expected operational=false when at least one monitor is explicitly failed") + } + if !ws.unknown { + t.Error("expected unknown=true when at least one monitor has nil Operational") + } +} + +func TestConvertConditions_MixedFailedAndPendingShowsFalse(t *testing.T) { + mFailed := &cozyv1alpha1.WorkloadMonitor{ + ObjectMeta: metav1.ObjectMeta{ + Name: "mon-failed", + Namespace: "default", + Labels: map[string]string{ + appsv1alpha1.ApplicationKindLabel: "PostgreSQL", + appsv1alpha1.ApplicationGroupLabel: "apps.cozystack.io", + appsv1alpha1.ApplicationNameLabel: "mydb", + }, + }, + Status: cozyv1alpha1.WorkloadMonitorStatus{ + Operational: ptr.To(false), + }, + } + mPending := &cozyv1alpha1.WorkloadMonitor{ + ObjectMeta: metav1.ObjectMeta{ + Name: "mon-pending", + Namespace: "default", + Labels: map[string]string{ + appsv1alpha1.ApplicationKindLabel: "PostgreSQL", + appsv1alpha1.ApplicationGroupLabel: "apps.cozystack.io", + appsv1alpha1.ApplicationNameLabel: "mydb", + }, + }, + Status: cozyv1alpha1.WorkloadMonitorStatus{ + Operational: nil, + }, + } + + r := newTestRESTWithSchemes(mFailed, mPending) + hr := &helmv2.HelmRelease{ + ObjectMeta: metav1.ObjectMeta{ + Name: "postgresql-mydb", + Namespace: "default", + Labels: map[string]string{ + ApplicationKindLabel: "PostgreSQL", + ApplicationGroupLabel: "apps.cozystack.io", + ApplicationNameLabel: "mydb", + }, + }, + } + hr.Status.Conditions = []metav1.Condition{ + {Type: "Ready", Status: metav1.ConditionTrue, Reason: "Succeeded", Message: "ok"}, + } + + app, err := r.convertHelmReleaseToApplication(context.TODO(), hr) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + // Concrete failure should take priority over unknown + wc := findCondition(app.GetConditions(), "WorkloadsReady") + if wc == nil { + t.Fatal("expected WorkloadsReady condition") + } + if wc.Status != metav1.ConditionFalse { + t.Errorf("expected WorkloadsReady=False (failure takes priority over unknown), got %s", wc.Status) + } +} + +func TestGetWorkloadsOperational_DifferentApp_NotFound(t *testing.T) { + m := &cozyv1alpha1.WorkloadMonitor{ + ObjectMeta: metav1.ObjectMeta{ + Name: "mon-1", + Namespace: "default", + Labels: map[string]string{ + appsv1alpha1.ApplicationKindLabel: "PostgreSQL", + appsv1alpha1.ApplicationGroupLabel: "apps.cozystack.io", + appsv1alpha1.ApplicationNameLabel: "other-db", + }, + }, + Status: cozyv1alpha1.WorkloadMonitorStatus{ + Operational: ptr.To(false), + }, + } + + r := newTestRESTWithSchemes(m) + ws, err := r.getWorkloadsOperational(context.TODO(), "default", "mydb") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if ws.found { + t.Error("expected found=false for different app name") + } + if !ws.operational { + t.Error("expected operational=true when no matching monitors found") + } +} + From dc3387c635eef5ee795c31a0909910eed052dd5f Mon Sep 17 00:00:00 2001 From: Aleksei Sviridkin Date: Mon, 13 Apr 2026 22:27:01 +0300 Subject: [PATCH 2/4] fix(workload-monitor): use fresh spec for MinReplicas check on retry Inside the RetryOnConflict block, derive the operational status from fresh.Spec.MinReplicas instead of the stale monitor.Spec.MinReplicas so that concurrent spec updates observed by the retry are respected. Assisted-By: Claude Signed-off-by: Aleksei Sviridkin --- internal/controller/workloadmonitor_controller.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/internal/controller/workloadmonitor_controller.go b/internal/controller/workloadmonitor_controller.go index 0cd3103e..9a967e65 100644 --- a/internal/controller/workloadmonitor_controller.go +++ b/internal/controller/workloadmonitor_controller.go @@ -388,9 +388,11 @@ func (r *WorkloadMonitorReconciler) Reconcile(ctx context.Context, req ctrl.Requ fresh.Status.ObservedReplicas = observedReplicas fresh.Status.AvailableReplicas = availableReplicas - // Default to operational = true, but check MinReplicas if set + // Default to operational = true, but check MinReplicas if set. + // Use fresh.Spec to avoid making decisions based on a stale cached copy + // when the spec was updated between the initial read and this retry. fresh.Status.Operational = pointer.Bool(true) - if monitor.Spec.MinReplicas != nil && availableReplicas < *monitor.Spec.MinReplicas { + if fresh.Spec.MinReplicas != nil && availableReplicas < *fresh.Spec.MinReplicas { fresh.Status.Operational = pointer.Bool(false) } return r.Status().Update(ctx, fresh) From d369b64d718beee24ff4dc57582af5710c05d1d5 Mon Sep 17 00:00:00 2001 From: Aleksei Sviridkin Date: Mon, 13 Apr 2026 22:42:11 +0300 Subject: [PATCH 3/4] fix(application): address review feedback for WorkloadMonitor integration Tenant applications live in a child namespace computed from the Tenant name, not in the HelmRelease's namespace. Scope the WorkloadMonitor watch cluster-wide when kindName is Tenant and reverse-map WM events back to the owning HelmRelease via computeTenantNamespace. The condition-enrichment path now looks up monitors in the computed child namespace too, so WorkloadsReady is populated for Tenants. Buffer WorkloadMonitor events that arrive before the initial-events-end bookmark and replay them once the bookmark is emitted, to preserve the watch-list contract while not dropping workload-state transitions that happen during the snapshot window. Pass the fresh WorkloadMonitor object from the watch event into the conversion path so WorkloadsReady reflects the state that triggered the event, even when the cache client is lagging behind the watch client. Derive WorkloadsReady.LastTransitionTime from a stable source (max of HelmRelease creation and condition timestamps, plus monitor timestamps) instead of metav1.Now(), so repeated conversions of the same underlying state produce identical timestamps. Assisted-By: Claude Signed-off-by: Aleksei Sviridkin --- pkg/registry/apps/application/rest.go | 196 +++++++++++++++--- .../apps/application/rest_conditions_test.go | 14 +- .../apps/application/rest_validation_test.go | 4 +- .../apps/application/rest_watch_test.go | 4 +- .../apps/application/rest_workloads_test.go | 16 +- 5 files changed, 184 insertions(+), 50 deletions(-) diff --git a/pkg/registry/apps/application/rest.go b/pkg/registry/apps/application/rest.go index 583a4a8c..0728ea13 100644 --- a/pkg/registry/apps/application/rest.go +++ b/pkg/registry/apps/application/rest.go @@ -704,13 +704,19 @@ func (r *REST) Watch(ctx context.Context, options *metainternalversion.ListOptio } customW.underlying = helmWatcher - // Start watch on WorkloadMonitor to detect pod readiness changes + // Start watch on WorkloadMonitor to detect pod readiness changes. + // For Tenant applications the WorkloadMonitor lives in a computed child + // namespace (see computeTenantNamespace), not in the HelmRelease namespace, + // so scoping the watch to `namespace` would miss all events. Use a + // cluster-wide watch in that case — label selectors still restrict the + // stream to the relevant kind/group. wmLabelSelector := labels.NewSelector().Add(*appKindReq, *appGroupReq) wmList := &cozyv1alpha1.WorkloadMonitorList{} - wmWatcher, err := r.w.Watch(ctx, wmList, &client.ListOptions{ - Namespace: namespace, - LabelSelector: wmLabelSelector, - }) + wmListOpts := &client.ListOptions{LabelSelector: wmLabelSelector} + if r.kindName != "Tenant" { + wmListOpts.Namespace = namespace + } + wmWatcher, err := r.w.Watch(ctx, wmList, wmListOpts) if err != nil { klog.Warningf("Failed to set up WorkloadMonitor watch, workload status changes won't trigger events: %v", err) // Non-fatal: proceed without WorkloadMonitor watch @@ -731,6 +737,16 @@ func (r *REST) Watch(ctx context.Context, options *metainternalversion.ListOptio initialEventsEndSent := !sendInitialEvents // If not sendInitialEvents, consider it already sent var lastResourceVersion string + // Buffer of WorkloadMonitor events that arrived before the initial + // snapshot finished. The watch-list contract requires the stream to + // deliver all ADDED events followed by the initial-events-end bookmark + // before any live updates, so we hold WM-triggered Modified events and + // replay them once the bookmark has been emitted. Without this, a + // workload whose status flips during the snapshot window (after the + // Application ADDED but before the bookmark) and then stops changing + // would leave the client with a stale WorkloadsReady forever. + var pendingWMEvents []watch.Event + // Get the starting resourceVersion from options // If client provides resourceVersion (e.g., from a previous List), we should skip // objects with resourceVersion <= startingRV (client already has them) @@ -741,6 +757,19 @@ func (r *REST) Watch(ctx context.Context, options *metainternalversion.ListOptio } } + drainPendingWMEvents := func() { + for _, ev := range pendingWMEvents { + select { + case customW.resultChan <- ev: + case <-customW.stopChan: + return + case <-ctx.Done(): + return + } + } + pendingWMEvents = nil + } + // Helper function to send initial-events-end bookmark sendInitialEventsEndBookmark := func() { if initialEventsEndSent { @@ -765,8 +794,11 @@ func (r *REST) Watch(ctx context.Context, options *metainternalversion.ListOptio select { case customW.resultChan <- bookmarkEvent: case <-customW.stopChan: + return case <-ctx.Done(): + return } + drainPendingWMEvents() } // Process watch events @@ -794,8 +826,10 @@ func (r *REST) Watch(ctx context.Context, options *metainternalversion.ListOptio APIVersion: appsv1alpha1.SchemeGroupVersion.String(), Kind: r.kindName, } + justFlipped := false if !initialEventsEndSent { initialEventsEndSent = true + justFlipped = true bookmarkApp.SetAnnotations(map[string]string{ "k8s.io/initial-events-end": "true", }) @@ -812,6 +846,9 @@ func (r *REST) Watch(ctx context.Context, options *metainternalversion.ListOptio case <-ctx.Done(): return } + if justFlipped { + drainPendingWMEvents() + } } continue } @@ -911,12 +948,6 @@ func (r *REST) Watch(ctx context.Context, options *metainternalversion.ListOptio } continue } - // Don't emit WM-triggered events until the initial snapshot is - // complete — the watch-list contract requires all ADDED events - // followed by the initial-events-end bookmark before any live updates. - if !initialEventsEndSent { - continue - } wm, ok := wmEvent.Object.(*cozyv1alpha1.WorkloadMonitor) if !ok { continue @@ -937,12 +968,37 @@ func (r *REST) Watch(ctx context.Context, options *metainternalversion.ListOptio if resourceName != "" && wmAppName != resourceName { continue } - hr := &helmv2.HelmRelease{} - if err := r.c.Get(ctx, client.ObjectKey{Namespace: wm.Namespace, Name: hrName}, hr); err != nil { - klog.V(4).Infof("Cannot find HelmRelease %s/%s for WorkloadMonitor event: %v", wm.Namespace, hrName, err) + + // Locate the owning HelmRelease. For most application kinds the + // WorkloadMonitor and its HelmRelease live in the same namespace, + // but Tenant workloads live in a child namespace (see + // computeTenantNamespace) while the HelmRelease remains in the + // parent/requested namespace — so the WM-to-HR namespace mapping + // differs. + hrNS := wm.Namespace + if r.kindName == "Tenant" { + hrNS = namespace + // Filter out WM events whose child namespace does not + // correspond to the Tenant in our watched namespace, since + // the cluster-wide WM watch delivers events for all tenants. + if r.computeTenantNamespace(namespace, wmAppName) != wm.Namespace { + continue + } + if !fieldFilter.MatchesNamespace(hrNS) { + continue + } + } else if !fieldFilter.MatchesNamespace(hrNS) { continue } - app, err := r.ConvertHelmReleaseToApplication(ctx, hr) + hr := &helmv2.HelmRelease{} + if err := r.c.Get(ctx, client.ObjectKey{Namespace: hrNS, Name: hrName}, hr); err != nil { + klog.V(4).Infof("Cannot find HelmRelease %s/%s for WorkloadMonitor event: %v", hrNS, hrName, err) + continue + } + // Pass the fresh WorkloadMonitor so conversion uses the latest + // operational status even if the cache (r.c) has not yet + // observed this watch event. + app, err := r.ConvertHelmReleaseToApplicationWithMonitor(ctx, hr, wm) if err != nil { klog.V(4).Infof("Error converting HelmRelease for WorkloadMonitor event: %v", err) continue @@ -961,9 +1017,17 @@ func (r *REST) Watch(ctx context.Context, options *metainternalversion.ListOptio // Use the WorkloadMonitor's ResourceVersion for the emitted event // so clients see a monotonically increasing RV and don't skip this update. app.SetResourceVersion(wm.GetResourceVersion()) + outEvent := watch.Event{Type: watch.Modified, Object: &app} + // Buffer WM-triggered events that arrive before the + // initial-events-end bookmark. They will be replayed in order + // immediately after the bookmark is emitted. + if !initialEventsEndSent { + pendingWMEvents = append(pendingWMEvents, outEvent) + continue + } lastResourceVersion = wm.GetResourceVersion() select { - case customW.resultChan <- watch.Event{Type: watch.Modified, Object: &app}: + case customW.resultChan <- outEvent: case <-customW.stopChan: return case <-ctx.Done(): @@ -1094,12 +1158,21 @@ func filterPrefixedMap(original map[string]string, prefix string) map[string]str return processed } -// ConvertHelmReleaseToApplication converts a HelmRelease to an Application +// ConvertHelmReleaseToApplication converts a HelmRelease to an Application. func (r *REST) ConvertHelmReleaseToApplication(ctx context.Context, hr *helmv2.HelmRelease) (appsv1alpha1.Application, error) { + return r.ConvertHelmReleaseToApplicationWithMonitor(ctx, hr, nil) +} + +// ConvertHelmReleaseToApplicationWithMonitor converts a HelmRelease to an +// Application, optionally overriding the cached copy of a WorkloadMonitor with +// a fresher version received from the watch client. This prevents the emitted +// Application object from carrying stale WorkloadsReady data when r.c (cache) +// lags behind r.w (watch). +func (r *REST) ConvertHelmReleaseToApplicationWithMonitor(ctx context.Context, hr *helmv2.HelmRelease, freshMonitor *cozyv1alpha1.WorkloadMonitor) (appsv1alpha1.Application, error) { klog.V(6).Infof("Converting HelmRelease to Application for resource %s", hr.GetName()) // Convert HelmRelease struct to Application struct - app, err := r.convertHelmReleaseToApplication(ctx, hr) + app, err := r.convertHelmReleaseToApplication(ctx, hr, freshMonitor) if err != nil { klog.Errorf("Error converting from HelmRelease to Application: %v", err) return appsv1alpha1.Application{}, err @@ -1212,8 +1285,11 @@ func (r *REST) validateTenantNamespaceLength(currentNamespace, tenantName string return allErrs } -// convertHelmReleaseToApplication implements the actual conversion logic -func (r *REST) convertHelmReleaseToApplication(ctx context.Context, hr *helmv2.HelmRelease) (appsv1alpha1.Application, error) { +// convertHelmReleaseToApplication implements the actual conversion logic. +// The optional freshMonitor is used to override the cache copy of a +// WorkloadMonitor when a newer version was delivered via the watch client — +// see ConvertHelmReleaseToApplicationWithMonitor for the rationale. +func (r *REST) convertHelmReleaseToApplication(ctx context.Context, hr *helmv2.HelmRelease, freshMonitor *cozyv1alpha1.WorkloadMonitor) (appsv1alpha1.Application, error) { // Filter out internal keys (starting with "_") from spec filteredSpec := filterInternalKeys(hr.Spec.Values) @@ -1250,28 +1326,49 @@ func (r *REST) convertHelmReleaseToApplication(ctx context.Context, hr *helmv2.H }) } } - // Enrich conditions with WorkloadMonitor operational status - ws, wsErr := r.getWorkloadsOperational(ctx, hr.Namespace, app.Name) + // Enrich conditions with WorkloadMonitor operational status. + // Tenant workloads live in a child namespace (computed from the Tenant name), + // not in the same namespace as the owning HelmRelease — look there instead. + workloadsNS := hr.Namespace + if r.kindName == "Tenant" { + workloadsNS = r.computeTenantNamespace(hr.Namespace, app.Name) + } + ws, wsErr := r.getWorkloadsOperational(ctx, workloadsNS, app.Name, freshMonitor) + // Derive a stable LastTransitionTime: use the owning HelmRelease's own + // condition update time (or CreationTimestamp as a floor) so that repeated + // conversions of the same underlying state produce identical timestamps. + wrTransition := hr.CreationTimestamp + for _, c := range hr.GetConditions() { + if c.LastTransitionTime.After(wrTransition.Time) { + wrTransition = c.LastTransitionTime + } + } + if ws.transitionTime.After(wrTransition.Time) { + wrTransition = ws.transitionTime + } + if wrTransition.IsZero() { + // Fallback for objects that somehow have no timestamps at all + // (e.g. hand-crafted test fixtures). In production HelmReleases + // always carry a CreationTimestamp, so the stable branch above + // is used. + wrTransition = metav1.Now() + } if wsErr != nil { // Fail-open: if we can't query WorkloadMonitors (e.g., informer cache not ready), // don't override Ready. Prefer operational availability over safety. // The WorkloadsReady=Unknown condition still signals the issue to the user. - klog.Warningf("Failed to check workload monitors for %s/%s: %v", hr.Namespace, app.Name, wsErr) + klog.Warningf("Failed to check workload monitors for %s/%s: %v", workloadsNS, app.Name, wsErr) conditions = append(conditions, metav1.Condition{ Type: "WorkloadsReady", Status: metav1.ConditionUnknown, - LastTransitionTime: metav1.Now(), + LastTransitionTime: wrTransition, Reason: "Error", Message: fmt.Sprintf("Failed to check workload status: %v", wsErr), }) } else if ws.found { - // LastTransitionTime is set to the current time because the Application - // resource is virtual (computed on-the-fly from HelmRelease). There is no - // persistent condition state to track actual transitions. This is consistent - // with how computed/virtual API resources work in Kubernetes. workloadsCondition := metav1.Condition{ Type: "WorkloadsReady", - LastTransitionTime: metav1.Now(), + LastTransitionTime: wrTransition, Reason: "WorkloadMonitorCheck", } switch { @@ -1321,11 +1418,20 @@ type workloadsStatus struct { operational bool found bool unknown bool // true when at least one monitor has nil Operational (not yet reconciled) + // transitionTime is the most recent metadata update time across the + // matching monitors. Used as WorkloadsReady.LastTransitionTime so that + // repeated conversions for the same underlying state produce stable + // timestamps (preserving the Kubernetes contract that identical + // resource versions represent identical content). + transitionTime metav1.Time } // getWorkloadsOperational checks WorkloadMonitor resources for an application and returns // aggregated operational status. If no monitors exist, returns found=false. -func (r *REST) getWorkloadsOperational(ctx context.Context, namespace, appName string) (workloadsStatus, error) { +// When freshOverride is non-nil, its status replaces the cached copy for the +// corresponding monitor — this keeps the result consistent with watch events +// when the cache (r.c) lags behind the watch client (r.w). +func (r *REST) getWorkloadsOperational(ctx context.Context, namespace, appName string, freshOverride *cozyv1alpha1.WorkloadMonitor) (workloadsStatus, error) { monitors := &cozyv1alpha1.WorkloadMonitorList{} if err := r.c.List(ctx, monitors, client.InNamespace(namespace), @@ -1337,19 +1443,47 @@ func (r *REST) getWorkloadsOperational(ctx context.Context, namespace, appName s ); err != nil { return workloadsStatus{}, err } + // Ensure the freshOverride is represented in the aggregation even when + // the cache has not yet observed it (brand-new resource) or is behind. + replaced := false + if freshOverride != nil { + for i := range monitors.Items { + if monitors.Items[i].UID == freshOverride.UID || + (monitors.Items[i].Name == freshOverride.Name && monitors.Items[i].Namespace == freshOverride.Namespace) { + monitors.Items[i] = *freshOverride + replaced = true + break + } + } + if !replaced { + monitors.Items = append(monitors.Items, *freshOverride) + } + } if len(monitors.Items) == 0 { return workloadsStatus{operational: true, found: false}, nil } operational := true unknown := false + var latest metav1.Time for _, m := range monitors.Items { if m.Status.Operational == nil { unknown = true } else if !*m.Status.Operational { operational = false } + // Pick the most recent monitor mtime as a stable transition time. + if t := latestMonitorTime(&m); t.After(latest.Time) { + latest = t + } } - return workloadsStatus{operational: operational, found: true, unknown: unknown}, nil + return workloadsStatus{operational: operational, found: true, unknown: unknown, transitionTime: latest}, nil +} + +// latestMonitorTime returns the most recent timestamp associated with a +// WorkloadMonitor — currently only the object creation time is guaranteed. +// Status does not carry a transition time, so we fall back to CreationTimestamp. +func latestMonitorTime(m *cozyv1alpha1.WorkloadMonitor) metav1.Time { + return m.CreationTimestamp } // convertApplicationToHelmRelease implements the actual conversion logic diff --git a/pkg/registry/apps/application/rest_conditions_test.go b/pkg/registry/apps/application/rest_conditions_test.go index dbb99d52..185f59b2 100644 --- a/pkg/registry/apps/application/rest_conditions_test.go +++ b/pkg/registry/apps/application/rest_conditions_test.go @@ -61,7 +61,7 @@ func TestConvertConditions_WorkloadsReadyAdded(t *testing.T) { {Type: "Released", Status: metav1.ConditionTrue, Reason: "Succeeded", Message: "Released"}, } - app, err := r.convertHelmReleaseToApplication(context.TODO(), hr) + app, err := r.convertHelmReleaseToApplication(context.TODO(), hr, nil) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -107,7 +107,7 @@ func TestConvertConditions_ReadyNotOverriddenWhenWorkloadsNotReady(t *testing.T) {Type: "Ready", Status: metav1.ConditionTrue, Reason: "Succeeded", Message: "Release applied"}, } - app, err := r.convertHelmReleaseToApplication(context.TODO(), hr) + app, err := r.convertHelmReleaseToApplication(context.TODO(), hr, nil) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -139,7 +139,7 @@ func TestConvertConditions_NoOverrideWhenNoMonitors(t *testing.T) { {Type: "Ready", Status: metav1.ConditionTrue, Reason: "Succeeded", Message: "Release applied"}, } - app, err := r.convertHelmReleaseToApplication(context.TODO(), hr) + app, err := r.convertHelmReleaseToApplication(context.TODO(), hr, nil) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -181,7 +181,7 @@ func TestConvertConditions_ReadyStaysTrue_WhenAllOperational(t *testing.T) { {Type: "Released", Status: metav1.ConditionTrue, Reason: "Succeeded", Message: "Released"}, } - app, err := r.convertHelmReleaseToApplication(context.TODO(), hr) + app, err := r.convertHelmReleaseToApplication(context.TODO(), hr, nil) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -219,7 +219,7 @@ func TestConvertConditions_WorkloadsReadyTimestampIsNonZero(t *testing.T) { // No Ready condition — HR still being reconciled hr.Status.Conditions = []metav1.Condition{} - app, err := r.convertHelmReleaseToApplication(context.TODO(), hr) + app, err := r.convertHelmReleaseToApplication(context.TODO(), hr, nil) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -255,7 +255,7 @@ func TestConvertConditions_WorkloadsReadyUnknownWhenNilOperational(t *testing.T) {Type: "Ready", Status: metav1.ConditionTrue, Reason: "Succeeded", Message: "ok"}, } - app, err := r.convertHelmReleaseToApplication(context.TODO(), hr) + app, err := r.convertHelmReleaseToApplication(context.TODO(), hr, nil) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -302,7 +302,7 @@ func TestConvertConditions_WorkloadsReadyUnknownOnError(t *testing.T) { {Type: "Ready", Status: metav1.ConditionTrue, Reason: "Succeeded", Message: "ok"}, } - app, err := r.convertHelmReleaseToApplication(context.TODO(), hr) + app, err := r.convertHelmReleaseToApplication(context.TODO(), hr, nil) if err != nil { t.Fatalf("unexpected error: %v", err) } diff --git a/pkg/registry/apps/application/rest_validation_test.go b/pkg/registry/apps/application/rest_validation_test.go index 088990ce..b806161c 100644 --- a/pkg/registry/apps/application/rest_validation_test.go +++ b/pkg/registry/apps/application/rest_validation_test.go @@ -190,7 +190,7 @@ func TestConvertHelmReleaseToApplication_TenantNamespaceKindGate(t *testing.T) { }, } - app, err := r.convertHelmReleaseToApplication(context.Background(), hr) + app, err := r.convertHelmReleaseToApplication(context.Background(), hr, nil) if err != nil { t.Fatalf("convertHelmReleaseToApplication: %v", err) } @@ -208,7 +208,7 @@ func TestConvertHelmReleaseToApplication_TenantNamespaceKindGate(t *testing.T) { }, } - app, err := r.convertHelmReleaseToApplication(context.Background(), hr) + app, err := r.convertHelmReleaseToApplication(context.Background(), hr, nil) if err != nil { t.Fatalf("convertHelmReleaseToApplication: %v", err) } diff --git a/pkg/registry/apps/application/rest_watch_test.go b/pkg/registry/apps/application/rest_watch_test.go index 8fe2a04a..93a51f3c 100644 --- a/pkg/registry/apps/application/rest_watch_test.go +++ b/pkg/registry/apps/application/rest_watch_test.go @@ -144,7 +144,7 @@ func TestWatchIntegration_MonitorDeletionDropsWorkloadsReady(t *testing.T) { r := newTestRESTWithSchemesFromClient(c) // Step 1: With non-operational monitor, WorkloadsReady=False, Ready=True - app1, err := r.convertHelmReleaseToApplication(context.TODO(), hr) + app1, err := r.convertHelmReleaseToApplication(context.TODO(), hr, nil) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -163,7 +163,7 @@ func TestWatchIntegration_MonitorDeletionDropsWorkloadsReady(t *testing.T) { } // Step 3: WorkloadsReady should disappear, Ready stays True - app2, err := r.convertHelmReleaseToApplication(context.TODO(), hr) + app2, err := r.convertHelmReleaseToApplication(context.TODO(), hr, nil) if err != nil { t.Fatalf("unexpected error: %v", err) } diff --git a/pkg/registry/apps/application/rest_workloads_test.go b/pkg/registry/apps/application/rest_workloads_test.go index 30dd85c6..b706c323 100644 --- a/pkg/registry/apps/application/rest_workloads_test.go +++ b/pkg/registry/apps/application/rest_workloads_test.go @@ -40,7 +40,7 @@ func newTestRESTWithSchemes(objs ...runtime.Object) *REST { func TestGetWorkloadsOperational_NoMonitors(t *testing.T) { r := newTestRESTWithSchemes() - ws, err := r.getWorkloadsOperational(context.TODO(), "default", "mydb") + ws, err := r.getWorkloadsOperational(context.TODO(), "default", "mydb", nil) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -83,7 +83,7 @@ func TestGetWorkloadsOperational_AllOperational(t *testing.T) { } r := newTestRESTWithSchemes(m1, m2) - ws, err := r.getWorkloadsOperational(context.TODO(), "default", "mydb") + ws, err := r.getWorkloadsOperational(context.TODO(), "default", "mydb", nil) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -126,7 +126,7 @@ func TestGetWorkloadsOperational_SomeNotOperational(t *testing.T) { } r := newTestRESTWithSchemes(m1, m2) - ws, err := r.getWorkloadsOperational(context.TODO(), "default", "mydb") + ws, err := r.getWorkloadsOperational(context.TODO(), "default", "mydb", nil) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -155,7 +155,7 @@ func TestGetWorkloadsOperational_OperationalNil(t *testing.T) { } r := newTestRESTWithSchemes(m) - ws, err := r.getWorkloadsOperational(context.TODO(), "default", "mydb") + ws, err := r.getWorkloadsOperational(context.TODO(), "default", "mydb", nil) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -198,7 +198,7 @@ func TestGetWorkloadsOperational_MixedNilAndOperational(t *testing.T) { } r := newTestRESTWithSchemes(m1, m2) - ws, err := r.getWorkloadsOperational(context.TODO(), "default", "mydb") + ws, err := r.getWorkloadsOperational(context.TODO(), "default", "mydb", nil) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -238,7 +238,7 @@ func TestGetWorkloadsOperational_MixedFailedAndPending(t *testing.T) { } r := newTestRESTWithSchemes(m1, m2) - ws, err := r.getWorkloadsOperational(context.TODO(), "default", "mydb") + ws, err := r.getWorkloadsOperational(context.TODO(), "default", "mydb", nil) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -296,7 +296,7 @@ func TestConvertConditions_MixedFailedAndPendingShowsFalse(t *testing.T) { {Type: "Ready", Status: metav1.ConditionTrue, Reason: "Succeeded", Message: "ok"}, } - app, err := r.convertHelmReleaseToApplication(context.TODO(), hr) + app, err := r.convertHelmReleaseToApplication(context.TODO(), hr, nil) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -328,7 +328,7 @@ func TestGetWorkloadsOperational_DifferentApp_NotFound(t *testing.T) { } r := newTestRESTWithSchemes(m) - ws, err := r.getWorkloadsOperational(context.TODO(), "default", "mydb") + ws, err := r.getWorkloadsOperational(context.TODO(), "default", "mydb", nil) if err != nil { t.Fatalf("unexpected error: %v", err) } From 9cc5deeabe390857381e7519151872ef30f4674a Mon Sep 17 00:00:00 2001 From: Aleksei Sviridkin Date: Thu, 16 Apr 2026 01:10:10 +0300 Subject: [PATCH 4/4] fix(dashboard): fall back to .firstTimestamp in Event Time column MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit core/v1 Events populate .lastTimestamp and .firstTimestamp but leave .eventTime null; events.k8s.io/v1 Events do the opposite. The previous column bound to .eventTime alone and rendered 'Invalid Date' for every Helm-generated event. Extend createTimestampColumn with an optional second jsonPath that is encoded as a nested reqsJsonPath fallback in the template, and use .eventTime → .firstTimestamp for Event Time and .lastTimestamp → .eventTime for Last Seen so both APIs render correctly. Signed-off-by: Aleksei Sviridkin --- .../controller/dashboard/static_helpers.go | 27 ++++++++++++++++--- .../controller/dashboard/static_refactored.go | 9 ++++--- 2 files changed, 29 insertions(+), 7 deletions(-) diff --git a/internal/controller/dashboard/static_helpers.go b/internal/controller/dashboard/static_helpers.go index 1a1018cb..db37e05b 100644 --- a/internal/controller/dashboard/static_helpers.go +++ b/internal/controller/dashboard/static_helpers.go @@ -650,12 +650,31 @@ func createStringColumn(name, jsonPath string) map[string]any { } } -// createTimestampColumn creates a timestamp column with custom formatting -func createTimestampColumn(name, jsonPath string) map[string]any { +// createTimestampColumn creates a timestamp column with custom formatting. +// Extra jsonPaths act as ordered fallbacks: the first non-null path wins, +// and `-` is used only when every path is null. Depth is capped at two +// because the template parser's non-greedy matcher cannot track more than +// one level of alternating quotes in a nested reqsJsonPath fallback. +func createTimestampColumn(name string, jsonPaths ...string) map[string]any { + if len(jsonPaths) == 0 { + panic("createTimestampColumn requires at least one jsonPath") + } + if len(jsonPaths) > 2 { + panic("createTimestampColumn supports at most two jsonPaths (primary + one fallback)") + } + + var text string + switch len(jsonPaths) { + case 1: + text = "{reqsJsonPath[0]['" + jsonPaths[0] + "']['-']}" + case 2: + text = "{reqsJsonPath[0]['" + jsonPaths[0] + "'][\"{reqsJsonPath[0]['" + jsonPaths[1] + "']['-']}\"]}" + } + return map[string]any{ "name": name, "type": "factory", - "jsonPath": jsonPath, + "jsonPath": jsonPaths[0], "customProps": map[string]any{ "disableEventBubbling": true, "items": []any{ @@ -673,7 +692,7 @@ func createTimestampColumn(name, jsonPath string) map[string]any { "data": map[string]any{ "formatter": "timestamp", "id": "time-value", - "text": "{reqsJsonPath[0]['" + jsonPath + "']['-']}", + "text": text, }, }, }, diff --git a/internal/controller/dashboard/static_refactored.go b/internal/controller/dashboard/static_refactored.go index a73026bc..d96bbaaa 100644 --- a/internal/controller/dashboard/static_refactored.go +++ b/internal/controller/dashboard/static_refactored.go @@ -224,10 +224,13 @@ func CreateAllCustomColumnsOverrides() []*dashboardv1alpha1.CustomColumnsOverrid createStringColumn("Name", ".name"), }), - // Factory details events + // Factory details events. Event Time falls back to `.firstTimestamp` + // because core/v1 Events (Helm, controller-runtime) populate only the + // legacy timestamps while events.k8s.io/v1 Events populate only + // `.eventTime`; taking the first non-null of the two covers both. createCustomColumnsOverride("factory-details-events", []any{ - createTimestampColumn("Last Seen", ".lastTimestamp"), - createTimestampColumn("Event Time", ".eventTime"), + createTimestampColumn("Last Seen", ".lastTimestamp", ".eventTime"), + createTimestampColumn("Event Time", ".eventTime", ".firstTimestamp"), createStringColumn("Type", ".type"), createStringColumn("Reason", ".reason"), createStringColumn("Object", ".involvedObject.kind"),