From bc6726da047bb478e9062270d0e693e77a069d01 Mon Sep 17 00:00:00 2001 From: Travis Bischel Date: Thu, 5 May 2022 20:03:24 -0600 Subject: [PATCH] rpk: avoid listing offsets for no topics, fix group seek For rpk group seek, if we seeked in a group that had no topics, we would list *all* topics (as is the api for list) and then commit a group to the beginning of *all* topics. We now only seek if there are topics to seek, otherwise we print the standard output that no offset was changed. We also fix this potential seek problem in rpk group describe, but realistically all groups have topics assigned. --- src/go/rpk/pkg/cli/cmd/group/describe.go | 7 ++-- src/go/rpk/pkg/cli/cmd/group/seek.go | 43 +++++++++++++----------- src/go/rpk/pkg/cli/cmd/topic/list.go | 2 +- 3 files changed, 30 insertions(+), 22 deletions(-) diff --git a/src/go/rpk/pkg/cli/cmd/group/describe.go b/src/go/rpk/pkg/cli/cmd/group/describe.go index 7b295f63262cf..d7a8ff7c75369 100644 --- a/src/go/rpk/pkg/cli/cmd/group/describe.go +++ b/src/go/rpk/pkg/cli/cmd/group/describe.go @@ -64,8 +64,11 @@ information about the members. out.Die("unable to fetch offsets for any group") } - listed, err := adm.ListEndOffsets(ctx, described.AssignedPartitions().Topics()...) - out.HandleShardError("ListOffsets", err) + var listed kadm.ListedOffsets + if topics := described.AssignedPartitions().Topics(); len(topics) > 0 { + listed, err = adm.ListEndOffsets(ctx, topics...) + out.HandleShardError("ListOffsets", err) + } printDescribed( described, diff --git a/src/go/rpk/pkg/cli/cmd/group/seek.go b/src/go/rpk/pkg/cli/cmd/group/seek.go index 8118b994a158e..5efbf63491c56 100644 --- a/src/go/rpk/pkg/cli/cmd/group/seek.go +++ b/src/go/rpk/pkg/cli/cmd/group/seek.go @@ -220,28 +220,33 @@ func seek( tps[topic] = map[int32]struct{}{} // ensure exists } topics := tps.Topics() - var listed kadm.ListedOffsets - var err error - switch to { - case "start": - listed, err = adm.ListStartOffsets(context.Background(), topics...) - case "end": - listed, err = adm.ListEndOffsets(context.Background(), topics...) - default: - var milli int64 - milli, err = strconv.ParseInt(to, 10, 64) - out.MaybeDie(err, "unable to parse millisecond %q: %v", to, err) - switch len(to) { - case 10: // e.g. "1622505600"; sec to milli - milli *= 1000 - case 13: // e.g. "1622505600000", already in milli - case 19: // e.g. "1622505600000000000"; nano to milli - milli /= 1e6 + if len(topics) > 0 { + var err error + switch to { + case "start": + listed, err = adm.ListStartOffsets(context.Background(), topics...) + case "end": + listed, err = adm.ListEndOffsets(context.Background(), topics...) default: - out.Die("--to timestamp %q is not a second, nor a millisecond, nor a nanosecond", to) + var milli int64 + milli, err = strconv.ParseInt(to, 10, 64) + out.MaybeDie(err, "unable to parse millisecond %q: %v", to, err) + switch len(to) { + case 10: // e.g. "1622505600"; sec to milli + milli *= 1000 + case 13: // e.g. "1622505600000", already in milli + case 19: // e.g. "1622505600000000000"; nano to milli + milli /= 1e6 + default: + out.Die("--to timestamp %q is not a second, nor a millisecond, nor a nanosecond", to) + } + listed, err = adm.ListOffsetsAfterMilli(context.Background(), milli, topics...) + } + if err == nil { // ListOffsets can return ShardErrors, but we want to be entirely successful + err = listed.Error() } - listed, err = adm.ListOffsetsAfterMilli(context.Background(), milli, topics...) + out.MaybeDie(err, "unable to list all offsets successfully: %v", err) } if err == nil { // ListOffsets can return ShardErrors, but we want to be entirely successful err = listed.Error() diff --git a/src/go/rpk/pkg/cli/cmd/topic/list.go b/src/go/rpk/pkg/cli/cmd/topic/list.go index 77e1c86727630..2236b577a6143 100644 --- a/src/go/rpk/pkg/cli/cmd/topic/list.go +++ b/src/go/rpk/pkg/cli/cmd/topic/list.go @@ -52,7 +52,7 @@ information. Run: func(cmd *cobra.Command, topics []string) { // The purpose of the regex flag really is for users to // know what topics they will delete when using regex. - // We forbit deleting internal topics (redpanda + // We forbid deleting internal topics (redpanda // actually does not expose these currently), so we // make -r and -i incompatible. if internal && re {