Compare commits

..

2 Commits

Author SHA1 Message Date
Brian Kane
73e4c791a9 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>
2026-03-28 21:37:45 -07:00
Jerrin Francis
69d046f7b3 Fix: Align legacy config provider keys with workflow repo to resolve CUE conflict and key collision (#7078)
* Fix: Align legacy config provider keys with workflow repo to resolve CUE conflict and kube key collision

  Rename legacy config #do keys and Go provider map keys from short form
  (create, read, list, delete) to hyphenated form (create-config,
  read-config, list-config, delete-config) to match workflow PR #225.

  This resolves three issues:
  - CUE unification failure when kubevela and workflow legacy config
    templates are concatenated into the same op package
  - Go provider key collision where kube's read/list/delete silently
    overwrote config's identically-named functions
  - Silent misdispatch of op.#ReadConfig/ListConfig/DeleteConfig to
    kube handlers instead of config handlers

Signed-off-by: Jerrin Francis <jerrinfrancis7@gmail.com>

* Adding testcase to test future key drifts

Signed-off-by: Jerrin Francis <jerrinfrancis7@gmail.com>

---------

Signed-off-by: Jerrin Francis <jerrinfrancis7@gmail.com>
2026-03-24 18:37:09 -07:00
7 changed files with 127 additions and 108 deletions

View File

@@ -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 {

View File

@@ -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))
}
})
}
}

View File

@@ -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{

View File

@@ -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)
}
})
}
}

View File

@@ -1,7 +1,7 @@
// config.cue
#CreateConfig: {
#do: "create"
#do: "create-config"
#provider: "op"
name: string
@@ -13,7 +13,7 @@
}
#DeleteConfig: {
#do: "delete"
#do: "delete-config"
#provider: "op"
name: string
@@ -21,7 +21,7 @@
}
#ReadConfig: {
#do: "read"
#do: "read-config"
#provider: "op"
name: string
@@ -31,7 +31,7 @@
}
#ListConfig: {
#do: "list"
#do: "list-config"
#provider: "op"
// Must query with the template

View File

@@ -148,9 +148,9 @@ func GetTemplate() string {
// GetProviders returns the cue providers.
func GetProviders() map[string]cuexruntime.ProviderFn {
return map[string]cuexruntime.ProviderFn{
"create": oamprovidertypes.OAMGenericProviderFn[CreateConfigProperties, any](CreateConfig),
"read": oamprovidertypes.OAMGenericProviderFn[config.NamespacedName, ReadResult](ReadConfig),
"list": oamprovidertypes.OAMGenericProviderFn[ListVars, ListResult](ListConfig),
"delete": oamprovidertypes.OAMGenericProviderFn[config.NamespacedName, any](DeleteConfig),
"create-config": oamprovidertypes.OAMGenericProviderFn[CreateConfigProperties, any](CreateConfig),
"read-config": oamprovidertypes.OAMGenericProviderFn[config.NamespacedName, ReadResult](ReadConfig),
"list-config": oamprovidertypes.OAMGenericProviderFn[ListVars, ListResult](ListConfig),
"delete-config": oamprovidertypes.OAMGenericProviderFn[config.NamespacedName, any](DeleteConfig),
}
}

View File

@@ -76,6 +76,15 @@ var _ = AfterSuite(func() {
var _ = Describe("Test the config provider", func() {
It("GetProviders returns expected hyphenated keys", func() {
providers := GetProviders()
Expect(providers).To(HaveKey("create-config"))
Expect(providers).To(HaveKey("read-config"))
Expect(providers).To(HaveKey("list-config"))
Expect(providers).To(HaveKey("delete-config"))
Expect(providers).To(HaveLen(4))
})
It("test creating a config", func() {
ctx := context.Background()
params := &CreateParams{