Merge pull request #25110 from metral/recursive-rollout

Automatic merge from submit-queue

enable recursive processing in kubectl rollout

This PR was split out of https://github.com/kubernetes/kubernetes/pull/23673 per @deads2k's suggestion: https://github.com/kubernetes/kubernetes/pull/23673#discussion-diff-61290415

It makes use of the recursive processing of a directory in `kubectl rollout {undo,pause,resume}` which now also conforms to the [command conventions](https://github.com/kubernetes/kubernetes/blob/master/docs/devel/kubectl-conventions.md#command-implementation-conventions).

`kubectl rollout history` is being omitted for the time being as its encountering what appears to be an anomaly of an issue: https://github.com/kubernetes/kubernetes/issues/24606. Once this issue has been resolved I will modify it to be recursive

hack/test-cmd.sh

@@ -1073,6 +1073,59 @@ __EOF__
   kube::test::get_object_assert pods "{{range.items}}{{$id_field}}:{{end}}" ''
   kube::test::if_has_string "${output_message}" "Object 'Kind' is missing"
 
+  ### Rollback a deployment
+  # Pre-condition: no deployments exist
+  kube::test::get_object_assert deployment "{{range.items}}{{$id_field}}:{{end}}" ''
+  # Command
+  # Create deployments (revision 1) recursively from directory of YAML files
+  ! kubectl create -f hack/testdata/recursive/deployment --recursive "${kube_flags[@]}"
+  kube::test::get_object_assert deployment "{{range.items}}{{$id_field}}:{{end}}" 'nginx0-deployment:nginx1-deployment:'
+  kube::test::get_object_assert deployment "{{range.items}}{{$deployment_image_field}}:{{end}}" "${IMAGE_NGINX}:${IMAGE_NGINX}:"
+  ## Rollback to revision 1 - should be no-op
+  output_message=$(! kubectl rollout undo -f hack/testdata/recursive/deployment --recursive --to-revision=1 2>&1 "${kube_flags[@]}")
+  kube::test::get_object_assert deployment "{{range.items}}{{$deployment_image_field}}:{{end}}" "${IMAGE_NGINX}:${IMAGE_NGINX}:"
+  kube::test::if_has_string "${output_message}" "Object 'Kind' is missing"
+  ## Pause the deployment
+  output_message=$(! kubectl rollout pause -f hack/testdata/recursive/deployment --recursive 2>&1 "${kube_flags[@]}")
+  kube::test::get_object_assert deployment "{{range.items}}{{.spec.paused}}:{{end}}" "true:true:"
+  kube::test::if_has_string "${output_message}" "Object 'Kind' is missing"
+  ## Resume the deployment
+  output_message=$(! kubectl rollout resume -f hack/testdata/recursive/deployment --recursive 2>&1 "${kube_flags[@]}")
+  kube::test::get_object_assert deployment "{{range.items}}{{.spec.paused}}:{{end}}" "<no value>:<no value>:"
+  kube::test::if_has_string "${output_message}" "Object 'Kind' is missing"
+  output_message=$(! kubectl rollout history -f hack/testdata/recursive/deployment --recursive 2>&1 "${kube_flags[@]}")
+  kube::test::if_has_string "${output_message}" "nginx0-deployment"
+  kube::test::if_has_string "${output_message}" "nginx1-deployment"
+  kube::test::if_has_string "${output_message}" "Object 'Kind' is missing"
+  # Clean up
+  ! kubectl delete -f hack/testdata/recursive/deployment --recursive "${kube_flags[@]}" --grace-period=0
+  sleep 1
+
+  ### Rollback a resource that cannot be rolled back (replication controller)
+  # Pre-condition: no replication controller exists
+  kube::test::get_object_assert rc "{{range.items}}{{$id_field}}:{{end}}" ''
+  # Command
+  # Create replication controllers (revision 1) recursively from directory of YAML files
+  ! kubectl create -f hack/testdata/recursive/rc --recursive "${kube_flags[@]}"
+  kube::test::get_object_assert rc "{{range.items}}{{$id_field}}:{{end}}" 'busybox0:busybox1:'
+  # Command
+  ## Rollback to revision 1 - should be no-op
+  output_message=$(! kubectl rollout undo -f hack/testdata/recursive/rc --recursive --to-revision=1 2>&1 "${kube_flags[@]}")
+  kube::test::if_has_string "${output_message}" 'no rollbacker has been implemented for {"" "ReplicationController"}'
+  kube::test::if_has_string "${output_message}" "Object 'Kind' is missing"
+  ## Pause the deployment
+  output_message=$(! kubectl rollout pause -f hack/testdata/recursive/rc --recursive 2>&1 "${kube_flags[@]}")
+  kube::test::if_has_string "${output_message}" 'error when pausing "hack/testdata/recursive/rc/busybox.yaml'
+  kube::test::if_has_string "${output_message}" 'error when pausing "hack/testdata/recursive/rc/rc/busybox.yaml'
+  kube::test::if_has_string "${output_message}" "Object 'Kind' is missing"
+  ## Resume the deployment
+  output_message=$(! kubectl rollout resume -f hack/testdata/recursive/rc --recursive 2>&1 "${kube_flags[@]}")
+  kube::test::if_has_string "${output_message}" 'error when resuming "hack/testdata/recursive/rc/busybox.yaml'
+  kube::test::if_has_string "${output_message}" 'error when resuming "hack/testdata/recursive/rc/rc/busybox.yaml'
+  kube::test::if_has_string "${output_message}" "Object 'Kind' is missing"
+  # Clean up
+  ! kubectl delete -f hack/testdata/recursive/rc --recursive "${kube_flags[@]}" --grace-period=0
+  sleep 1
 
   ##############
   # Namespaces #

pkg/kubectl/cmd/rollout/rollout_history.go

@@ -23,7 +23,6 @@ import (
 	"k8s.io/kubernetes/pkg/kubectl"
 	cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util"
 	"k8s.io/kubernetes/pkg/kubectl/resource"
-	"k8s.io/kubernetes/pkg/util/errors"
 
 	"github.com/spf13/cobra"
 )
@@ -77,30 +76,31 @@ func RunHistory(f *cmdutil.Factory, cmd *cobra.Command, out io.Writer, args []st
 		return err
 	}
 
-	infos, err := resource.NewBuilder(mapper, typer, resource.ClientMapperFunc(f.ClientForMapping), f.Decoder(true)).
+	r := resource.NewBuilder(mapper, typer, resource.ClientMapperFunc(f.ClientForMapping), f.Decoder(true)).
 		NamespaceParam(cmdNamespace).DefaultNamespace().
 		FilenameParam(enforceNamespace, options.Recursive, options.Filenames...).
 		ResourceTypeOrNameArgs(true, args...).
+		ContinueOnError().
 		Latest().
 		Flatten().
-		Do().
+		Do()
-		Infos()
+	err = r.Err()
 	if err != nil {
 		return err
 	}
 
-	errs := []error{}
+	err = r.Visit(func(info *resource.Info, err error) error {
-	for _, info := range infos {
+		if err != nil {
+			return err
+		}
 		mapping := info.ResourceMapping()
 		historyViewer, err := f.HistoryViewer(mapping)
 		if err != nil {
-			errs = append(errs, err)
+			return err
-			continue
 		}
 		historyInfo, err := historyViewer.History(info.Namespace, info.Name)
 		if err != nil {
-			errs = append(errs, err)
+			return err
-			continue
 		}
 
 		if revisionDetail > 0 {
@@ -115,12 +115,11 @@ func RunHistory(f *cmdutil.Factory, cmd *cobra.Command, out io.Writer, args []st
 			// Print all revisions
 			formattedOutput, printErr := kubectl.PrintRolloutHistory(historyInfo, mapping.Resource, info.Name)
 			if printErr != nil {
-				errs = append(errs, printErr)
+				return printErr
-				continue
 			}
 			fmt.Fprintf(out, "%s\n", formattedOutput)
 		}
-	}
+		return nil
-
+	})
-	return errors.NewAggregate(errs)
+	return err
 }

pkg/kubectl/cmd/rollout/rollout_pause.go

@@ -17,7 +17,6 @@ limitations under the License.
 package rollout
 
 import (
-	"fmt"
 	"io"
 
 	"github.com/spf13/cobra"
@@ -27,6 +26,7 @@ import (
 	cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util"
 	"k8s.io/kubernetes/pkg/kubectl/resource"
 	"k8s.io/kubernetes/pkg/runtime"
+	utilerrors "k8s.io/kubernetes/pkg/util/errors"
 )
 
 // PauseConfig is the start of the data required to perform the operation.  As new fields are added, add them here instead of
@@ -35,7 +35,7 @@ type PauseConfig struct {
 	PauseObject func(object runtime.Object) (bool, error)
 	Mapper      meta.RESTMapper
 	Typer       runtime.ObjectTyper
-	Info        *resource.Info
+	Infos       []*resource.Info
 
 	Out       io.Writer
 	Filenames []string
@@ -64,8 +64,16 @@ func NewCmdRolloutPause(f *cmdutil.Factory, out io.Writer) *cobra.Command {
 		Long:    pause_long,
 		Example: pause_example,
 		Run: func(cmd *cobra.Command, args []string) {
-			cmdutil.CheckErr(opts.CompletePause(f, cmd, out, args))
+			allErrs := []error{}
-			cmdutil.CheckErr(opts.RunPause())
+			err := opts.CompletePause(f, cmd, out, args)
+			if err != nil {
+				allErrs = append(allErrs, err)
+			}
+			err = opts.RunPause()
+			if err != nil {
+				allErrs = append(allErrs, err)
+			}
+			cmdutil.CheckErr(utilerrors.Flatten(utilerrors.NewAggregate(allErrs)))
 		},
 	}
 
@@ -89,32 +97,39 @@ func (o *PauseConfig) CompletePause(f *cmdutil.Factory, cmd *cobra.Command, out
 		return err
 	}
 
-	infos, err := resource.NewBuilder(o.Mapper, o.Typer, resource.ClientMapperFunc(f.ClientForMapping), f.Decoder(true)).
+	r := resource.NewBuilder(o.Mapper, o.Typer, resource.ClientMapperFunc(f.ClientForMapping), f.Decoder(true)).
 		NamespaceParam(cmdNamespace).DefaultNamespace().
 		FilenameParam(enforceNamespace, o.Recursive, o.Filenames...).
 		ResourceTypeOrNameArgs(true, args...).
-		SingleResourceType().
+		ContinueOnError().
 		Latest().
-		Do().Infos()
+		Flatten().
+		Do()
+	err = r.Err()
 	if err != nil {
 		return err
 	}
-	if len(infos) != 1 {
+
-		return fmt.Errorf("rollout pause is only supported on individual resources - %d resources were found", len(infos))
+	o.Infos, err = r.Infos()
+	if err != nil {
+		return err
 	}
-	o.Info = infos[0]
 	return nil
 }
 
 func (o PauseConfig) RunPause() error {
-	isAlreadyPaused, err := o.PauseObject(o.Info.Object)
+	allErrs := []error{}
-	if err != nil {
+	for _, info := range o.Infos {
-		return err
+		isAlreadyPaused, err := o.PauseObject(info.Object)
+		if err != nil {
+			allErrs = append(allErrs, cmdutil.AddSourceToErr("pausing", info.Source, err))
+			continue
+		}
+		if isAlreadyPaused {
+			cmdutil.PrintSuccess(o.Mapper, false, o.Out, info.Mapping.Resource, info.Name, "already paused")
+			continue
+		}
+		cmdutil.PrintSuccess(o.Mapper, false, o.Out, info.Mapping.Resource, info.Name, "paused")
 	}
-	if isAlreadyPaused {
+	return utilerrors.NewAggregate(allErrs)
-		cmdutil.PrintSuccess(o.Mapper, false, o.Out, o.Info.Mapping.Resource, o.Info.Name, "already paused")
-		return nil
-	}
-	cmdutil.PrintSuccess(o.Mapper, false, o.Out, o.Info.Mapping.Resource, o.Info.Name, "paused")
-	return nil
 }

pkg/kubectl/cmd/rollout/rollout_resume.go

@@ -17,7 +17,6 @@ limitations under the License.
 package rollout
 
 import (
-	"fmt"
 	"io"
 
 	"github.com/spf13/cobra"
@@ -27,6 +26,7 @@ import (
 	cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util"
 	"k8s.io/kubernetes/pkg/kubectl/resource"
 	"k8s.io/kubernetes/pkg/runtime"
+	utilerrors "k8s.io/kubernetes/pkg/util/errors"
 )
 
 // ResumeConfig is the start of the data required to perform the operation.  As new fields are added, add them here instead of
@@ -35,7 +35,7 @@ type ResumeConfig struct {
 	ResumeObject func(object runtime.Object) (bool, error)
 	Mapper       meta.RESTMapper
 	Typer        runtime.ObjectTyper
-	Info         *resource.Info
+	Infos        []*resource.Info
 
 	Out       io.Writer
 	Filenames []string
@@ -62,8 +62,16 @@ func NewCmdRolloutResume(f *cmdutil.Factory, out io.Writer) *cobra.Command {
 		Long:    resume_long,
 		Example: resume_example,
 		Run: func(cmd *cobra.Command, args []string) {
-			cmdutil.CheckErr(opts.CompleteResume(f, cmd, out, args))
+			allErrs := []error{}
-			cmdutil.CheckErr(opts.RunResume())
+			err := opts.CompleteResume(f, cmd, out, args)
+			if err != nil {
+				allErrs = append(allErrs, err)
+			}
+			err = opts.RunResume()
+			if err != nil {
+				allErrs = append(allErrs, err)
+			}
+			cmdutil.CheckErr(utilerrors.Flatten(utilerrors.NewAggregate(allErrs)))
 		},
 	}
 
@@ -87,32 +95,45 @@ func (o *ResumeConfig) CompleteResume(f *cmdutil.Factory, cmd *cobra.Command, ou
 		return err
 	}
 
-	infos, err := resource.NewBuilder(o.Mapper, o.Typer, resource.ClientMapperFunc(f.ClientForMapping), f.Decoder(true)).
+	r := resource.NewBuilder(o.Mapper, o.Typer, resource.ClientMapperFunc(f.ClientForMapping), f.Decoder(true)).
 		NamespaceParam(cmdNamespace).DefaultNamespace().
 		FilenameParam(enforceNamespace, o.Recursive, o.Filenames...).
 		ResourceTypeOrNameArgs(true, args...).
-		SingleResourceType().
+		ContinueOnError().
 		Latest().
-		Do().Infos()
+		Flatten().
+		Do()
+	err = r.Err()
 	if err != nil {
 		return err
 	}
-	if len(infos) != 1 {
+
-		return fmt.Errorf("rollout resume is only supported on individual resources - %d resources were found", len(infos))
+	err = r.Visit(func(info *resource.Info, err error) error {
+		if err != nil {
+			return err
+		}
+		o.Infos = append(o.Infos, info)
+		return nil
+	})
+	if err != nil {
+		return err
 	}
-	o.Info = infos[0]
 	return nil
 }
 
 func (o ResumeConfig) RunResume() error {
-	isAlreadyResumed, err := o.ResumeObject(o.Info.Object)
+	allErrs := []error{}
-	if err != nil {
+	for _, info := range o.Infos {
-		return err
+		isAlreadyResumed, err := o.ResumeObject(info.Object)
+		if err != nil {
+			allErrs = append(allErrs, cmdutil.AddSourceToErr("resuming", info.Source, err))
+			continue
+		}
+		if isAlreadyResumed {
+			cmdutil.PrintSuccess(o.Mapper, false, o.Out, info.Mapping.Resource, info.Name, "already resumed")
+			continue
+		}
+		cmdutil.PrintSuccess(o.Mapper, false, o.Out, info.Mapping.Resource, info.Name, "resumed")
 	}
-	if isAlreadyResumed {
+	return utilerrors.NewAggregate(allErrs)
-		cmdutil.PrintSuccess(o.Mapper, false, o.Out, o.Info.Mapping.Resource, o.Info.Name, "already resumed")
-		return nil
-	}
-	cmdutil.PrintSuccess(o.Mapper, false, o.Out, o.Info.Mapping.Resource, o.Info.Name, "resumed")
-	return nil
 }

pkg/kubectl/cmd/rollout/rollout_undo.go

@@ -17,7 +17,6 @@ limitations under the License.
 package rollout
 
 import (
-	"fmt"
 	"io"
 
 	"k8s.io/kubernetes/pkg/api/meta"
@@ -25,6 +24,7 @@ import (
 	cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util"
 	"k8s.io/kubernetes/pkg/kubectl/resource"
 	"k8s.io/kubernetes/pkg/runtime"
+	utilerrors "k8s.io/kubernetes/pkg/util/errors"
 
 	"github.com/spf13/cobra"
 )
@@ -32,11 +32,11 @@ import (
 // UndoOptions is the start of the data required to perform the operation.  As new fields are added, add them here instead of
 // referencing the cmd.Flags()
 type UndoOptions struct {
-	Rollbacker kubectl.Rollbacker
+	Rollbackers []kubectl.Rollbacker
-	Mapper     meta.RESTMapper
+	Mapper      meta.RESTMapper
-	Typer      runtime.ObjectTyper
+	Typer       runtime.ObjectTyper
-	Info       *resource.Info
+	Infos       []*resource.Info
-	ToRevision int64
+	ToRevision  int64
 
 	Out       io.Writer
 	Filenames []string
@@ -53,7 +53,7 @@ kubectl rollout undo deployment/abc --to-revision=3`
 )
 
 func NewCmdRolloutUndo(f *cmdutil.Factory, out io.Writer) *cobra.Command {
-	options := &UndoOptions{}
+	opts := &UndoOptions{}
 
 	cmd := &cobra.Command{
 		Use:     "undo (TYPE NAME | TYPE/NAME) [flags]",
@@ -61,15 +61,23 @@ func NewCmdRolloutUndo(f *cmdutil.Factory, out io.Writer) *cobra.Command {
 		Long:    undo_long,
 		Example: undo_example,
 		Run: func(cmd *cobra.Command, args []string) {
-			cmdutil.CheckErr(options.CompleteUndo(f, cmd, out, args))
+			allErrs := []error{}
-			cmdutil.CheckErr(options.RunUndo())
+			err := opts.CompleteUndo(f, cmd, out, args)
+			if err != nil {
+				allErrs = append(allErrs, err)
+			}
+			err = opts.RunUndo()
+			if err != nil {
+				allErrs = append(allErrs, err)
+			}
+			cmdutil.CheckErr(utilerrors.Flatten(utilerrors.NewAggregate(allErrs)))
 		},
 	}
 
 	cmd.Flags().Int64("to-revision", 0, "The revision to rollback to. Default to 0 (last revision).")
 	usage := "Filename, directory, or URL to a file identifying the resource to get from a server."
-	kubectl.AddJsonFilenameFlag(cmd, &options.Filenames, usage)
+	kubectl.AddJsonFilenameFlag(cmd, &opts.Filenames, usage)
-	cmdutil.AddRecursiveFlag(cmd, &options.Recursive)
+	cmdutil.AddRecursiveFlag(cmd, &opts.Recursive)
 	return cmd
 }
 
@@ -87,31 +95,43 @@ func (o *UndoOptions) CompleteUndo(f *cmdutil.Factory, cmd *cobra.Command, out i
 		return err
 	}
 
-	infos, err := resource.NewBuilder(o.Mapper, o.Typer, resource.ClientMapperFunc(f.ClientForMapping), f.Decoder(true)).
+	r := resource.NewBuilder(o.Mapper, o.Typer, resource.ClientMapperFunc(f.ClientForMapping), f.Decoder(true)).
 		NamespaceParam(cmdNamespace).DefaultNamespace().
 		FilenameParam(enforceNamespace, o.Recursive, o.Filenames...).
 		ResourceTypeOrNameArgs(true, args...).
+		ContinueOnError().
 		Latest().
 		Flatten().
-		Do().
+		Do()
-		Infos()
+	err = r.Err()
 	if err != nil {
 		return err
 	}
 
-	if len(infos) != 1 {
+	err = r.Visit(func(info *resource.Info, err error) error {
-		return fmt.Errorf("rollout undo is only supported on individual resources - %d resources were found", len(infos))
+		if err != nil {
-	}
+			return err
-	o.Info = infos[0]
+		}
-	o.Rollbacker, err = f.Rollbacker(o.Info.ResourceMapping())
+		rollbacker, err := f.Rollbacker(info.ResourceMapping())
+		if err != nil {
+			return err
+		}
+		o.Infos = append(o.Infos, info)
+		o.Rollbackers = append(o.Rollbackers, rollbacker)
+		return nil
+	})
 	return err
 }
 
 func (o *UndoOptions) RunUndo() error {
-	result, err := o.Rollbacker.Rollback(o.Info.Namespace, o.Info.Name, nil, o.ToRevision, o.Info.Object)
+	allErrs := []error{}
-	if err != nil {
+	for ix, info := range o.Infos {
-		return err
+		result, err := o.Rollbackers[ix].Rollback(info.Namespace, info.Name, nil, o.ToRevision, info.Object)
+		if err != nil {
+			allErrs = append(allErrs, cmdutil.AddSourceToErr("undoing", info.Source, err))
+			continue
+		}
+		cmdutil.PrintSuccess(o.Mapper, false, o.Out, info.Mapping.Resource, info.Name, result)
 	}
-	cmdutil.PrintSuccess(o.Mapper, false, o.Out, o.Info.Mapping.Resource, o.Info.Name, result)
+	return utilerrors.NewAggregate(allErrs)
-	return nil
 }