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

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.


(cherry picked from commit 73e4c791a9)

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

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

@@ -215,20 +215,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")
@@ -270,6 +268,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 {
@@ -713,13 +715,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{}{}
@@ -734,16 +735,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))
-			}
 		})
 	}
 }

pkg/resourcekeeper/resourcekeeper.go

@@ -26,6 +26,7 @@ import (
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
+	"github.com/oam-dev/kubevela/apis/core.oam.dev/common"
 	"github.com/oam-dev/kubevela/apis/core.oam.dev/v1alpha1"
 	"github.com/oam-dev/kubevela/apis/core.oam.dev/v1beta1"
 	"github.com/oam-dev/kubevela/pkg/multicluster"
@@ -45,6 +46,9 @@ type ResourceKeeper interface {
 
 	DispatchComponentRevision(context.Context, *appsv1.ControllerRevision) error
 	DeleteComponentRevision(context.Context, *appsv1.ControllerRevision) error
+
+	// GetAppliedResources returns the current applied resources from the ResourceTracker.
+	GetAppliedResources() []common.ClusterObjectReference
 }
 
 type resourceKeeper struct {
@@ -125,6 +129,20 @@ func (h *resourceKeeper) loadResourceTrackers(ctx context.Context) (err error) {
 	return err
 }
 
+// GetAppliedResources returns all resources from the current ResourceTracker as ClusterObjectReferences.
+// Resources pending deletion (Deleted=true) are included as they still exist in the cluster.
+// Returns an empty slice if no current ResourceTracker is loaded.
+func (h *resourceKeeper) GetAppliedResources() []common.ClusterObjectReference {
+	if h._currentRT == nil {
+		return []common.ClusterObjectReference{}
+	}
+	refs := make([]common.ClusterObjectReference, 0, len(h._currentRT.Spec.ManagedResources))
+	for _, mr := range h._currentRT.Spec.ManagedResources {
+		refs = append(refs, mr.ClusterObjectReference)
+	}
+	return refs
+}
+
 // NewResourceKeeper create a handler for dispatching and deleting resources
 func NewResourceKeeper(ctx context.Context, cli client.Client, app *v1beta1.Application) (_ ResourceKeeper, err error) {
 	h := &resourceKeeper{

pkg/resourcekeeper/resourcekeeper_test.go

@@ -21,11 +21,14 @@ import (
 	"fmt"
 	"testing"
 
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	corev1 "k8s.io/api/core/v1"
 	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client/fake"
 
+	oamcommon "github.com/oam-dev/kubevela/apis/core.oam.dev/common"
 	"github.com/oam-dev/kubevela/apis/core.oam.dev/v1beta1"
 	"github.com/oam-dev/kubevela/pkg/oam"
 	"github.com/oam-dev/kubevela/pkg/oam/util"
@@ -97,3 +100,62 @@ func TestNewResourceKeeper(t *testing.T) {
 	r.NotNil(currentRT)
 	r.Equal(3, len(rk._historyRTs))
 }
+
+func TestGetAppliedResources(t *testing.T) {
+	ref := func(name, kind, component string, deleted bool) v1beta1.ManagedResource {
+		return v1beta1.ManagedResource{
+			ClusterObjectReference: oamcommon.ClusterObjectReference{
+				Creator: oamcommon.WorkflowResourceCreator,
+				ObjectReference: corev1.ObjectReference{
+					Name:       name,
+					Namespace:  "default",
+					Kind:       kind,
+					APIVersion: "v1",
+				},
+			},
+			OAMObjectReference: oamcommon.OAMObjectReference{Component: component},
+			Deleted:            deleted,
+		}
+	}
+
+	tests := []struct {
+		name          string
+		managedRes    []v1beta1.ManagedResource
+		expectedNames []string
+	}{
+		{
+			name: "returns all resources including pending-delete",
+			managedRes: []v1beta1.ManagedResource{
+				ref("shared-config", "ConfigMap", "my-component", false),
+				ref("old-config", "ConfigMap", "removed-component", true),
+			},
+			expectedNames: []string{"shared-config", "old-config"},
+		},
+		{
+			name:          "returns empty when no current RT",
+			managedRes:    nil,
+			expectedNames: []string{},
+		},
+		{
+			name:          "returns empty when RT has no resources",
+			managedRes:    []v1beta1.ManagedResource{},
+			expectedNames: []string{},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			rk := &resourceKeeper{}
+			if tt.managedRes != nil {
+				rk._currentRT = &v1beta1.ResourceTracker{
+					Spec: v1beta1.ResourceTrackerSpec{ManagedResources: tt.managedRes},
+				}
+			}
+			result := rk.GetAppliedResources()
+			require.Equal(t, len(tt.expectedNames), len(result))
+			for i, name := range tt.expectedNames {
+				assert.Equal(t, name, result[i].Name)
+			}
+		})
+	}
+}