diff --git a/pkg/controller/kcc/cnc_test.go b/pkg/controller/kcc/cnc_test.go index 941f88d289..39d69ccf2a 100644 --- a/pkg/controller/kcc/cnc_test.go +++ b/pkg/controller/kcc/cnc_test.go @@ -75,8 +75,8 @@ func TestCustomNodeConfigController_Run(t *testing.T) { kccTargetList: []runtime.Object{ &v1alpha1.AdminQoSConfiguration{ TypeMeta: v1.TypeMeta{ - Kind: "EvictionConfiguration", - APIVersion: "config.katalyst.kubewharf.io/v1alpha1", + Kind: crd.ResourceKindAdminQoSConfiguration, + APIVersion: v1alpha1.SchemeGroupVersion.String(), }, ObjectMeta: v1.ObjectMeta{ Name: "default", diff --git a/pkg/controller/kcc/kcc_test.go b/pkg/controller/kcc/kcc_test.go index 9be7c43137..96c045f5cf 100644 --- a/pkg/controller/kcc/kcc_test.go +++ b/pkg/controller/kcc/kcc_test.go @@ -107,8 +107,8 @@ func TestKatalystCustomConfigController_Run(t *testing.T) { kccTargetList: []runtime.Object{ &v1alpha1.AdminQoSConfiguration{ TypeMeta: v1.TypeMeta{ - Kind: "EvictionConfiguration", - APIVersion: "config.katalyst.kubewharf.io/v1alpha1", + Kind: crd.ResourceKindAdminQoSConfiguration, + APIVersion: v1alpha1.SchemeGroupVersion.String(), }, ObjectMeta: v1.ObjectMeta{ Name: "default", @@ -167,8 +167,8 @@ func TestKatalystCustomConfigController_Run(t *testing.T) { kccTargetList: []runtime.Object{ &v1alpha1.AdminQoSConfiguration{ TypeMeta: v1.TypeMeta{ - Kind: "EvictionConfiguration", - APIVersion: "config.katalyst.kubewharf.io/v1alpha1", + Kind: crd.ResourceKindAdminQoSConfiguration, + APIVersion: v1alpha1.SchemeGroupVersion.String(), }, ObjectMeta: v1.ObjectMeta{ Name: "default", @@ -231,8 +231,8 @@ func TestKatalystCustomConfigController_Run(t *testing.T) { kccTargetList: []runtime.Object{ &v1alpha1.AdminQoSConfiguration{ TypeMeta: v1.TypeMeta{ - Kind: "EvictionConfiguration", - APIVersion: "config.katalyst.kubewharf.io/v1alpha1", + Kind: crd.ResourceKindAdminQoSConfiguration, + APIVersion: v1alpha1.SchemeGroupVersion.String(), }, ObjectMeta: v1.ObjectMeta{ Name: "default", diff --git a/pkg/controller/kcc/kcct.go b/pkg/controller/kcc/kcct.go index b1fcbc25c1..c4c47614e4 100644 --- a/pkg/controller/kcc/kcct.go +++ b/pkg/controller/kcc/kcct.go @@ -257,6 +257,28 @@ func (k *KatalystCustomConfigTargetController) katalystCustomConfigTargetHandler if err != nil { return err } + } else if !oldKCCTargetResource.CheckValid() && targetResource.CheckValid() { + // if old kcc is overlap and change to valid now, it needs trigger all other invalid kcc targets to reconcile + targets, err := k.listAllKCCTargetResource(gvr) + if err != nil { + return err + } + + var invalidTargets []util.KCCTargetResource + for _, target := range targets { + if targetResource.GetName() == target.GetName() && targetResource.GetNamespace() == target.GetNamespace() { + continue + } + + if !target.CheckValid() { + invalidTargets = append(invalidTargets, target) + } + } + + err = k.enqueueTargets(gvr, invalidTargets) + if err != nil { + return err + } } } diff --git a/pkg/controller/kcc/kcct_test.go b/pkg/controller/kcc/kcct_test.go index 5ba011d8d1..27e3b2d237 100644 --- a/pkg/controller/kcc/kcct_test.go +++ b/pkg/controller/kcc/kcct_test.go @@ -24,6 +24,7 @@ import ( "github.com/stretchr/testify/assert" v1 "k8s.io/api/core/v1" apiequality "k8s.io/apimachinery/pkg/api/equality" + "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/labels" @@ -91,9 +92,11 @@ func TestKatalystCustomConfigTargetController_Run(t *testing.T) { t.Parallel() type args struct { - kccList []runtime.Object - kccTargetList []runtime.Object - kccConfig *config.Configuration + kccList []runtime.Object + kccListChanged []runtime.Object + kccTargetList []runtime.Object + kccTargetListChanged []runtime.Object + kccConfig *config.Configuration } tests := []struct { name string @@ -122,8 +125,8 @@ func TestKatalystCustomConfigTargetController_Run(t *testing.T) { kccTargetList: []runtime.Object{ &v1alpha1.AdminQoSConfiguration{ TypeMeta: metav1.TypeMeta{ - Kind: "EvictionConfiguration", - APIVersion: "config.katalyst.kubewharf.io/v1alpha1", + Kind: crd.ResourceKindAdminQoSConfiguration, + APIVersion: v1alpha1.SchemeGroupVersion.String(), }, ObjectMeta: metav1.ObjectMeta{ Name: "default", @@ -143,8 +146,8 @@ func TestKatalystCustomConfigTargetController_Run(t *testing.T) { }, &v1alpha1.AdminQoSConfiguration{ TypeMeta: metav1.TypeMeta{ - Kind: "EvictionConfiguration", - APIVersion: "config.katalyst.kubewharf.io/v1alpha1", + Kind: crd.ResourceKindAdminQoSConfiguration, + APIVersion: v1alpha1.SchemeGroupVersion.String(), }, ObjectMeta: metav1.ObjectMeta{ Name: "aa=bb", @@ -167,8 +170,8 @@ func TestKatalystCustomConfigTargetController_Run(t *testing.T) { }, &v1alpha1.AdminQoSConfiguration{ TypeMeta: metav1.TypeMeta{ - Kind: "EvictionConfiguration", - APIVersion: "config.katalyst.kubewharf.io/v1alpha1", + Kind: crd.ResourceKindAdminQoSConfiguration, + APIVersion: v1alpha1.SchemeGroupVersion.String(), }, ObjectMeta: metav1.ObjectMeta{ Name: "node-1", @@ -200,6 +203,104 @@ func TestKatalystCustomConfigTargetController_Run(t *testing.T) { }, }, }, + { + name: "kcc target from valid to invalid", + args: args{ + kccList: []runtime.Object{ + &v1alpha1.KatalystCustomConfig{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-kcc", + Namespace: "default", + }, + Spec: v1alpha1.KatalystCustomConfigSpec{ + TargetType: crd.AdminQoSConfigurationGVR, + NodeLabelSelectorAllowedKeyList: []v1alpha1.PriorityNodeLabelSelectorAllowedKeyList{ + { + Priority: 0, + KeyList: []string{"aa"}, + }, + }, + }, + }, + }, + kccTargetList: []runtime.Object{ + &v1alpha1.AdminQoSConfiguration{ + TypeMeta: metav1.TypeMeta{ + Kind: crd.ResourceKindAdminQoSConfiguration, + APIVersion: v1alpha1.SchemeGroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "config-1", + Namespace: "default", + }, + Spec: v1alpha1.AdminQoSConfigurationSpec{ + GenericConfigSpec: v1alpha1.GenericConfigSpec{ + NodeLabelSelector: "aa=bb", + }, + Config: v1alpha1.AdminQoSConfig{ + EvictionConfig: &v1alpha1.EvictionConfig{ + ReclaimedResourcesEvictionConfig: &v1alpha1.ReclaimedResourcesEvictionConfig{ + EvictionThreshold: map[v1.ResourceName]float64{ + v1.ResourceCPU: 5.0, + }, + }, + }, + }, + }, + }, + &v1alpha1.AdminQoSConfiguration{ + TypeMeta: metav1.TypeMeta{ + Kind: crd.ResourceKindAdminQoSConfiguration, + APIVersion: v1alpha1.SchemeGroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "config-2", + Namespace: "default", + }, + Spec: v1alpha1.AdminQoSConfigurationSpec{ + GenericConfigSpec: v1alpha1.GenericConfigSpec{ + NodeLabelSelector: "aa=bb", + }, + Config: v1alpha1.AdminQoSConfig{ + EvictionConfig: &v1alpha1.EvictionConfig{ + ReclaimedResourcesEvictionConfig: &v1alpha1.ReclaimedResourcesEvictionConfig{ + EvictionThreshold: map[v1.ResourceName]float64{ + v1.ResourceCPU: 5.0, + }, + }, + }, + }, + }, + }, + }, + kccTargetListChanged: []runtime.Object{ + &v1alpha1.AdminQoSConfiguration{ + TypeMeta: metav1.TypeMeta{ + Kind: crd.ResourceKindAdminQoSConfiguration, + APIVersion: v1alpha1.SchemeGroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "config-2", + Namespace: "default", + }, + Spec: v1alpha1.AdminQoSConfigurationSpec{ + GenericConfigSpec: v1alpha1.GenericConfigSpec{ + NodeLabelSelector: "aa=cc", + }, + Config: v1alpha1.AdminQoSConfig{ + EvictionConfig: &v1alpha1.EvictionConfig{ + ReclaimedResourcesEvictionConfig: &v1alpha1.ReclaimedResourcesEvictionConfig{ + EvictionThreshold: map[v1.ResourceName]float64{ + v1.ResourceCPU: 5.0, + }, + }, + }, + }, + }, + }, + }, + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -215,7 +316,7 @@ func TestKatalystCustomConfigTargetController_Run(t *testing.T) { genericContext.InternalInformerFactory.Config().V1alpha1().KatalystCustomConfigs(), ) - kcc, err := NewKatalystCustomConfigTargetController( + targetController, err := NewKatalystCustomConfigTargetController( ctx, conf.GenericConfiguration, conf.GenericControllerConfiguration, @@ -229,9 +330,23 @@ func TestKatalystCustomConfigTargetController_Run(t *testing.T) { genericContext.StartInformer(ctx) go targetHandler.Run() - go kcc.Run() + go targetController.Run() - cache.WaitForCacheSync(kcc.ctx.Done(), kcc.syncedFunc...) + cache.WaitForCacheSync(targetController.ctx.Done(), targetController.syncedFunc...) + time.Sleep(1 * time.Second) + + for _, kcc := range tt.args.kccListChanged { + _, err := genericContext.Client.InternalClient.ConfigV1alpha1().CustomNodeConfigs().Update(ctx, kcc.(*v1alpha1.CustomNodeConfig), metav1.UpdateOptions{}) + assert.NoError(t, err) + } + time.Sleep(1 * time.Second) + + for _, kccTarget := range tt.args.kccTargetListChanged { + gvr, _ := meta.UnsafeGuessKindToResource(kccTarget.GetObjectKind().GroupVersionKind()) + obj := toTestUnstructured(kccTarget) + _, err := genericContext.Client.DynamicClient.Resource(gvr).Namespace(obj.GetNamespace()).Update(ctx, obj, metav1.UpdateOptions{}) + assert.NoError(t, err) + } time.Sleep(1 * time.Second) }) } diff --git a/pkg/controller/kcc/util/kcct.go b/pkg/controller/kcc/util/kcct.go index b0ede16c09..5021531169 100644 --- a/pkg/controller/kcc/util/kcct.go +++ b/pkg/controller/kcc/util/kcct.go @@ -39,10 +39,10 @@ import ( "github.com/kubewharf/katalyst-core/pkg/util/native" ) -// GetRelatedCNCForTargetConfig get current related cnc for the config obey follow rule: -// 1. only updated and deleting config will be concerned, which means that ObservedGeneration and Generation is equal, to make sure the config has been checked by kcc controller -// 2. only valid config will be concerned, because invalid config cover cnc should keep current state -// 3. select related cnc by the config's labelSelector or nodeNames +// GetRelatedCNCForTargetConfig retrieves the currently related CNCs for the configuration based on the following rules: +// 1. Only configurations that have been updated or marked for deletion are considered. This is determined by checking if ObservedGeneration and Generation are equal, ensuring that the configuration has been validated by the KCC controller. +// 2. Only valid configurations are considered, as invalid configurations may affect the associated CNCs and should maintain their current state. +// 3. All CNCs are selected because modifying the target resource can involve more CNCs than those initially selected for the target resource. func GetRelatedCNCForTargetConfig(customNodeConfigLister v1alpha1.CustomNodeConfigLister, unstructured *unstructured.Unstructured) ([]*apisv1alpha1.CustomNodeConfig, error) { targetResource := util.ToKCCTargetResource(unstructured) if !targetResource.IsUpdated() && targetResource.GetDeletionTimestamp() == nil { @@ -53,36 +53,6 @@ func GetRelatedCNCForTargetConfig(customNodeConfigLister v1alpha1.CustomNodeConf return nil, nil } - labelSelector := targetResource.GetLabelSelector() - if labelSelector != "" { - selector, err := labels.Parse(labelSelector) - if err != nil { - return nil, fmt.Errorf("obj %s is valid but parse labelSelector failed", - native.GenerateUniqObjectNameKey(targetResource)) - } - - customNodeConfigList, err := customNodeConfigLister.List(selector) - if err != nil { - return nil, err - } - - return customNodeConfigList, nil - } - - nodeNames := targetResource.GetNodeNames() - if len(nodeNames) > 0 { - var relatedCustomNodeConfig []*apisv1alpha1.CustomNodeConfig - for _, nodeName := range nodeNames { - cnc, err := customNodeConfigLister.Get(nodeName) - if err != nil { - return nil, err - } - - relatedCustomNodeConfig = append(relatedCustomNodeConfig, cnc) - } - return relatedCustomNodeConfig, nil - } - return customNodeConfigLister.List(labels.Everything()) }