From 0cdf632cdc1777616705ba46502399b1825326de Mon Sep 17 00:00:00 2001 From: Sean Sullivan Date: Tue, 29 Jun 2021 22:50:35 -0700 Subject: [PATCH] remove crd-specific code in apply tasks --- pkg/apply/task/apply_task.go | 122 +----------------------------- pkg/apply/task/apply_task_test.go | 66 ++++------------ pkg/apply/task/prune_task.go | 2 + 3 files changed, 16 insertions(+), 174 deletions(-) diff --git a/pkg/apply/task/apply_task.go b/pkg/apply/task/apply_task.go index 71957c55..6fa65734 100644 --- a/pkg/apply/task/apply_task.go +++ b/pkg/apply/task/apply_task.go @@ -20,7 +20,6 @@ import ( "k8s.io/kubectl/pkg/cmd/apply" cmddelete "k8s.io/kubectl/pkg/cmd/delete" "k8s.io/kubectl/pkg/cmd/util" - "k8s.io/kubectl/pkg/util/slice" applyerror "sigs.k8s.io/cli-utils/pkg/apply/error" "sigs.k8s.io/cli-utils/pkg/apply/event" "sigs.k8s.io/cli-utils/pkg/apply/info" @@ -52,7 +51,6 @@ type ApplyTask struct { InfoHelper info.InfoHelper Mapper meta.RESTMapper Objects []*unstructured.Unstructured - CRDs []*unstructured.Unstructured // Used for determining inventory during errors PrevInventory map[object.ObjMetadata]bool DryRunStrategy common.DryRunStrategy @@ -91,43 +89,7 @@ func (a *ApplyTask) Identifiers() []object.ObjMetadata { func (a *ApplyTask) Start(taskContext *taskrunner.TaskContext) { go func() { objects := a.Objects - klog.V(4).Infof("apply task starting; attempting to apply %d objects", len(objects)) - - // If this is a dry run, we need to handle situations where - // we have a CRD and a CR in the same resource set, but the CRD - // will not actually have been applied when we reach the CR. - if a.DryRunStrategy.ClientOrServerDryRun() { - klog.V(4).Infof("dry-run filtering custom resources...") - // Find all resources in the set that doesn't exist in the - // RESTMapper, but where we do have the CRD for the type in - // the resource set. - objs, objsWithCRD, err := a.filterCRsWithCRDInSet(objects) - if err != nil { - sendBatchApplyEvents(taskContext, objs, err) - a.sendTaskResult(taskContext) - return - } - - // Just send the apply event here. We know it must be a - // Created event since the type didn't already exist in the - // cluster. - for _, obj := range objsWithCRD { - taskContext.EventChannel() <- createApplyEvent(object.UnstructuredToObjMetaOrDie(obj), event.Created, nil) - } - // Update the resource set to no longer include the CRs. - klog.V(4).Infof("after dry-run filtering custom resources, %d objects left", len(objs)) - objects = objs - } - - // ApplyOptions doesn't allow an empty set of resources, so check - // for that here. It could happen if this is dry-run and we removed - // all resources in the previous step. - if len(objects) == 0 { - klog.V(4).Infoln("no objects to apply after dry-run filtering--returning") - a.sendTaskResult(taskContext) - return - } - + klog.V(2).Infof("apply task starting (%d objects)", len(objects)) // Create a new instance of the applyOptions interface and use it // to apply the objects. ao, dynamic, err := applyOptionsFactoryFunc(taskContext.EventChannel(), @@ -291,40 +253,6 @@ func (a *ApplyTask) sendTaskResult(taskContext *taskrunner.TaskContext) { taskContext.TaskChannel() <- taskrunner.TaskResult{} } -// filterCRsWithCRDInSet loops through all the resources and filters out the -// resources that doesn't exist in the RESTMapper, but where we do have a CRD -// in the resource set that defines the needed type. It returns two slices, -// the seconds contains the resources that meets the above criteria while the -// first slice contains the remaining resources. -func (a *ApplyTask) filterCRsWithCRDInSet(objects []*unstructured.Unstructured) ([]*unstructured.Unstructured, []*unstructured.Unstructured, error) { - var objs []*unstructured.Unstructured - var objsWithCRD []*unstructured.Unstructured - - crdsInfo := buildCRDsInfo(a.CRDs) - for _, obj := range objects { - gvk := obj.GroupVersionKind() - - // First check if we find the type in the RESTMapper. - //TODO: Maybe we do care if there is a new version of the CRD? - _, err := a.Mapper.RESTMapping(gvk.GroupKind()) - if err != nil && !meta.IsNoMatchError(err) { - return objs, objsWithCRD, err - } - - // If we can't find the type in the RESTMapper, but we do have the - // CRD in the set of resources, filter out the object. - if meta.IsNoMatchError(err) && crdsInfo.includesCRDForCR(obj) { - objsWithCRD = append(objsWithCRD, obj) - continue - } - - // If the resource is in the RESTMapper, or it is not there but we - // also don't have the CRD, just keep the resource. - objs = append(objs, obj) - } - return objs, objsWithCRD, nil -} - // objInCluster returns true if the passed object is in the slice of // previous inventory, because an object in the previous inventory // exists in the cluster. @@ -335,54 +263,6 @@ func (a *ApplyTask) objInCluster(obj object.ObjMetadata) bool { return false } -type crdsInfo struct { - crds []crdInfo -} - -// includesCRDForCR checks if we have information about a CRD that defines -// the types needed for the provided CR. -func (c *crdsInfo) includesCRDForCR(cr *unstructured.Unstructured) bool { - gvk := cr.GroupVersionKind() - for _, crd := range c.crds { - if gvk.Group == crd.group && - gvk.Kind == crd.kind && - slice.ContainsString(crd.versions, gvk.Version, nil) { - return true - } - } - return false -} - -type crdInfo struct { - group string - kind string - versions []string -} - -func buildCRDsInfo(crds []*unstructured.Unstructured) *crdsInfo { - var crdsInf []crdInfo - for _, crd := range crds { - group, _, _ := unstructured.NestedString(crd.Object, "spec", "group") - kind, _, _ := unstructured.NestedString(crd.Object, "spec", "names", "kind") - - var versions []string - crdVersions, _, _ := unstructured.NestedSlice(crd.Object, "spec", "versions") - for _, ver := range crdVersions { - verObj := ver.(map[string]interface{}) - version, _, _ := unstructured.NestedString(verObj, "name") - versions = append(versions, version) - } - crdsInf = append(crdsInf, crdInfo{ - kind: kind, - group: group, - versions: versions, - }) - } - return &crdsInfo{ - crds: crdsInf, - } -} - // ClearTimeout is not supported by the ApplyTask. func (a *ApplyTask) ClearTimeout() {} diff --git a/pkg/apply/task/apply_task_test.go b/pkg/apply/task/apply_task_test.go index 15b31e83..5ef04fea 100644 --- a/pkg/apply/task/apply_task_test.go +++ b/pkg/apply/task/apply_task_test.go @@ -349,11 +349,10 @@ func TestApplyTask_FetchGeneration(t *testing.T) { func TestApplyTask_DryRun(t *testing.T) { testCases := map[string]struct { objs []*unstructured.Unstructured - crds []*unstructured.Unstructured expectedObjects []object.ObjMetadata expectedEvents []event.Event }{ - "dry run with no CRDs or CRs": { + "simple dry run": { objs: []*unstructured.Unstructured{ toUnstructured(map[string]interface{}{ "apiVersion": "apps/v1", @@ -377,7 +376,7 @@ func TestApplyTask_DryRun(t *testing.T) { expectedEvents: []event.Event{}, }, "dry run with CRD and CR": { - crds: []*unstructured.Unstructured{ + objs: []*unstructured.Unstructured{ toUnstructured(map[string]interface{}{ "apiVersion": "apiextensions.k8s.io/v1", "kind": "CustomResourceDefinition", @@ -396,8 +395,6 @@ func TestApplyTask_DryRun(t *testing.T) { }, }, }), - }, - objs: []*unstructured.Unstructured{ toUnstructured(map[string]interface{}{ "apiVersion": "custom.io/v1alpha1", "kind": "Custom", @@ -406,52 +403,13 @@ func TestApplyTask_DryRun(t *testing.T) { }, }), }, - expectedObjects: []object.ObjMetadata{}, - expectedEvents: []event.Event{ - { - Type: event.ApplyType, - }, - }, - }, - "dry run with CRD and CR and CRD already installed": { - crds: []*unstructured.Unstructured{ - toUnstructured(map[string]interface{}{ - "apiVersion": "apiextensions.k8s.io/v1", - "kind": "CustomResourceDefinition", - "metadata": map[string]interface{}{ - "name": "foo", - }, - "spec": map[string]interface{}{ - "group": "anothercustom.io", - "names": map[string]interface{}{ - "kind": "AnotherCustom", - }, - "versions": []interface{}{ - map[string]interface{}{ - "name": "v2", - }, - }, - }, - }), - }, - objs: []*unstructured.Unstructured{ - toUnstructured(map[string]interface{}{ - "apiVersion": "anothercustom.io/v2", - "kind": "AnotherCustom", - "metadata": map[string]interface{}{ - "name": "bar", - "namespace": "barbar", - }, - }), - }, expectedObjects: []object.ObjMetadata{ { GroupKind: schema.GroupKind{ - Group: "anothercustom.io", - Kind: "AnotherCustom", + Group: "custom.io", + Kind: "Custom", }, - Name: "bar", - Namespace: "barbar", + Name: "bar", }, }, expectedEvents: []event.Event{}, @@ -490,7 +448,6 @@ func TestApplyTask_DryRun(t *testing.T) { InfoHelper: &fakeInfoHelper{}, Mapper: restMapper, DryRunStrategy: drs, - CRDs: tc.crds, InvInfo: &fakeInventoryInfo{}, } @@ -530,12 +487,11 @@ func TestApplyTask_DryRun(t *testing.T) { func TestApplyTaskWithError(t *testing.T) { testCases := map[string]struct { objs []*unstructured.Unstructured - crds []*unstructured.Unstructured expectedObjects []object.ObjMetadata expectedEvents []event.Event }{ "some resources have apply error": { - crds: []*unstructured.Unstructured{ + objs: []*unstructured.Unstructured{ toUnstructured(map[string]interface{}{ "apiVersion": "apiextensions.k8s.io/v1", "kind": "CustomResourceDefinition", @@ -554,8 +510,6 @@ func TestApplyTaskWithError(t *testing.T) { }, }, }), - }, - objs: []*unstructured.Unstructured{ toUnstructured(map[string]interface{}{ "apiVersion": "anothercustom.io/v2", "kind": "AnotherCustom", @@ -574,6 +528,13 @@ func TestApplyTaskWithError(t *testing.T) { }), }, expectedObjects: []object.ObjMetadata{ + { + GroupKind: schema.GroupKind{ + Group: "apiextensions.k8s.io", + Kind: "CustomResourceDefinition", + }, + Name: "foo", + }, { GroupKind: schema.GroupKind{ Group: "anothercustom.io", @@ -625,7 +586,6 @@ func TestApplyTaskWithError(t *testing.T) { InfoHelper: &fakeInfoHelper{}, Mapper: restMapper, DryRunStrategy: drs, - CRDs: tc.crds, InvInfo: &fakeInventoryInfo{}, } diff --git a/pkg/apply/task/prune_task.go b/pkg/apply/task/prune_task.go index b86728fe..ee6db475 100644 --- a/pkg/apply/task/prune_task.go +++ b/pkg/apply/task/prune_task.go @@ -6,6 +6,7 @@ package task import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/klog/v2" "sigs.k8s.io/cli-utils/pkg/apply/event" "sigs.k8s.io/cli-utils/pkg/apply/filter" "sigs.k8s.io/cli-utils/pkg/apply/prune" @@ -52,6 +53,7 @@ func (p *PruneTask) Identifiers() []object.ObjMetadata { // to signal to the taskrunner that the task has completed (or failed). func (p *PruneTask) Start(taskContext *taskrunner.TaskContext) { go func() { + klog.V(2).Infof("prune task starting (%d objects)", len(p.Objects)) // Create filter to prevent deletion of currently applied // objects. Must be done here to wait for applied UIDs. uidFilter := filter.CurrentUIDFilter{