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

Adding an age column to the cli, closes #417 #420

Merged
merged 4 commits into from
Nov 17, 2022

Conversation

cedi
Copy link
Contributor

@cedi cedi commented Nov 15, 2022

As an SRE, I deploy a lot of hcloud resources automatically via the hcloud API and use the cli to validate those changes. I am deploying using immutable VMs rather than phoenix deployments (instead of updating existing VMs, I re-create them with the newest config) Since I am using the API in automation to create hcloud resources, my hcloud resources include Ids in their name, making it a mess sometimes to figure out which is the newest VM. To know which VM got deployed when, I often find myself using the hcloud-cli like this:

$ hcloud server list -s name -o 'columns=name,status,created,ipv4,ipv6'
NAME                           STATUS    CREATED                        IPV4             IPV6
cedi-dev-control-plane-xxxxx   running   Sun Nov 13 13:43:05 CET 2022   XXX.XXX.XXX.XXX  2a01:4f8:xxxx:xxxx::/64
cedi-dev-control-plane-xxxxx   running   Sun Nov 13 13:51:46 CET 2022   XXX.XXX.XXX.XXX  2a01:4f8:xxxx:xxxx::/64
cedi-dev-control-plane-xxxxx   running   Sun Nov 13 13:40:22 CET 2022   XXX.XXX.XXX.XXX  2a01:4f8:xxxx:xxxx::/64
cedi-dev-worker-cxp31-xxxxx    running   Wed Nov  9 17:15:32 CET 2022   XXX.XXX.XXX.XXX  2a01:4f8:xxxx:xxxx::/64
cedi-dev-worker-cxp31-xxxxx    running   Wed Nov  9 17:26:36 CET 2022   XXX.XXX.XXX.XXX  2a01:4f8:xxxx:xxxx::/64
cedi-dev-worker-cxp31-xxxxx    running   Wed Nov  9 16:46:55 CET 2022   XXX.XXX.XXX.XXX  2a01:4f8:xxxx:xxxx::/64
cedi-dev-worker-cxp31-xxxxx    running   Sun Nov 13 14:02:41 CET 2022   XXX.XXX.XXX.XXX  2a01:4f8:xxxx:xxxx::/64

However, I noticed the created column contains the "raw" create DateTime string which is good for computers to read, but bad for humans. Using the hcloud dashboard in my browser, I can see the created timestamp as a Duration since time.Now():

Screenshot 2022-11-13 at 14 17 18

With this commit I add a "age" column to the output of hcloud cli:

$ hcloud server list
ID         NAME                           STATUS    IPV4             IPV6                      PRIVATE NET           DATACENTER   AGE
25550867   cedi-dev-control-plane-xxxxx   running   xxx.xxx.xx.xxx   2a01:4f8:xxxx:xxxx::/64   10.0.0.x (cedi-dev)   fsn1-dcxx    2d
25551100   cedi-dev-control-plane-xxxxx   running   xx.xx.xxx.xx     2a01:4f8:xxx:xxxx::/64    10.0.0.x (cedi-dev)   fsn1-dcxx    2d
25551348   cedi-dev-worker-cxp31-xxxxx    running   xx.xx.xx.xxx     2a01:4f8:xxxx:xxxx::/64   10.0.0.x (cedi-dev)   fsn1-dcxx    2d
25586128   cedi-dev-worker-cxp31-xxxxx    running   xx.xx.xxx.xx     2a01:4f8:xxxx:xxxx::/64   10.0.0.x (cedi-dev)   fsn1-dcxx    1d
25586289   cedi-dev-control-plane-xxxxx   running   xxx.xx.xx.xx     2a01:4f8:xxxx:xxxx::/64   10.0.0.x (cedi-dev)   fsn1-dcxx    1d
25588261   cedi-dev-worker-cxp31-xxxxx    running   xxx.xx.xxx.xxx   2a01:4f8:xxx:xxxx::/64    10.0.0.x (cedi-dev)   fsn1-dcxx    23h

$ hcloud load-balancer list
ID       NAME                            IPV4              IPV6                    TYPE   LOCATION   NETWORK ZONE   AGE
912000   cedi-dev-kube-apiserver-xxxxx   xxx.xxx.xxx.xxx   2a01:4f8:xxxx:xx::1     lb11   fsn1       eu-central     24d
912020   cedi-dev-ingress-lb             xxx.xxx.xxx.xx    2a01:4f8:xxxx:xxxx::1   lb11   nbg1       eu-central     24d

I also added the "age" column to the "default_columns" in most commands

As an SRE, I deploy a lot of hcloud resources automatically via the hcloud API and use the cli to validate those changes.
I am deploying using immutable VMs rather than phoenix deployments (instead of updating existing VMs, I re-create them with the newest config)
Since I am using the API in automation to create hcloud resources, my hcloud resources include Ids in their name, making it a mess sometimes to figure out which is the newest VM.
To know which VM got deployed when, I often find myself using the hcloud-cli like this:

```bash
$ hcloud server list -s name -o 'columns=name,status,created,ipv4,ipv6'
NAME                           STATUS    CREATED                        IPV4             IPV6
cedi-dev-control-plane-xxxxx   running   Sun Nov 13 13:43:05 CET 2022   XXX.XXX.XXX.XXX  2a01:4f8:xxxx:xxxx::/64
cedi-dev-control-plane-xxxxx   running   Sun Nov 13 13:51:46 CET 2022   XXX.XXX.XXX.XXX  2a01:4f8:xxxx:xxxx::/64
cedi-dev-control-plane-xxxxx   running   Sun Nov 13 13:40:22 CET 2022   XXX.XXX.XXX.XXX  2a01:4f8:xxxx:xxxx::/64
cedi-dev-worker-cxp31-xxxxx    running   Wed Nov  9 17:15:32 CET 2022   XXX.XXX.XXX.XXX  2a01:4f8:xxxx:xxxx::/64
cedi-dev-worker-cxp31-xxxxx    running   Wed Nov  9 17:26:36 CET 2022   XXX.XXX.XXX.XXX  2a01:4f8:xxxx:xxxx::/64
cedi-dev-worker-cxp31-xxxxx    running   Wed Nov  9 16:46:55 CET 2022   XXX.XXX.XXX.XXX  2a01:4f8:xxxx:xxxx::/64
cedi-dev-worker-cxp31-xxxxx    running   Sun Nov 13 14:02:41 CET 2022   XXX.XXX.XXX.XXX  2a01:4f8:xxxx:xxxx::/64
```

However, I noticed the `created` column contains the "raw" create DateTime string which is good for computers to read, but bad for humans.
Using the hcloud dashboard in my browser, I can see the created timestamp as a Duration since `time.Now()`:

<img width="215" alt="Screenshot 2022-11-13 at 14 17 18" src="https://user-images.githubusercontent.com/1952599/201523632-35a91d6d-4039-4469-a308-6c2355f70652.png">

With this commit I add a "age" column to the output of hcloud cli:

```bash
$ hcloud server list
ID         NAME                           STATUS    IPV4             IPV6                      PRIVATE NET           DATACENTER   AGE
25550867   cedi-dev-control-plane-xxxxx   running   xxx.xxx.xx.xxx   2a01:4f8:xxxx:xxxx::/64   10.0.0.x (cedi-dev)   fsn1-dcxx    2d
25551100   cedi-dev-control-plane-xxxxx   running   xx.xx.xxx.xx     2a01:4f8:xxx:xxxx::/64    10.0.0.x (cedi-dev)   fsn1-dcxx    2d
25551348   cedi-dev-worker-cxp31-xxxxx    running   xx.xx.xx.xxx     2a01:4f8:xxxx:xxxx::/64   10.0.0.x (cedi-dev)   fsn1-dcxx    2d
25586128   cedi-dev-worker-cxp31-xxxxx    running   xx.xx.xxx.xx     2a01:4f8:xxxx:xxxx::/64   10.0.0.x (cedi-dev)   fsn1-dcxx    1d
25586289   cedi-dev-control-plane-xxxxx   running   xxx.xx.xx.xx     2a01:4f8:xxxx:xxxx::/64   10.0.0.x (cedi-dev)   fsn1-dcxx    1d
25588261   cedi-dev-worker-cxp31-xxxxx    running   xxx.xx.xxx.xxx   2a01:4f8:xxx:xxxx::/64    10.0.0.x (cedi-dev)   fsn1-dcxx    23h

```

I also added the "age" column to the "default_columns" in most commands
cedi added a commit to cedi/cli that referenced this pull request Nov 16, 2022
I am a heavy user of the hcloud-cli. However, I really don't like the
default sorting by server-id, and would prefer sorting by age
(see hetznercloud#417/hetznercloud#420) or name.

Currently, I always type `hcloud server list -s name` to sort by name.

since the `-s` flag is part of the `list` subcommand and not the `hcloud`
base command (like in many other software that makes use of the Cobra
framework), I cannot simply `alias hcloud-"hcloud -s name"` in my shell
to default the sorting to "by name".

This commit adds a `[defaults]` section to the config toml which allows
to override the default sorting behavior for individual `hcloud`
subcommands.

The following behavior is used to determine the sorting-order

| config file | flag    | result               |
|-------------|---------|----------------------|
| not-set     | not-set | sort by id (default) |
| not-set     | set     | sort by flag value   |
| set         | not-set | sort by config value |
| set         | set     | sort by flag value   |
@apricote apricote self-requested a review November 17, 2022 08:12
Copy link
Member

@apricote apricote left a comment

Choose a reason for hiding this comment

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

This looks great! It would be awesome if you could add some simple unit tests for the util.Age function, but I can also do that after we merge this PR if you don't have the time for it.

This commit adds unit-testing to the `Ago` function as well as fixing
existing unit-tests
@cedi
Copy link
Contributor Author

cedi commented Nov 17, 2022

This looks great! It would be awesome if you could add some simple unit tests for the util.Age function, but I can also do that after we merge this PR if you don't have the time for it.

Hey @apricote, thanks for the feedback! I totally forgot adding unit-testing for this. Sorry!

I added some tests for Ago, as well as fixing the existing tests that I accidentally broke

Copy link
Member

@apricote apricote left a comment

Choose a reason for hiding this comment

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

Thanks for your effort :)

internal/cmd/util/util_internal_test.go Outdated Show resolved Hide resolved
internal/cmd/network/list_test.go Outdated Show resolved Hide resolved
@cedi cedi requested a review from apricote November 17, 2022 14:57
@apricote apricote merged commit aed825d into hetznercloud:main Nov 17, 2022
cedi added a commit to cedi/cli that referenced this pull request Nov 17, 2022
I am a heavy user of the hcloud-cli. However, I really don't like the
default sorting by server-id, and would prefer sorting by age
(see hetznercloud#417/hetznercloud#420) or name.

Currently, I always type `hcloud server list -s name` to sort by name.

since the `-s` flag is part of the `list` subcommand and not the `hcloud`
base command (like in many other software that makes use of the Cobra
framework), I cannot simply `alias hcloud-"hcloud -s name"` in my shell
to default the sorting to "by name".

This commit adds a `[defaults]` section to the config toml which allows
to override the default sorting behavior for individual `hcloud`
subcommands.

The following behavior is used to determine the sorting-order

| config file | flag    | result               |
|-------------|---------|----------------------|
| not-set     | not-set | sort by id (default) |
| not-set     | set     | sort by flag value   |
| set         | not-set | sort by config value |
| set         | set     | sort by flag value   |
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants