Skip to content

Commit

Permalink
fix kcc and target resource modify no trigger others update
Browse files Browse the repository at this point in the history
  • Loading branch information
@luomingmeng
luomingmeng committed Aug 9, 2023
1 parent cb0f206 commit 63208a6
Show file tree
Hide file tree
Showing 5 changed files with 161 additions and 54 deletions.
4 changes: 2 additions & 2 deletions pkg/controller/kcc/cnc_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
12 changes: 6 additions & 6 deletions pkg/controller/kcc/kcc_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down
22 changes: 22 additions & 0 deletions pkg/controller/kcc/kcct.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
}

Expand Down
139 changes: 127 additions & 12 deletions pkg/controller/kcc/kcct_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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",
Expand All @@ -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",
Expand All @@ -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",
Expand Down Expand Up @@ -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) {
Expand All @@ -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,
Expand All @@ -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)
})
}
Expand Down
38 changes: 4 additions & 34 deletions pkg/controller/kcc/util/kcct.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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())
}

Expand Down

0 comments on commit 63208a6

Please sign in to comment.