[2.7] diff-bundle results in runtime error

Bug #1882573 reported by Alexander Balderson on 2020-06-08
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Achilleas Anagnostopoulos
Achilleas Anagnostopoulos

Bug Description

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

$ juju --version

$ 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
 /build/juju/parts/juju/go/src/github.com/juju/juju/cmd/juju/main.go:37 +0x8c

i also get this with 2.8.0-bionic-amd64

I'll include some bundles shortly

deployed bundle with redacted livepatch key:

diffed bundle:

Tim Penhey (thumper) wrote :

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.

Changed in juju:
importance: Undecided → Medium
assignee: nobody → Achilleas Anagnostopoulos (achilleasa)
status: New → In Progress
milestone: none → 2.7.7

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.

Pete Vander Giessen (petevg) wrote :

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).

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

Changed in juju:
status: In Progress → Fix Committed

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

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!

Changed in juju:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers