Skip to content

Commit 1563909

Browse files
Liujingfang1Liujingfang1
committed
improve PruneTask/ApplyTask task logic for inventory policy and add unit
test
1 parent 73e29da commit 1563909

File tree

4 files changed

+244
-20
lines changed

4 files changed

+244
-20
lines changed

‎pkg/apply/prune/prune.go

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -144,8 +144,7 @@ func (po *PruneOptions) Prune(localInv inventory.InventoryInfo, localObjs []*uns
144144
continue
145145
}
146146
// Handle lifecycle directive preventing deletion.
147-
if preventDeleteAnnotation(metadata.GetAnnotations()) {
148-
klog.V(4).Infof("prune object lifecycle directive; do not prune: %s", uid)
147+
if !canPrune(localInv, obj, o.InventoryPolicy, uid) {
149148
eventChannel <- createPruneEvent(clusterObj, obj, event.PruneSkipped)
150149
localIds = append(localIds, clusterObj)
151150
continue
@@ -161,13 +160,6 @@ func (po *PruneOptions) Prune(localInv inventory.InventoryInfo, localObjs []*uns
161160
continue
162161
}
163162
}
164-
if !inventory.CanPrune(localInv, obj, o.InventoryPolicy) {
165-
klog.V(4).Infof("skip pruning object that doesn't belong to current inventory: %s/%s",
166-
clusterObj.Namespace, clusterObj.Name)
167-
eventChannel <- createPruneEvent(clusterObj, obj, event.PruneSkipped)
168-
localIds = append(localIds, clusterObj)
169-
continue
170-
}
171163
if !o.DryRunStrategy.ClientOrServerDryRun() {
172164
klog.V(4).Infof("prune object delete: %s/%s", clusterObj.Namespace, clusterObj.Name)
173165
err = namespacedClient.Delete(context.TODO(), clusterObj.Name, metav1.DeleteOptions{})
@@ -231,3 +223,16 @@ func createPruneFailedEvent(objMeta object.ObjMetadata, err error) event.Event {
231223
},
232224
}
233225
}
226+
227+
func canPrune(localInv inventory.InventoryInfo, obj *unstructured.Unstructured,
228+
policy inventory.InventoryPolicy, uid string) bool {
229+
if !inventory.CanPrune(localInv, obj, policy) {
230+
klog.V(4).Infof("skip pruning object that doesn't belong to current inventory: %s", uid)
231+
return false
232+
}
233+
if preventDeleteAnnotation(obj.GetAnnotations()) {
234+
klog.V(4).Infof("prune object lifecycle directive; do not prune: %s", uid)
235+
return false
236+
}
237+
return true
238+
}

‎pkg/apply/task/apply_task.go

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -130,16 +130,21 @@ func (a *ApplyTask) Start(taskContext *taskrunner.TaskContext) {
130130
clusterObj, err := getClusterObj(dynamic, info)
131131
if err != nil {
132132
if !apierrors.IsNotFound(err) {
133-
canApply, err := inventory.CanApply(a.InvInfo, clusterObj, a.InventoryPolicy)
134-
if !canApply {
135-
taskContext.EventChannel() <- createApplyEvent(
136-
object.UnstructuredToObjMeta(obj),
137-
event.Unchanged,
138-
err)
139-
continue
140-
}
133+
taskContext.EventChannel() <- createApplyEvent(
134+
object.UnstructuredToObjMeta(obj),
135+
event.Unchanged,
136+
err)
137+
continue
141138
}
142139
}
140+
canApply, err := inventory.CanApply(a.InvInfo, clusterObj, a.InventoryPolicy)
141+
if !canApply {
142+
taskContext.EventChannel() <- createApplyEvent(
143+
object.UnstructuredToObjMeta(obj),
144+
event.Unchanged,
145+
err)
146+
continue
147+
}
143148
// add the inventory annotation to the resource being applied.
144149
inventory.AddInventoryIDAnnotation(obj, a.InvInfo)
145150
infos = append(infos, info)

‎pkg/apply/task/apply_task_test.go

Lines changed: 216 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import (
1919
"sigs.k8s.io/cli-utils/pkg/apply/event"
2020
"sigs.k8s.io/cli-utils/pkg/apply/taskrunner"
2121
"sigs.k8s.io/cli-utils/pkg/common"
22+
"sigs.k8s.io/cli-utils/pkg/inventory"
2223
"sigs.k8s.io/cli-utils/pkg/object"
2324
"sigs.k8s.io/cli-utils/pkg/testutil"
2425
)
@@ -253,7 +254,7 @@ func TestApplyTask_DryRun(t *testing.T) {
253254
}
254255
defer func() { applyOptionsFactoryFunc = oldAO }()
255256
getClusterObj = func(d dynamic.Interface, info *resource.Info) (*unstructured.Unstructured, error) {
256-
return tc.objs[0], nil
257+
return addOwningInventory(tc.objs[0], "id"), nil
257258
}
258259

259260
applyTask := &ApplyTask{
@@ -389,7 +390,7 @@ func TestApplyTaskWithError(t *testing.T) {
389390
defer func() { applyOptionsFactoryFunc = oldAO }()
390391

391392
getClusterObj = func(d dynamic.Interface, info *resource.Info) (*unstructured.Unstructured, error) {
392-
return tc.objs[0], nil
393+
return addOwningInventory(tc.objs[0], "id"), nil
393394
}
394395
applyTask := &ApplyTask{
395396
Objects: tc.objs,
@@ -433,6 +434,187 @@ func TestApplyTaskWithError(t *testing.T) {
433434
}
434435
}
435436

437+
var deployment = toUnstructured(map[string]interface{}{
438+
"apiVersion": "apps/v1",
439+
"kind": "Deployment",
440+
"metadata": map[string]interface{}{
441+
"name": "deploy",
442+
"namespace": "default",
443+
},
444+
})
445+
446+
var deploymentObjMetadata = []object.ObjMetadata{
447+
{
448+
GroupKind: schema.GroupKind{
449+
Group: "apps",
450+
Kind: "Deployment",
451+
},
452+
Name: "deploy",
453+
Namespace: "default",
454+
},
455+
}
456+
457+
func TestApplyTaskWithDifferentInventoryAnnotation(t *testing.T) {
458+
testCases := map[string]struct {
459+
obj *unstructured.Unstructured
460+
clusterObj *unstructured.Unstructured
461+
policy inventory.InventoryPolicy
462+
expectedObjects []object.ObjMetadata
463+
expectedEvents []event.Event
464+
}{
465+
"InventoryPolicyMustMatch with object doesn't exist on cluster - Can Apply": {
466+
obj: deployment,
467+
clusterObj: nil,
468+
policy: inventory.InventoryPolicyMustMatch,
469+
expectedObjects: deploymentObjMetadata,
470+
expectedEvents: []event.Event{},
471+
},
472+
"InventoryPolicyMustMatch with object annotation is empty - Can't Apply": {
473+
obj: deployment,
474+
clusterObj: removeOwningInventory(deployment),
475+
policy: inventory.InventoryPolicyMustMatch,
476+
expectedObjects: nil,
477+
expectedEvents: []event.Event{
478+
{
479+
Type: event.ApplyType,
480+
ApplyEvent: event.ApplyEvent{
481+
Error: inventory.NewNeedAdoptionError(
482+
fmt.Errorf("can't adopt an object without the annotation config.k8s.io/owning-inventory")),
483+
},
484+
},
485+
},
486+
},
487+
"InventoryPolicyMustMatch with object annotation doesn't match - Can't Apply": {
488+
obj: deployment,
489+
clusterObj: addOwningInventory(deployment, "unmatchd"),
490+
policy: inventory.InventoryPolicyMustMatch,
491+
expectedObjects: nil,
492+
expectedEvents: []event.Event{
493+
{
494+
Type: event.ApplyType,
495+
ApplyEvent: event.ApplyEvent{
496+
Error: inventory.NewInventoryOverlapError(
497+
fmt.Errorf("can't apply the resource since its annotation config.k8s.io/owning-inventory is a different inventory object")),
498+
},
499+
},
500+
},
501+
},
502+
"InventoryPolicyMustMatch with object annotation matches - Can Apply": {
503+
obj: deployment,
504+
clusterObj: addOwningInventory(deployment, "id"),
505+
policy: inventory.InventoryPolicyMustMatch,
506+
expectedObjects: deploymentObjMetadata,
507+
expectedEvents: nil,
508+
},
509+
"AdoptIfNoInventory with object doesn't exist on cluster - Can Apply": {
510+
obj: deployment,
511+
clusterObj: nil,
512+
policy: inventory.AdoptIfNoInventory,
513+
expectedObjects: deploymentObjMetadata,
514+
expectedEvents: []event.Event{},
515+
},
516+
"AdoptIfNoInventory with object annotation is empty - Can Apply": {
517+
obj: deployment,
518+
clusterObj: removeOwningInventory(deployment),
519+
policy: inventory.AdoptIfNoInventory,
520+
expectedObjects: deploymentObjMetadata,
521+
expectedEvents: []event.Event{},
522+
},
523+
"AdoptIfNoInventory with object annotation doesn't match - Can't Apply": {
524+
obj: deployment,
525+
clusterObj: addOwningInventory(deployment, "notmatch"),
526+
policy: inventory.AdoptIfNoInventory,
527+
expectedObjects: nil,
528+
expectedEvents: []event.Event{
529+
{
530+
Type: event.ApplyType,
531+
ApplyEvent: event.ApplyEvent{
532+
Error: inventory.NewInventoryOverlapError(
533+
fmt.Errorf("can't apply the resource since its annotation config.k8s.io/owning-inventory is a different inventory object")),
534+
},
535+
},
536+
},
537+
},
538+
"AdoptIfNoInventory with object annotation matches - Can Apply": {
539+
obj: deployment,
540+
clusterObj: addOwningInventory(deployment, "id"),
541+
policy: inventory.AdoptIfNoInventory,
542+
expectedObjects: deploymentObjMetadata,
543+
expectedEvents: []event.Event{},
544+
},
545+
"AdoptAll with object doesn't exist on cluster - Can Apply": {
546+
obj: deployment,
547+
clusterObj: nil,
548+
policy: inventory.AdoptAll,
549+
expectedObjects: deploymentObjMetadata,
550+
expectedEvents: []event.Event{},
551+
},
552+
}
553+
554+
for tn, tc := range testCases {
555+
drs := common.DryRunNone
556+
t.Run(tn, func(t *testing.T) {
557+
eventChannel := make(chan event.Event)
558+
taskContext := taskrunner.NewTaskContext(eventChannel)
559+
560+
restMapper := testutil.NewFakeRESTMapper(schema.GroupVersionKind{
561+
Group: "apps",
562+
Version: "v1",
563+
Kind: "Deployment",
564+
})
565+
566+
ao := &fakeApplyOptions{}
567+
oldAO := applyOptionsFactoryFunc
568+
applyOptionsFactoryFunc = func(chan event.Event, common.ServerSideOptions, common.DryRunStrategy, util.Factory) (applyOptions, dynamic.Interface, error) {
569+
return ao, nil, nil
570+
}
571+
defer func() { applyOptionsFactoryFunc = oldAO }()
572+
573+
getClusterObj = func(d dynamic.Interface, info *resource.Info) (*unstructured.Unstructured, error) {
574+
return tc.clusterObj, nil
575+
}
576+
applyTask := &ApplyTask{
577+
Objects: []*unstructured.Unstructured{tc.obj},
578+
InfoHelper: &fakeInfoHelper{},
579+
Mapper: restMapper,
580+
DryRunStrategy: drs,
581+
InvInfo: &fakeInventoryInfo{},
582+
InventoryPolicy: tc.policy,
583+
}
584+
585+
var events []event.Event
586+
var wg sync.WaitGroup
587+
wg.Add(1)
588+
go func() {
589+
defer wg.Done()
590+
for msg := range eventChannel {
591+
events = append(events, msg)
592+
}
593+
}()
594+
595+
applyTask.Start(taskContext)
596+
<-taskContext.TaskChannel()
597+
close(eventChannel)
598+
wg.Wait()
599+
600+
assert.Equal(t, len(tc.expectedObjects), len(ao.passedObjects))
601+
for i, obj := range ao.passedObjects {
602+
actual, err := object.InfoToObjMeta(obj)
603+
if err != nil {
604+
continue
605+
}
606+
assert.Equal(t, tc.expectedObjects[i], actual)
607+
}
608+
609+
assert.Equal(t, len(tc.expectedEvents), len(events))
610+
for i, e := range events {
611+
assert.Equal(t, tc.expectedEvents[i].Type, e.Type)
612+
assert.Equal(t, tc.expectedEvents[i].ApplyEvent.Error.Error(), e.ApplyEvent.Error.Error())
613+
}
614+
})
615+
}
616+
}
617+
436618
func toUnstructured(obj map[string]interface{}) *unstructured.Unstructured {
437619
return &unstructured.Unstructured{
438620
Object: obj,
@@ -452,6 +634,9 @@ func toUnstructureds(rss []resourceInfo) []*unstructured.Unstructured {
452634
"namespace": rs.namespace,
453635
"uid": string(rs.uid),
454636
"generation": rs.generation,
637+
"annotations": map[string]interface{}{
638+
"config.k8s.io/owning-inventory": "id",
639+
},
455640
},
456641
},
457642
})
@@ -507,3 +692,32 @@ func (fi *fakeInventoryInfo) Namespace() string {
507692
func (fi *fakeInventoryInfo) ID() string {
508693
return "id"
509694
}
695+
696+
func addOwningInventory(obj *unstructured.Unstructured, id string) *unstructured.Unstructured {
697+
if obj == nil {
698+
return nil
699+
}
700+
newObj := obj.DeepCopy()
701+
annotations := newObj.GetAnnotations()
702+
if len(annotations) == 0 {
703+
annotations = make(map[string]string)
704+
}
705+
706+
annotations["config.k8s.io/owning-inventory"] = id
707+
newObj.SetAnnotations(annotations)
708+
return newObj
709+
}
710+
711+
func removeOwningInventory(obj *unstructured.Unstructured) *unstructured.Unstructured {
712+
if obj == nil {
713+
return nil
714+
}
715+
newObj := obj.DeepCopy()
716+
annotations := newObj.GetAnnotations()
717+
if len(annotations) == 0 {
718+
return newObj
719+
}
720+
delete(annotations, "config.k8s.io/owning-inventory")
721+
newObj.SetAnnotations(annotations)
722+
return newObj
723+
}

‎pkg/inventory/policy.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ func CanApply(inv InventoryInfo, obj *unstructured.Unstructured, policy Inventor
9797
if policy != InventoryPolicyMustMatch {
9898
return true, nil
9999
}
100-
err := fmt.Errorf("%v can't adopt an object without the annotation %s", InventoryPolicyMustMatch, owningInventoryKey)
100+
err := fmt.Errorf("can't adopt an object without the annotation %s", owningInventoryKey)
101101
return false, NewNeedAdoptionError(err)
102102
case Match:
103103
return true, nil

0 commit comments

Comments
 (0)