Merge pull request kubernetes-sigs#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,