Merge pull request #599 from mortent/DestoryerBuilder

Use builder for creating the destroyer

Author: k8s-ci-robot

cmd/destroy/cmddestroy.go

@@ -118,7 +118,10 @@ func (r *Runner) RunE(cmd *cobra.Command, args []string) error {
 	if err != nil {
 		return err
 	}
-	d, err := apply.NewDestroyer(r.factory, invClient)
+	d, err := apply.NewDestroyerBuilder().
+		WithFactory(r.factory).
+		WithInventoryClient(invClient).
+		Build()
 	if err != nil {
 		return err
 	}

cmd/preview/cmdpreview.go

@@ -156,7 +156,10 @@ func (r *Runner) RunE(cmd *cobra.Command, args []string) error {
 			InventoryPolicy:   inventoryPolicy,
 		})
 	} else {
-		d, err := apply.NewDestroyer(r.factory, invClient)
+		d, err := apply.NewDestroyerBuilder().
+			WithFactory(r.factory).
+			WithInventoryClient(invClient).
+			Build()
 		if err != nil {
 			return err
 		}

pkg/apply/applier_builder.go

@@ -4,9 +4,6 @@
 package apply
 
 import (
-	"errors"
-	"fmt"
-
 	"k8s.io/apimachinery/pkg/api/meta"
 	"k8s.io/cli-runtime/pkg/resource"
 	"k8s.io/client-go/discovery"
@@ -20,15 +17,7 @@ import (
 )
 
 type ApplierBuilder struct {
-	// factory is only used to retrieve things that have not been provided explicitly.
-	factory                      util.Factory
-	invClient                    inventory.Client
-	client                       dynamic.Interface
-	discoClient                  discovery.CachedDiscoveryInterface
-	mapper                       meta.RESTMapper
-	restConfig                   *rest.Config
-	unstructuredClientForMapping func(*meta.RESTMapping) (resource.RESTClient, error)
-	statusWatcher                watcher.StatusWatcher
+	commonBuilder
 }
 
 // NewApplierBuilder returns a new ApplierBuilder.
@@ -58,60 +47,6 @@ func (b *ApplierBuilder) Build() (*Applier, error) {
 	}, nil
 }
 
-func (b *ApplierBuilder) finalize() (*ApplierBuilder, error) {
-	bx := *b // make a copy before mutating any fields. Shallow copy is good enough.
-	var err error
-	if bx.invClient == nil {
-		return nil, errors.New("inventory client must be provided")
-	}
-	if bx.client == nil {
-		if bx.factory == nil {
-			return nil, fmt.Errorf("a factory must be provided or all other options: %v", err)
-		}
-		bx.client, err = bx.factory.DynamicClient()
-		if err != nil {
-			return nil, fmt.Errorf("error getting dynamic client: %v", err)
-		}
-	}
-	if bx.discoClient == nil {
-		if bx.factory == nil {
-			return nil, fmt.Errorf("a factory must be provided or all other options: %v", err)
-		}
-		bx.discoClient, err = bx.factory.ToDiscoveryClient()
-		if err != nil {
-			return nil, fmt.Errorf("error getting discovery client: %v", err)
-		}
-	}
-	if bx.mapper == nil {
-		if bx.factory == nil {
-			return nil, fmt.Errorf("a factory must be provided or all other options: %v", err)
-		}
-		bx.mapper, err = bx.factory.ToRESTMapper()
-		if err != nil {
-			return nil, fmt.Errorf("error getting rest mapper: %v", err)
-		}
-	}
-	if bx.restConfig == nil {
-		if bx.factory == nil {
-			return nil, fmt.Errorf("a factory must be provided or all other options: %v", err)
-		}
-		bx.restConfig, err = bx.factory.ToRESTConfig()
-		if err != nil {
-			return nil, fmt.Errorf("error getting rest config: %v", err)
-		}
-	}
-	if bx.unstructuredClientForMapping == nil {
-		if bx.factory == nil {
-			return nil, fmt.Errorf("a factory must be provided or all other options: %v", err)
-		}
-		bx.unstructuredClientForMapping = bx.factory.UnstructuredClientForMapping
-	}
-	if bx.statusWatcher == nil {
-		bx.statusWatcher = watcher.NewDefaultStatusWatcher(bx.client, bx.mapper)
-	}
-	return &bx, nil
-}
-
 func (b *ApplierBuilder) WithFactory(factory util.Factory) *ApplierBuilder {
 	b.factory = factory
 	return b

pkg/apply/builder.go

@@ -0,0 +1,84 @@
+// Copyright 2022 The Kubernetes Authors.
+// SPDX-License-Identifier: Apache-2.0
+
+package apply
+
+import (
+	"errors"
+	"fmt"
+
+	"k8s.io/apimachinery/pkg/api/meta"
+	"k8s.io/cli-runtime/pkg/resource"
+	"k8s.io/client-go/discovery"
+	"k8s.io/client-go/dynamic"
+	"k8s.io/client-go/rest"
+	"k8s.io/kubectl/pkg/cmd/util"
+	"sigs.k8s.io/cli-utils/pkg/inventory"
+	"sigs.k8s.io/cli-utils/pkg/kstatus/watcher"
+)
+
+type commonBuilder struct {
+	// factory is only used to retrieve things that have not been provided explicitly.
+	factory                      util.Factory
+	invClient                    inventory.Client
+	client                       dynamic.Interface
+	discoClient                  discovery.CachedDiscoveryInterface
+	mapper                       meta.RESTMapper
+	restConfig                   *rest.Config
+	unstructuredClientForMapping func(*meta.RESTMapping) (resource.RESTClient, error)
+	statusWatcher                watcher.StatusWatcher
+}
+
+func (cb *commonBuilder) finalize() (*commonBuilder, error) {
+	cx := *cb // make a copy before mutating any fields. Shallow copy is good enough.
+	var err error
+	if cx.invClient == nil {
+		return nil, errors.New("inventory client must be provided")
+	}
+	if cx.client == nil {
+		if cx.factory == nil {
+			return nil, fmt.Errorf("a factory must be provided or all other options: %v", err)
+		}
+		cx.client, err = cx.factory.DynamicClient()
+		if err != nil {
+			return nil, fmt.Errorf("error getting dynamic client: %v", err)
+		}
+	}
+	if cx.discoClient == nil {
+		if cx.factory == nil {
+			return nil, fmt.Errorf("a factory must be provided or all other options: %v", err)
+		}
+		cx.discoClient, err = cx.factory.ToDiscoveryClient()
+		if err != nil {
+			return nil, fmt.Errorf("error getting discovery client: %v", err)
+		}
+	}
+	if cx.mapper == nil {
+		if cx.factory == nil {
+			return nil, fmt.Errorf("a factory must be provided or all other options: %v", err)
+		}
+		cx.mapper, err = cx.factory.ToRESTMapper()
+		if err != nil {
+			return nil, fmt.Errorf("error getting rest mapper: %v", err)
+		}
+	}
+	if cx.restConfig == nil {
+		if cx.factory == nil {
+			return nil, fmt.Errorf("a factory must be provided or all other options: %v", err)
+		}
+		cx.restConfig, err = cx.factory.ToRESTConfig()
+		if err != nil {
+			return nil, fmt.Errorf("error getting rest config: %v", err)
+		}
+	}
+	if cx.unstructuredClientForMapping == nil {
+		if cx.factory == nil {
+			return nil, fmt.Errorf("a factory must be provided or all other options: %v", err)
+		}
+		cx.unstructuredClientForMapping = cx.factory.UnstructuredClientForMapping
+	}
+	if cx.statusWatcher == nil {
+		cx.statusWatcher = watcher.NewDefaultStatusWatcher(cx.client, cx.mapper)
+	}
+	return &cx, nil
+}

pkg/apply/common_test.go

@@ -109,7 +109,10 @@ func newTestDestroyer(
 
 	invClient := newTestInventory(t, tf)
 
-	destroyer, err := NewDestroyer(tf, invClient)
+	destroyer, err := NewDestroyerBuilder().
+		WithFactory(tf).
+		WithInventoryClient(invClient).
+		Build()
 	require.NoError(t, err)
 	destroyer.statusWatcher = statusWatcher
 

pkg/apply/destroyer.go

@@ -8,9 +8,11 @@ import (
 	"fmt"
 	"time"
 
+	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/client-go/discovery"
+	"k8s.io/client-go/dynamic"
 	"k8s.io/klog/v2"
-	cmdutil "k8s.io/kubectl/pkg/cmd/util"
 	"sigs.k8s.io/cli-utils/pkg/apis/actuation"
 	"sigs.k8s.io/cli-utils/pkg/apply/cache"
 	"sigs.k8s.io/cli-utils/pkg/apply/event"
@@ -26,41 +28,16 @@ import (
 	"sigs.k8s.io/cli-utils/pkg/object/validation"
 )
 
-// NewDestroyer returns a new destroyer. It will set up the ApplyOptions and
-// PruneOptions which are responsible for capturing any command line flags.
-// It currently requires IOStreams, but this is a legacy from when
-// the ApplyOptions were responsible for printing progress. This is now
-// handled by a separate printer with the KubectlPrinterAdapter bridging
-// between the two.
-func NewDestroyer(factory cmdutil.Factory, invClient inventory.Client) (*Destroyer, error) {
-	pruner, err := prune.NewPruner(factory, invClient)
-	if err != nil {
-		return nil, fmt.Errorf("error setting up PruneOptions: %w", err)
-	}
-	client, err := factory.DynamicClient()
-	if err != nil {
-		return nil, err
-	}
-	mapper, err := factory.ToRESTMapper()
-	if err != nil {
-		return nil, err
-	}
-	statusWatcher := watcher.NewDefaultStatusWatcher(client, mapper)
-	return &Destroyer{
-		pruner:        pruner,
-		statusWatcher: statusWatcher,
-		factory:       factory,
-		invClient:     invClient,
-	}, nil
-}
-
 // Destroyer performs the step of grabbing all the previous inventory objects and
 // prune them. This also deletes all the previous inventory objects
 type Destroyer struct {
 	pruner        *prune.Pruner
 	statusWatcher watcher.StatusWatcher
-	factory       cmdutil.Factory
 	invClient     inventory.Client
+	mapper        meta.RESTMapper
+	client        dynamic.Interface
+	openAPIGetter discovery.OpenAPISchemaInterface
+	infoHelper    info.Helper
 }
 
 type DestroyerOptions struct {
@@ -112,18 +89,13 @@ func (d *Destroyer) Run(ctx context.Context, invInfo inventory.Info, options Des
 			handleError(eventChannel, err)
 			return
 		}
-		mapper, err := d.factory.ToRESTMapper()
-		if err != nil {
-			handleError(eventChannel, err)
-			return
-		}
 
 		// Validate the resources to make sure we catch those problems early
 		// before anything has been updated in the cluster.
 		vCollector := &validation.Collector{}
 		validator := &validation.Validator{
 			Collector: vCollector,
-			Mapper:    mapper,
+			Mapper:    d.mapper,
 		}
 		validator.Validate(deleteObjs)
 
@@ -132,11 +104,6 @@ func (d *Destroyer) Run(ctx context.Context, invInfo inventory.Info, options Des
 		taskContext := taskrunner.NewTaskContext(eventChannel, resourceCache)
 
 		klog.V(4).Infoln("destroyer building task queue...")
-		dynamicClient, err := d.factory.DynamicClient()
-		if err != nil {
-			handleError(eventChannel, err)
-			return
-		}
 		deleteFilters := []filter.ValidationFilter{
 			filter.PreventRemoveFilter{},
 			filter.InventoryPolicyPruneFilter{
@@ -151,10 +118,10 @@ func (d *Destroyer) Run(ctx context.Context, invInfo inventory.Info, options Des
 		}
 		taskBuilder := &solver.TaskQueueBuilder{
 			Pruner:        d.pruner,
-			DynamicClient: dynamicClient,
-			OpenAPIGetter: d.factory.OpenAPIGetter(),
-			InfoHelper:    info.NewHelper(mapper, d.factory.UnstructuredClientForMapping),
-			Mapper:        mapper,
+			DynamicClient: d.client,
+			OpenAPIGetter: d.openAPIGetter,
+			InfoHelper:    d.infoHelper,
+			Mapper:        d.mapper,
 			InvClient:     d.invClient,
 			Collector:     vCollector,
 			PruneFilters:  deleteFilters,