Print clusterID, memberID and leaseID in hexdecimal

[ahrtr]

ClusterID, memberID and leaseID are just identities, so it doesn't make sense to print them in decimal. Instead, we should always print them in hexadecimal.

Please note the existing behavior isn't consistent. Sometimes etcdctl prints them in hexdecimal (such as etcdctl endpoint status -w table), but sometimes in decimal (such as etctctl endpoint status -w fields).

Previous output

wachao-a01:bin wachao$ ./etcdctl  member list -w fields
"ClusterID" : 14841639068965178418
"MemberID" : 10276657743932975437
"RaftTerm" : 5
"ID" : 10276657743932975437
"Name" : "default"
"PeerURL" : "http://localhost:2380"
"ClientURL" : "http://localhost:2379"
"IsLearner" : false

wachao-a01:bin wachao$ ./etcdctl endpoint status -w fields
"ClusterID" : 14841639068965178418
"MemberID" : 10276657743932975437
"Revision" : 3
"RaftTerm" : 5
"Version" : "3.6.0-alpha.0"
"StorageVersion" : "3.6.0"
"DBSize" : 24576
"DBSizeInUse" : 24576
"Leader" : 10276657743932975437
"IsLearner" : false
"RaftIndex" : 23
"RaftTerm" : 5
"RaftAppliedIndex" : 23
"Errors" : []
"Endpoint" : "127.0.0.1:2379"

wachao-a01:bin wachao$ ./etcdctl  lease list -w fields
"ClusterID" : 14841639068965178418
"MemberID" : 10276657743932975437
"Revision" : 3
"RaftTerm" : 8
"ID" : 7587863798256828424
"ID" : 7587863798360740612
"ID" : 7587863798360740614
"ID" : 7587863798256828422

New output

wachao-a01:bin wachao$ ./etcdctl member list -w fields
"ClusterID" : cdf818194e3a8c32
"MemberID" : 8e9e05c52164694d
"RaftTerm" : 7
"ID" : 8e9e05c52164694d
"Name" : "default"
"PeerURL" : "http://localhost:2380"
"ClientURL" : "http://localhost:2379"
"IsLearner" : false

wachao-a01:bin wachao$ ./etcdctl endpoint status -w fields
"ClusterID" : cdf818194e3a8c32
"MemberID" : 8e9e05c52164694d
"Revision" : 3
"RaftTerm" : 7
"Version" : "3.6.0-alpha.0"
"StorageVersion" : "3.6.0"
"DBSize" : 24576
"DBSizeInUse" : 24576
"Leader" : 10276657743932975437
"IsLearner" : false
"RaftIndex" : 27
"RaftTerm" : 7
"RaftAppliedIndex" : 27
"Errors" : []
"Endpoint" : "127.0.0.1:2379"

wachao-a01:bin wachao$ ./etcdctl  lease list -w fields
"ClusterID" : cdf818194e3a8c32
"MemberID" : 8e9e05c52164694d
"Revision" : 3
"RaftTerm" : 7
"ID" : 694d81ec1b250008
"ID" : 694d81ec1b250006

Signed-off-by: Benjamin Wang [email protected]

[ahrtr]

cc @serathius @ptabor @spzala

@liggitt @dims @neolit123 do you have any concerns from K8s perspective?

[codecov-commenter]

Codecov Report

Merging #14208 (5cae1a1) into main (20bf49c) will decrease coverage by 0.40%. The diff coverage is 8.57%.

@@            Coverage Diff             @@
##             main   #14208      +/-   ##
==========================================
- Coverage   75.49%   75.09%   -0.41%     
==========================================
  Files         455      455              
  Lines       36859    36908      +49     
==========================================
- Hits        27828    27715     -113     
- Misses       7299     7441     +142     
- Partials     1732     1752      +20     

Flag	Coverage Δ
all	75.09% <8.57%> (-0.41%)	:arrow_down:

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
etcdctl/ctlv3/command/printer_fields.go	6.09% <5.88%> (-1.10%)	:arrow_down:
etcdctl/ctlv3/command/printer.go	23.20% <100.00%> (ø)
server/etcdserver/api/rafthttp/peer_status.go	87.87% <0.00%> (-12.13%)	:arrow_down:
server/auth/simple_token.go	80.00% <0.00%> (-8.47%)	:arrow_down:
server/etcdserver/api/rafthttp/peer.go	87.01% <0.00%> (-8.45%)	:arrow_down:
client/pkg/v3/fileutil/lock_linux.go	72.22% <0.00%> (-8.34%)	:arrow_down:
client/pkg/v3/tlsutil/tlsutil.go	83.33% <0.00%> (-8.34%)	:arrow_down:
client/v3/leasing/util.go	91.66% <0.00%> (-6.67%)	:arrow_down:
server/etcdserver/api/v3rpc/util.go	74.19% <0.00%> (-6.46%)	:arrow_down:
client/v3/concurrency/mutex.go	61.64% <0.00%> (-5.48%)	:arrow_down:
... and 31 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 20bf49c...5cae1a1. Read the comment docs.

[neolit123]

Given the discrepancy is exposed to the user on the CLI it might make sense to make it possible for them opt out with a flag for a transitional period until the the behavior is consistent accross the CLI commands.

In terms of hex vs decimal for IDs - it depends. Decimal makes it easier to process / human parse IDs in e.g. scripts.

[ptabor]

In terms of hex vs decimal for IDs - it depends. Decimal makes it easier to process / human parse IDs in e.g. scripts.

Storing this as typed integer saves a few bytes but caries risk of `signed vs. unsigned' mismatch and overflow. As we don't compare this values for inequality I think that hex are steering users towards safer code where they would script them as 'strings'.

[ahrtr]

Thanks @neolit123 and @ptabor for the feedback, which basically makes sense to me.

Actually I thought about backward compatible, the reason I did not do it was that there is no any change on JSON output which should be the most common case for programmatically parsing the output.

But anyway, I updated this PR and leverage the existing flag --hex

wachao-a01:bin wachao$ ./etcdctl  member list -w fields --hex
"ClusterID" : cdf818194e3a8c32
"MemberID" : 8e9e05c52164694d
"RaftTerm" : 2
"ID" : 8e9e05c52164694d
"Name" : "default"
"PeerURL" : "http://localhost:2380"
"ClientURL" : "http://localhost:2379"
"IsLearner" : false


wachao-a01:bin wachao$ ./etcdctl  lease list -w fields --hex
"ClusterID" : cdf818194e3a8c32
"MemberID" : 8e9e05c52164694d
"Revision" : 1
"RaftTerm" : 2
"ID" : 694d820437ab0a07

PTAL @ptabor @neolit123 @serathius @spzala thx

[ahrtr]

Thanks @spzala for the review comment, which basically makes sense to me.

Points from my side:

1. Unfortunately, there is no any unit tests for any command so far. So I will investigate how to add unit test and add an example case firstly in a separate PR; afterwards, let's get more contributors involved to add unit tests for all commands. What do you think?
2. With regard the Changelog, I intentionally did not do it, because there are some other related changes (such as https://github.com/etcd-io/etcd/pull/14120#issuecomment-1195159939 , and the JSON output is also printing decimal for MemberID etc.) to be done. So I plan to add the ChangeLog after all the related items are done.

[spzala]

Thanks @spzala for the review comment, which basically makes sense to me.

Points from my side:

1. Unfortunately, there is no any unit tests for any command so far. So I will investigate how to add unit test and add an example case firstly in a separate PR; afterwards, let's get more contributors involved to add unit tests for all commands. What do you think?

2. With regard the Changelog, I intentionally did not do it, because there are some other related changes (such as [server: Implement compaction hash checking #14120 (comment)](https://github.com/etcd-io/etcd/pull/14120#issuecomment-1195159939) , and the JSON output is also printing decimal for MemberID etc.) to be done. So I plan to add the ChangeLog after all the related items are done.

@ahrtr thank you, and both the points you mentioned sounds good to me. I will be happy to share work on adding unit test coverage.

[spzala]

@ahrtr considering @ptabor is away for sometime and his comments are addressed, we should be good with merging changes, if that sounds good to you?. Thanks!

[ahrtr]

Thanks @spzala .

Actually this PR should be a small and safe change because there is no any change on the JSON output so far, so let me merge it.