Fix: rebuild appliedResources from ResourceTracker instead of filtering by name (#7083)

appliedResources entries use resource names, not component names, so filtering
them against component names incorrectly dropped valid entries. Rebuild directly
from the current ResourceTracker each reconcile - already in memory, no extra
API calls.

Signed-off-by: Brian Kane <briankane1@gmail.com>

pkg/controller/core.oam.dev/v1beta1/application/application_controller.go

@@ -238,20 +238,18 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
 	}
 
 	handler.addServiceStatus(false, app.Status.Services...)
-	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(
+	handler.addAppliedResource(true, app.Status.AppliedResources...)
+	app.Status.AppliedResources = handler.appliedResources
+
+	// Remove services[] entries for components that no longer exist in spec
+	filteredServices, 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")
@@ -316,6 +314,10 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
 	default:
 	}
 
+	// Rebuild appliedResources from the ResourceTracker now that the workflow has finished
+	// dispatching. The RT is the authoritative source
+	app.Status.AppliedResources = handler.resourceKeeper.GetAppliedResources()
+
 	var phase = common.ApplicationRunning
 	isHealthy := evalStatus(logCtx, handler, appFile, appParser)
 	if !isHealthy {
@@ -853,13 +855,12 @@ 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).
+// filterRemovedComponentsFromStatus removes services[] entries for components no longer in spec.
+// Returns filtered services and whether any 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) {
+) (filteredServices []common.ApplicationComponentStatus, removed bool) {
 	componentMap := make(map[string]struct{}, len(components))
 	for _, comp := range components {
 		componentMap[comp.Name] = struct{}{}
@@ -874,16 +875,7 @@ func filterRemovedComponentsFromStatus(
 		}
 	}
 
-	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
+	return filteredServices, removed
 }
 
 func evalStatus(ctx monitorContext.Context, handler *AppHandler, appFile *appfile.Appfile, appParser *appfile.Parser) bool {

pkg/controller/core.oam.dev/v1beta1/application/evalstatus_test.go

@@ -23,7 +23,6 @@ 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"
 
@@ -185,16 +184,14 @@ 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             string
+		components       []common.ApplicationComponent
+		statusServices   []common.ApplicationComponentStatus
+		expectedServices []string
+		servicesRemoved  bool
 	}{
 		{
-			name: "removed components are filtered from status",
+			name: "removed component is filtered from services",
 			components: []common.ApplicationComponent{
 				{Name: "backend", Type: "webservice"},
 			},
@@ -202,55 +199,21 @@ func TestFilterRemovedComponentsFromStatus(t *testing.T) {
 				{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,
+			expectedServices: []string{"backend"},
+			servicesRemoved:  true,
 		},
 		{
-			name:       "all components removed results in empty status",
+			name:       "all components removed results in empty services",
 			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,
+			expectedServices: []string{},
+			servicesRemoved:  true,
 		},
 		{
-			name: "no components removed keeps all status entries",
+			name: "no components removed keeps all services",
 			components: []common.ApplicationComponent{
 				{Name: "frontend", Type: "webservice"},
 				{Name: "backend", Type: "webservice"},
@@ -259,38 +222,20 @@ func TestFilterRemovedComponentsFromStatus(t *testing.T) {
 				{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,
+			expectedServices: []string{"frontend", "backend"},
+			servicesRemoved:  false,
 		},
 	}
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			filteredServices, filteredResources, componentsRemoved := filterRemovedComponentsFromStatus(
+			filteredServices, servicesRemoved := filterRemovedComponentsFromStatus(
 				tt.components,
 				tt.statusServices,
-				tt.statusResources,
 			)
 
-			assert.Equal(t, tt.componentsRemoved, componentsRemoved,
-				"componentsRemoved flag should match expected value")
+			assert.Equal(t, tt.servicesRemoved, servicesRemoved,
+				"servicesRemoved flag should match expected value")
 
 			assert.Equal(t, len(tt.expectedServices), len(filteredServices),
 				"filtered services count should match expected")
@@ -298,13 +243,6 @@ func TestFilterRemovedComponentsFromStatus(t *testing.T) {
 				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))
-			}
 		})
 	}
 }