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

Fixes get --show-all #18165

Merged
merged 1 commit into from
Dec 19, 2015
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions pkg/kubectl/cmd/get.go
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,15 @@ func RunGet(f *cmdutil.Factory, out io.Writer, cmd *cobra.Command, args []string
return cmdutil.UsageError(cmd, "Required resource not specified.")
}

// always show resources when getting by name or filename
argsHasNames, err := resource.HasNames(args)
if err != nil {
return err
}
if len(options.Filenames) > 0 || argsHasNames {
cmd.Flag("show-all").Value.Set("true")
}

// handle watch separately since we cannot watch multiple resource types
isWatch, isWatchOnly := cmdutil.GetFlagBool(cmd, "watch"), cmdutil.GetFlagBool(cmd, "watch-only")
if isWatch || isWatchOnly {
Expand Down
23 changes: 23 additions & 0 deletions pkg/kubectl/cmd/get_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -589,6 +589,29 @@ func TestGetMultipleTypeObjectsWithDirectReference(t *testing.T) {
t.Errorf("unexpected empty output")
}
}

func TestGetByNameForcesFlag(t *testing.T) {
pods, _, _ := testData()

f, tf, codec := NewAPIFactory()
tf.Printer = &testPrinter{}
tf.Client = &fake.RESTClient{
Codec: codec,
Resp: &http.Response{StatusCode: 200, Body: objBody(codec, &pods.Items[0])},
}
tf.Namespace = "test"
buf := bytes.NewBuffer([]byte{})

cmd := NewCmdGet(f, buf)
cmd.SetOutput(buf)
cmd.Run(cmd, []string{"pods", "foo"})

showAllFlag, _ := cmd.Flags().GetBool("show-all")
if !showAllFlag {
t.Errorf("expected showAll to be true when getting resource by name")
}
}

func watchTestData() ([]api.Pod, []watch.Event) {
pods := []api.Pod{
{
Expand Down
49 changes: 32 additions & 17 deletions pkg/kubectl/resource/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -282,23 +282,7 @@ func (b *Builder) SelectAllParam(selectAll bool) *Builder {
// When two or more arguments are received, they must be a single type and resource name(s).
// The allowEmptySelector permits to select all the resources (via Everything func).
func (b *Builder) ResourceTypeOrNameArgs(allowEmptySelector bool, args ...string) *Builder {
// convert multiple resources to resource tuples, a,b,c d as a transform to a/d b/d c/d
if len(args) >= 2 {
resources := []string{}
resources = append(resources, SplitResourceArgument(args[0])...)
if len(resources) > 1 {
names := []string{}
names = append(names, args[1:]...)
newArgs := []string{}
for _, resource := range resources {
for _, name := range names {
newArgs = append(newArgs, strings.Join([]string{resource, name}, "/"))
}
}
args = newArgs
}
}

args = normalizeMultipleResourcesArgs(args)
if ok, err := hasCombinedTypeArgs(args); ok {
if err != nil {
b.errs = append(b.errs, err)
Expand Down Expand Up @@ -369,6 +353,27 @@ func hasCombinedTypeArgs(args []string) (bool, error) {
}
}

// Normalize args convert multiple resources to resource tuples, a,b,c d
// as a transform to a/d b/d c/d
func normalizeMultipleResourcesArgs(args []string) []string {
if len(args) >= 2 {
resources := []string{}
resources = append(resources, SplitResourceArgument(args[0])...)
if len(resources) > 1 {
names := []string{}
names = append(names, args[1:]...)
newArgs := []string{}
for _, resource := range resources {
for _, name := range names {
newArgs = append(newArgs, strings.Join([]string{resource, name}, "/"))
}
}
return newArgs
}
}
return args
}

// splitResourceTypeName handles type/name resource formats and returns a resource tuple
// (empty or not), whether it successfully found one, and an error
func splitResourceTypeName(s string) (resourceTuple, bool, error) {
Expand Down Expand Up @@ -697,3 +702,13 @@ func SplitResourceArgument(arg string) []string {
}
return out
}

// HasNames returns true if the provided args contain resource names
func HasNames(args []string) (bool, error) {
args = normalizeMultipleResourcesArgs(args)
hasCombinedTypes, err := hasCombinedTypeArgs(args)
if err != nil {
return false, err
}
return hasCombinedTypes || len(args) > 1, nil
}
58 changes: 58 additions & 0 deletions pkg/kubectl/resource/builder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1003,3 +1003,61 @@ func TestReplaceAliases(t *testing.T) {
}
}
}

func TestHasNames(t *testing.T) {
tests := []struct {
args []string
expectedHasName bool
expectedError error
}{
{
args: []string{""},
expectedHasName: false,
expectedError: nil,
},
{
args: []string{"rc"},
expectedHasName: false,
expectedError: nil,
},
{
args: []string{"rc,pod,svc"},
expectedHasName: false,
expectedError: nil,
},
{
args: []string{"rc/foo"},
expectedHasName: true,
expectedError: nil,
},
{
args: []string{"rc", "foo"},
expectedHasName: true,
expectedError: nil,
},
{
args: []string{"rc,pod,svc", "foo"},
expectedHasName: true,
expectedError: nil,
},
{
args: []string{"rc/foo", "rc/bar", "rc/zee"},
expectedHasName: true,
expectedError: nil,
},
{
args: []string{"rc/foo", "bar"},
expectedHasName: false,
expectedError: fmt.Errorf("when passing arguments in resource/name form, all arguments must include the resource"),
},
}
for _, test := range tests {
hasNames, err := HasNames(test.args)
if !reflect.DeepEqual(test.expectedError, err) {
t.Errorf("expected HasName to error %v, got %s", test.expectedError, err)
}
if hasNames != test.expectedHasName {
t.Errorf("expected HasName to return %v for %s", test.expectedHasName, test.args)
}
}
}
8 changes: 4 additions & 4 deletions pkg/kubectl/resource_printer.go
Original file line number Diff line number Diff line change
Expand Up @@ -550,10 +550,10 @@ func translateTimestamp(timestamp unversioned.Time) string {
}

func printPod(pod *api.Pod, w io.Writer, options printOptions) error {
return printPodBase(pod, w, true, options)
return printPodBase(pod, w, options)
}

func printPodBase(pod *api.Pod, w io.Writer, showIfTerminating bool, options printOptions) error {
func printPodBase(pod *api.Pod, w io.Writer, options printOptions) error {
name := pod.Name
namespace := pod.Namespace

Expand All @@ -563,7 +563,7 @@ func printPodBase(pod *api.Pod, w io.Writer, showIfTerminating bool, options pri

reason := string(pod.Status.Phase)
// if not printing all pods, skip terminated pods (default)
if !showIfTerminating && !options.showAll && (reason == string(api.PodSucceeded) || reason == string(api.PodFailed)) {
if !options.showAll && (reason == string(api.PodSucceeded) || reason == string(api.PodFailed)) {
return nil
}
if pod.Status.Reason != "" {
Expand Down Expand Up @@ -623,7 +623,7 @@ func printPodBase(pod *api.Pod, w io.Writer, showIfTerminating bool, options pri

func printPodList(podList *api.PodList, w io.Writer, options printOptions) error {
for _, pod := range podList.Items {
if err := printPodBase(&pod, w, false, options); err != nil {
if err := printPodBase(&pod, w, options); err != nil {
return err
}
}
Expand Down