[2.7] diff-bundle results in runtime error

[Alexander Balderson]

Trying to run diff-bundle against a deployed model results in a runtime error:

$ juju --version
2.7.6-bionic-amd64

$ juju diff-bundle kubernetes_bundle.yaml
panic: runtime error: index out of range

goroutine 1 [running]:
github.com/juju/juju/vendor/github.com/juju/bundlechanges.relationFromEndpoints(0xc4208bd560, 0x2, 0x2, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, ...)
 /build/juju/parts/juju/go/src/github.com/juju/juju/vendor/github.com/juju/bundlechanges/diff.go:405 +0x206
github.com/juju/juju/vendor/github.com/juju/bundlechanges.(*differ).diffRelations(0xc420902360, 0xc420903020)
 /build/juju/parts/juju/go/src/github.com/juju/juju/vendor/github.com/juju/bundlechanges/diff.go:197 +0xf4
github.com/juju/juju/vendor/github.com/juju/bundlechanges.(*differ).build(0xc420902360, 0xc420902360, 0x0, 0x3c91bc0)
 /build/juju/parts/juju/go/src/github.com/juju/juju/vendor/github.com/juju/bundlechanges/diff.go:72 +0x5f
github.com/juju/juju/vendor/github.com/juju/bundlechanges.BuildDiff(0xc420a04c80, 0xc4200e95c0, 0x0, 0x3c91bc0, 0xc420902db0, 0x0, 0x0, 0x0)
 /build/juju/parts/juju/go/src/github.com/juju/juju/vendor/github.com/juju/bundlechanges/diff.go:61 +0xab
github.com/juju/juju/cmd/juju/application.(*bundleDiffCommand).Run(0xc420b98240, 0xc4202beeb0, 0x0, 0x0)
 /build/juju/parts/juju/go/src/github.com/juju/juju/cmd/juju/application/bundlediff.go:138 +0x1e4
github.com/juju/juju/cmd/modelcmd.(*modelCommandWrapper).Run(0xc420ac5ec0, 0xc4202beeb0, 0xc420ac5ec0, 0xc420902990)
 /build/juju/parts/juju/go/src/github.com/juju/juju/cmd/modelcmd/modelcommand.go:608 +0x11c
github.com/juju/juju/cmd/modelcmd.(*baseCommandWrapper).Run(0xc42042f030, 0xc4202beeb0, 0x0, 0x0)
 /build/juju/parts/juju/go/src/github.com/juju/juju/cmd/modelcmd/base.go:471 +0xab
github.com/juju/juju/vendor/github.com/juju/cmd.(*SuperCommand).Run(0xc4208e7900, 0xc4202beeb0, 0xc4202beeb0, 0x0)
 /build/juju/parts/juju/go/src/github.com/juju/juju/vendor/github.com/juju/cmd/supercommand.go:516 +0x2cc
github.com/juju/juju/vendor/github.com/juju/cmd.Main(0x3cd9640, 0xc4208e7900, 0xc4202beeb0, 0xc42004c0d0, 0x2, 0x2, 0x326b860)
 /build/juju/parts/juju/go/src/github.com/juju/juju/vendor/github.com/juju/cmd/cmd.go:379 +0x2d9
github.com/juju/juju/cmd/juju/commands.main.Run(0x385d9e0, 0xc42004c0c0, 0x3, 0x3, 0x0)
 /build/juju/parts/juju/go/src/github.com/juju/juju/cmd/juju/commands/main.go:143 +0x1af
github.com/juju/juju/cmd/juju/commands.Main(0xc42004c0c0, 0x3, 0x3, 0xc4201120c0)
 /build/juju/parts/juju/go/src/github.com/juju/juju/cmd/juju/commands/main.go:89 +0x4b
main.main()
 /build/juju/parts/juju/go/src/github.com/juju/juju/cmd/juju/main.go:37 +0x8c

[Alexander Balderson (asbalderson)]

i also get this with 2.8.0-bionic-amd64

I'll include some bundles shortly

[Alexander Balderson (asbalderson)]

deployed bundle with redacted livepatch key:
https://pastebin.ubuntu.com/p/Q89kdV9rY6/

diffed bundle:
https://pastebin.ubuntu.com/p/dw6MXfwBkk/

[Tim Penhey (thumper)]

The key bug is the diff's handling of the relations.

The first bundle has all relations defined with endpoints, but the second one has

- - canonical-livepatch
  - etcd

The code needs to be resiliant to this, but currently isn't.

[Achilleas Anagnostopoulos (achilleasa)]

There are two different approaches for handling this but both have caveats and I am not sure which one is the best way to move forward:

1) When diffing, perform a quick scan of the bundle that we parsed from disk and emit an error for relations that lack an endpoint. The argument for such an approach is that we should probably be strict for diffs in particular as a relation might be applicable to several endpoints and we shouldn't try to infer them. This approach would contradict our docs at https://juju.is/docs/bundle-reference:

# Each side of a relation (each line) has the format
# '<application>:<endpoint>', where 'application' must also be represented
# under the 'applications' element. Including the endpoint is not stricly
# necessary as it might be determined automatically. However, it is best
# practice to do so.

However, we could interpret the "might be determined automatically" as not applicable when diffing; in which case displaying an error would be OK.

2) Try to infer the endpoint from the controller's view of the model and fall back to displaying an error in case of ambiguities. In the attached bundle files, it seems that all the missing endpoints are "juju-info" (containerd, canonical-livepatch, kubernetes-worker etc). We could add extra logic into the bundlechanges package to handle the "juju-info" endpoint but I am not that keen on doing so because we are adding magic (though one could argue that auto-injecting the "juju-info" relation is already magical) to the package. We probably also have to duplicate any added functionality into pylibjuju.

@petevg @thumper: Any recommendation? In my view, the first solution is cleaner but would appreciate some info before proceeding further.

[Pen Gale (pengale)]

I like option #1. We can always add the magic in later if needed. For now, simple is best.

(I'd encourage @asbalderson to comment if they feel that a fix sans magic would not actually count as a fix for this bug).

[Achilleas Anagnostopoulos (achilleasa)]

PR https://github.com/juju/juju/pull/11692 includes a fix for the 2.7 branch.

[Achilleas Anagnostopoulos (achilleasa)]

PR https://github.com/juju/juju/pull/11698 forward ports the fix to 2.8

[Alexander Balderson (asbalderson)]

I'm good with this fix.

The errors i was trying to track down with the diff were because of the ambiguous relations.

Thank you!