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

[JUJU-1067] prevent panic in show controller #14013

Merged
merged 2 commits into from
May 10, 2022

Conversation

hmlanigan
Copy link
Member

Prevent panic seen while doing trying to reproduce an issue seen:

panic: runtime error: index out of range [0] with length 0

goroutine 1 [running]:
github.com/juju/juju/cmd/juju/controller.(*showControllerCommand).convertModelsForShow(0xc000219440, {0xc000143390, 0x9\}
, 0xc000aadb48, \{0xc000c08cf0, 0x2, 0x16\}, \{0x0, 0x0, 0x0\})
        /home/heather/work/src/github.com/juju/juju/cmd/juju/controller/showcontroller.go:480 +0x659

QA steps

Not entirely sure how to reproduce, seen a few times only.

$ juju show-controller

for i, m := range models {
modelDetails := ModelDetails{ModelUUID: m.UUID, OldModelUUID: m.UUID}
if i >= len(modelStatus) {
Copy link
Member

Choose a reason for hiding this comment

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

Small nit on my part. Feel free to disregard.

Move this check to the top of the for loop. Alternatively make the models slice the same length as the modelStatus slice before the for loop and we can remove the defensive programming for each iteration.

Prevent a panic, ensure the index used on a slice isn't larger than the
slice itself.
@hmlanigan
Copy link
Member Author

$$merge$$

@jujubot jujubot merged commit f95b123 into juju:2.9 May 10, 2022
@hmlanigan hmlanigan deleted the show-controller-panic branch May 10, 2022 17:07
jujubot added a commit that referenced this pull request May 16, 2022
#14017

Merge from 2.9 to bring forward:
- #14007 from wallyworld/update-golang-deps
- #14013 from hmlanigan/show-controller-panic
- #14012 from hmlanigan/remove-unit-subordinate
- #13998 from turrisxyz/setup-permissions
- #14011 from ycliuhw/update-ck-overlay
- #14010 from tlm/juju-jenkins-tests
- #14004 from tlm/juju-jenkins-tests
- #14003 from manadart/2.9-close-unit-loggers
- #14001 from jack-w-shaw/JUJU-1050_show_unit_life
- #13890 from juju/dependabot/github_actions/actions/upload-artifact-3
- #14000 from wallyworld/use-go1.18
- #13999 from tlm/juju-jenkins-tests
- #13991 from ycliuhw/fix/lp1968643
- #13997 from tlm/cross-compile
- #13931 from arnodel/juju-885-remove-modelaccess-connection-iface-method
- #13992 from jack-w-shaw/JUJU-1034_reduce_over_verbose_logging_network
- #13993 from hmlanigan/fix-intermittent-fail-testenqueuedoperation
- #13990 from wallyworld/handle-azure-quota-errors
- #13989 from benhoyt/start-worker-if-dead
- #13987 from hpidcock/filter-test-packages

Conflicts:
- cmd/juju/controller/showcontroller.go
- go.mod
- go.sum
- provider/azure/environ_test.go
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants