`juju status` panics on some combinations of deployed applications with particular names

[ca-scribner]

Description

For some specific sets of deployed application names, juju status panics instead of displaying the deployed applications.

Juju version

3.6.7

Cloud

No response

Expected behaviour

that juju status would render what is in the model successfully for any combination of deployed applications

Reproduce / Test

juju add-model test

# deploys successfully
juju deploy grafana-agent-k8s x234567890x

# renders successfully
juju status

# deploys successfully
juju deploy grafana-agent-k8s x2345678901234567890x

# renders successfully
juju status

# deploys successfully
juju deploy grafana-agent-k8s x23456789012345678901234567890x

# panics
juju status

output:

Model   Controller      Cloud/Region              Version  SLA          Timestamp
testc1  local-microk8s  local-microk8s/localhost  3.6.7    unsupported  14:20:43-04:00

panic: parsing number 23456789012345678901234567890: strconv.Atoi: parsing "23456789012345678901234567890": value out of range

goroutine 1 [running]:
github.com/juju/naturalsort.splitAtNumber({0xc0009dea40, 0x1f})
        /build/snapcraft-juju-a98554e81ff3bbc6574e9e4859860106/parts/juju/build/vendor/github.com/juju/naturalsort/naturalsort.go:74 +0x1aa
github.com/juju/naturalsort.naturally.Less({0xc0001ada40?, 0x40?, 0x5631060?}, 0x5631060?, 0x0?)
        /build/snapcraft-juju-a98554e81ff3bbc6574e9e4859860106/parts/juju/build/vendor/github.com/juju/naturalsort/naturalsort.go:45 +0x89
sort.insertionSort({0x6f16288, 0xc0009dcee8}, 0x0, 0x3)
        /snap/go/10888/src/sort/zsortinterface.go:12 +0xa7
sort.pdqsort({0x6f16288, 0xc0009dcee8}, 0x18?, 0x5723900?, 0xc00098c601?)
        /snap/go/10888/src/sort/zsortinterface.go:73 +0x305
sort.Sort({0x6f16288, 0xc0009dcee8})
        /snap/go/10888/src/sort/sort.go:54 +0x54
github.com/juju/naturalsort.Sort(...)
        /build/snapcraft-juju-a98554e81ff3bbc6574e9e4859860106/parts/juju/build/vendor/github.com/juju/naturalsort/naturalsort.go:15
github.com/juju/juju/cmd/juju/status.printApplications(_, {{{0xc000c73c28, 0x6}, {0xc000c73c30, 0x4}, {0xc0001ec630, 0xe}, {0xc000c3cd26, 0xe}, {0xc000c73c34, ...}, ...}, ...})
        /build/snapcraft-juju-a98554e81ff3bbc6574e9e4859860106/parts/juju/build/cmd/juju/status/output_tabular.go:156 +0x3d4
github.com/juju/juju/cmd/juju/status.FormatTabular({0x6ec1900, 0xc0000f4038}, 0x1, {0x62c93a0?, 0xc000c89080?})
        /build/snapcraft-juju-a98554e81ff3bbc6574e9e4859860106/parts/juju/build/cmd/juju/status/output_tabular.go:102 +0xc4a
github.com/juju/juju/cmd/juju/status.(*statusCommand).FormatTabular(0xc000812b60?, {0x6ec1900?, 0xc0000f4038?}, {0x62c93a0?, 0xc000c89080?})
        /build/snapcraft-juju-a98554e81ff3bbc6574e9e4859860106/parts/juju/build/cmd/juju/status/status.go:507 +0xd9
github.com/juju/cmd/v3.(*Output).writeFormatter(0x4162a5?, 0xc000115020, 0x65a4b27?, {0x62c93a0?, 0xc000c89080?})
        /build/snapcraft-juju-a98554e81ff3bbc6574e9e4859860106/parts/juju/build/vendor/github.com/juju/cmd/v3/output.go:207 +0xf2
github.com/juju/cmd/v3.(*Output).Write(0xc000537158, 0xc000115020, {0x62c93a0, 0xc000c89080})
        /build/snapcraft-juju-a98554e81ff3bbc6574e9e4859860106/parts/juju/build/vendor/github.com/juju/cmd/v3/output.go:182 +0x58
github.com/juju/juju/cmd/juju/status.(*statusCommand).runStatus(0xc000537080, 0xc000115020)
        /build/snapcraft-juju-a98554e81ff3bbc6574e9e4859860106/parts/juju/build/cmd/juju/status/status.go:357 +0x719
github.com/juju/juju/cmd/juju/status.(*statusCommand).Run(0xc0007f4c00?, 0x6f60c01?)
        /build/snapcraft-juju-a98554e81ff3bbc6574e9e4859860106/parts/juju/build/cmd/juju/status/status.go:428 +0x99
github.com/juju/juju/cmd/modelcmd.(*modelCommandWrapper).Run(0xc0007f4c00, 0xc000115020)
        /build/snapcraft-juju-a98554e81ff3bbc6574e9e4859860106/parts/juju/build/cmd/modelcmd/modelcommand.go:680 +0x123
github.com/juju/juju/cmd/modelcmd.(*baseCommandWrapper).Run(0xc0003e76b0, 0xc000115020)
        /build/snapcraft-juju-a98554e81ff3bbc6574e9e4859860106/parts/juju/build/cmd/modelcmd/base.go:569 +0x95
github.com/juju/cmd/v3.(*SuperCommand).Run(0xc000536f20, 0xc000115020)
        /build/snapcraft-juju-a98554e81ff3bbc6574e9e4859860106/parts/juju/build/vendor/github.com/juju/cmd/v3/supercommand.go:557 +0x369
github.com/juju/cmd/v3.Main({0x6f36750, 0xc000536f20}, 0xc000115020, {0xc0007f58c0, 0x3, 0x3})
        /build/snapcraft-juju-a98554e81ff3bbc6574e9e4859860106/parts/juju/build/vendor/github.com/juju/cmd/v3/cmd.go:468 +0x232
github.com/juju/juju/cmd/juju/commands.jujuMain.Run({0xc0002e6480?}, {0xc000070100, 0x4, 0x4})
        /build/snapcraft-juju-a98554e81ff3bbc6574e9e4859860106/parts/juju/build/cmd/juju/commands/main.go:216 +0xa07
github.com/juju/juju/cmd/juju/commands.Main(...)
        /build/snapcraft-juju-a98554e81ff3bbc6574e9e4859860106/parts/juju/build/cmd/juju/commands/main.go:126
main.main()
        /build/snapcraft-juju-a98554e81ff3bbc6574e9e4859860106/parts/juju/build/cmd/juju/main.go:29 +0x6c

Notes & References

No response

[jujubot]

This bug has now been triaged. JUJU-8136

[adisazhar123]

@ca-scribner thanks for reporting this. I was able to reproduce it locally on my machine. From my initial investigation, the panic points me to this method call (ref) in naturalsort library. It appears that the given value, 23456789012345678901234567890, is larger than what an int64 can hold at 9223372036854775807 (ref).

My initial thoughts are we could replace the int with a big.Int. Rather than calling strconv.Atoi() (ref), we instantiate a big.Int, and call SetString() on the object. For the comparison (ref), we'd have to use Cmp().

@wallyworld what are your thoughts on the approach above?

[ca-scribner]

I have no context and haven't looked into it in the codebase, but it feels strange this would need parsing to an int at all given that it is a name

[wallyworld]

I have no context and haven't looked into it in the codebase, but it feels strange this would need parsing to an int at all given that it is a name

It's a name but one with a potential number embedded in it, so the number is being extracted for a natural sort. It's just that the number is way larger than the code author expected people to use.

[wallyworld]

@ca-scribner thanks for reporting this. I was able to reproduce it locally on my machine. From my initial investigation, the panic points me to this method call (ref) in naturalsort library. It appears that the given value, 23456789012345678901234567890, is larger than what an int64 can hold at 9223372036854775807 (ref).

My initial thoughts are we could replace the int with a big.Int. Rather than calling strconv.Atoi() (ref), we instantiate a big.Int, and call SetString() on the object. For the comparison (ref), we'd have to use Cmp().

@wallyworld what are your thoughts on the approach above?

Libraries shouldn't panic. That the naturalsort lib does is unfortunate. We what need to do is:

1. bring the library in tree under core - it doesn't pay it's way as a separate library and we are aggressively doing that for 4.0
2. eliminate the panic
3. big int can still fail with SetString I'm of the view that if an int64 is not large enough, we just default back to an alphabetical sort. It's a corner case to have a number that big in practice and humans won't really notice at that point anyway