From 555e4416f4abb5d0ee53d34f56f7e45d5cade8e5 Mon Sep 17 00:00:00 2001 From: Brian Kane Date: Wed, 21 Jan 2026 09:26:17 +0000 Subject: [PATCH] Fix: 7018 Ensure Component removals are correctly persisted and reflected in status (#7027) Signed-off-by: Brian Kane --- .../application/application_controller.go | 52 ++++++- .../v1beta1/application/evalstatus_test.go | 127 ++++++++++++++++++ 2 files changed, 178 insertions(+), 1 deletion(-) diff --git a/pkg/controller/core.oam.dev/v1beta1/application/application_controller.go b/pkg/controller/core.oam.dev/v1beta1/application/application_controller.go index 50f032f5f..a31916ec1 100644 --- a/pkg/controller/core.oam.dev/v1beta1/application/application_controller.go +++ b/pkg/controller/core.oam.dev/v1beta1/application/application_controller.go @@ -224,6 +224,22 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu handler.addAppliedResource(true, app.Status.AppliedResources...) app.Status.AppliedResources = handler.appliedResources app.Status.Services = handler.services + + // Remove status entries for components that no longer exist in spec + filteredServices, filteredResources, componentsRemoved := filterRemovedComponentsFromStatus( + app.Spec.Components, + app.Status.Services, + app.Status.AppliedResources, + ) + app.Status.Services = filteredServices + app.Status.AppliedResources = filteredResources + handler.services = filteredServices + handler.appliedResources = filteredResources + + if componentsRemoved { + logCtx.Info("Removed deleted components from status") + } + workflowUpdated := app.Status.Workflow.Message != "" && workflowInstance.Status.Message == "" workflowInstance.Status.Phase = workflowState app.Status.Workflow = workflow.ConvertWorkflowStatus(workflowInstance.Status, app.Status.Workflow.AppRevision) @@ -319,7 +335,8 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu Reason: condition.ReasonReconcileSuccess, }) r.Recorder.Event(app, event.Normal(velatypes.ReasonDeployed, velatypes.MessageDeployed)) - return r.gcResourceTrackers(logCtx, handler, phase, true, false) + // Use Update instead of Patch when components were removed to properly clear status arrays + return r.gcResourceTrackers(logCtx, handler, phase, true, componentsRemoved) } func (r *Reconciler) stateKeep(logCtx monitorContext.Context, handler *AppHandler, app *v1beta1.Application) { @@ -730,6 +747,39 @@ func setVelaVersion(app *v1beta1.Application) { } } +// filterRemovedComponentsFromStatus removes status entries for components no longer in spec. +// Returns filtered lists and whether any components were removed (used to determine Update vs Patch). +func filterRemovedComponentsFromStatus( + components []common.ApplicationComponent, + services []common.ApplicationComponentStatus, + appliedResources []common.ClusterObjectReference, +) (filteredServices []common.ApplicationComponentStatus, filteredResources []common.ClusterObjectReference, removed bool) { + componentMap := make(map[string]struct{}, len(components)) + for _, comp := range components { + componentMap[comp.Name] = struct{}{} + } + + filteredServices = make([]common.ApplicationComponentStatus, 0, len(services)) + for _, svc := range services { + if _, found := componentMap[svc.Name]; found { + filteredServices = append(filteredServices, svc) + } else { + removed = true + } + } + + filteredResources = make([]common.ClusterObjectReference, 0, len(appliedResources)) + for _, res := range appliedResources { + if _, found := componentMap[res.Name]; found { + filteredResources = append(filteredResources, res) + } else { + removed = true + } + } + + return filteredServices, filteredResources, removed +} + func evalStatus(ctx monitorContext.Context, handler *AppHandler, appFile *appfile.Appfile, appParser *appfile.Parser) bool { healthCheck := handler.checkComponentHealth(appParser, appFile) if !hasHealthCheckPolicy(appFile.ParsedPolicies) { diff --git a/pkg/controller/core.oam.dev/v1beta1/application/evalstatus_test.go b/pkg/controller/core.oam.dev/v1beta1/application/evalstatus_test.go index 9d0c7b033..3b266440f 100644 --- a/pkg/controller/core.oam.dev/v1beta1/application/evalstatus_test.go +++ b/pkg/controller/core.oam.dev/v1beta1/application/evalstatus_test.go @@ -23,6 +23,7 @@ import ( "cuelang.org/go/cue" "github.com/stretchr/testify/assert" + corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" @@ -181,3 +182,129 @@ func Test_applyComponentHealthToServices(t *testing.T) { }) } } + +func TestFilterRemovedComponentsFromStatus(t *testing.T) { + tests := []struct { + name string + components []common.ApplicationComponent + statusServices []common.ApplicationComponentStatus + statusResources []common.ClusterObjectReference + expectedServices []string + expectedResources []string + componentsRemoved bool + }{ + { + name: "removed components are filtered from status", + components: []common.ApplicationComponent{ + {Name: "backend", Type: "webservice"}, + }, + statusServices: []common.ApplicationComponentStatus{ + {Name: "frontend", Namespace: "default"}, + {Name: "backend", Namespace: "default"}, + }, + statusResources: []common.ClusterObjectReference{ + { + ObjectReference: corev1.ObjectReference{ + Name: "frontend", + Namespace: "default", + Kind: "Deployment", + }, + }, + { + ObjectReference: corev1.ObjectReference{ + Name: "backend", + Namespace: "default", + Kind: "Deployment", + }, + }, + }, + expectedServices: []string{"backend"}, + expectedResources: []string{"backend"}, + componentsRemoved: true, + }, + { + name: "all components removed results in empty status", + components: []common.ApplicationComponent{}, + statusServices: []common.ApplicationComponentStatus{ + {Name: "frontend", Namespace: "default"}, + {Name: "backend", Namespace: "default"}, + }, + statusResources: []common.ClusterObjectReference{ + { + ObjectReference: corev1.ObjectReference{ + Name: "frontend", + Namespace: "default", + Kind: "Deployment", + }, + }, + { + ObjectReference: corev1.ObjectReference{ + Name: "backend", + Namespace: "default", + Kind: "Deployment", + }, + }, + }, + expectedServices: []string{}, + expectedResources: []string{}, + componentsRemoved: true, + }, + { + name: "no components removed keeps all status entries", + components: []common.ApplicationComponent{ + {Name: "frontend", Type: "webservice"}, + {Name: "backend", Type: "webservice"}, + }, + statusServices: []common.ApplicationComponentStatus{ + {Name: "frontend", Namespace: "default"}, + {Name: "backend", Namespace: "default"}, + }, + statusResources: []common.ClusterObjectReference{ + { + ObjectReference: corev1.ObjectReference{ + Name: "frontend", + Namespace: "default", + Kind: "Deployment", + }, + }, + { + ObjectReference: corev1.ObjectReference{ + Name: "backend", + Namespace: "default", + Kind: "Deployment", + }, + }, + }, + expectedServices: []string{"frontend", "backend"}, + expectedResources: []string{"frontend", "backend"}, + componentsRemoved: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + filteredServices, filteredResources, componentsRemoved := filterRemovedComponentsFromStatus( + tt.components, + tt.statusServices, + tt.statusResources, + ) + + assert.Equal(t, tt.componentsRemoved, componentsRemoved, + "componentsRemoved flag should match expected value") + + assert.Equal(t, len(tt.expectedServices), len(filteredServices), + "filtered services count should match expected") + for i, expectedName := range tt.expectedServices { + assert.Equal(t, expectedName, filteredServices[i].Name, + fmt.Sprintf("service at index %d should be %s", i, expectedName)) + } + + assert.Equal(t, len(tt.expectedResources), len(filteredResources), + "filtered resources count should match expected") + for i, expectedName := range tt.expectedResources { + assert.Equal(t, expectedName, filteredResources[i].Name, + fmt.Sprintf("resource at index %d should be %s", i, expectedName)) + } + }) + } +}