Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

enable recursive processing in kubectl rollout #25110

Merged
merged 1 commit into from
May 15, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
53 changes: 53 additions & 0 deletions hack/test-cmd.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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 #
Expand Down
29 changes: 14 additions & 15 deletions pkg/kubectl/cmd/rollout/rollout_history.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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().
Infos()
Do()
err = r.Err()
if err != nil {
return err
}

errs := []error{}
for _, info := range infos {
err = r.Visit(func(info *resource.Info, err error) error {
if err != nil {
return err
}
mapping := info.ResourceMapping()
historyViewer, err := f.HistoryViewer(mapping)
if err != nil {
errs = append(errs, err)
continue
return err
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are you returning instead of continuing?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you're right - i'll update this

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I thought about this some more:

The continue was relevant because the previous code was iterating through the files in a for loop, but because Visit() handles iteration and error aggregation for us, issuing a return err accomplishes the same task, as we'll still collect the error if its encountered, and it'll then just move on to the next iteration.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

}
historyInfo, err := historyViewer.History(info.Namespace, info.Name)
if err != nil {
errs = append(errs, err)
continue
return err
}

if revisionDetail > 0 {
Expand All @@ -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)
continue
return printErr
}
fmt.Fprintf(out, "%s\n", formattedOutput)
}
}

return errors.NewAggregate(errs)
return nil
})
return err
}
53 changes: 34 additions & 19 deletions pkg/kubectl/cmd/rollout/rollout_pause.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ limitations under the License.
package rollout

import (
"fmt"
"io"

"github.com/spf13/cobra"
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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))
cmdutil.CheckErr(opts.RunPause())
allErrs := []error{}
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)))
},
}

Expand All @@ -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)
if err != nil {
return err
}
if isAlreadyPaused {
cmdutil.PrintSuccess(o.Mapper, false, o.Out, o.Info.Mapping.Resource, o.Info.Name, "already paused")
return nil
allErrs := []error{}
for _, info := range o.Infos {
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")
}
cmdutil.PrintSuccess(o.Mapper, false, o.Out, o.Info.Mapping.Resource, o.Info.Name, "paused")
return nil
return utilerrors.NewAggregate(allErrs)
}
59 changes: 40 additions & 19 deletions pkg/kubectl/cmd/rollout/rollout_resume.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ limitations under the License.
package rollout

import (
"fmt"
"io"

"github.com/spf13/cobra"
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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))
cmdutil.CheckErr(opts.RunResume())
allErrs := []error{}
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)))
},
}

Expand All @@ -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)
if err != nil {
return err
}
if isAlreadyResumed {
cmdutil.PrintSuccess(o.Mapper, false, o.Out, o.Info.Mapping.Resource, o.Info.Name, "already resumed")
return nil
allErrs := []error{}
for _, info := range o.Infos {
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")
}
cmdutil.PrintSuccess(o.Mapper, false, o.Out, o.Info.Mapping.Resource, o.Info.Name, "resumed")
return nil
return utilerrors.NewAggregate(allErrs)
}