Join GitHub today
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
Make required packages variable and non-global #6783
Conversation
|
!!build!! |
|
!!build!! |
|
!!onemoretime!! |
|
!!cmon!! |
| + if a == arch.ARM64 { | ||
| + // ARM64 doesn't support legacy BIOS so it requires Extensible Firmware | ||
| + // Interface. | ||
| + requiredPackages = append(requiredPackages, "qemu-efi") |
howbazaar
Jan 9, 2017
Owner
Are we sure that adding it to the end is OK given that we have to have 'qemu-kvm' needs to be before 'libvirt-bin'?
I'd also like to see the qemu packages in the initial list together, so at the start is fine.
reedobrien
Jan 9, 2017
Contributor
The issue with order is an upstart issue, it apparently doesn't reload libvirt after installing qemu-kvm, but no big deal. Reordered to put all the qemu packages first. If on ARM64 the efi package is prepended to the slice. It has no dependencies so it should be fine up front.
howbazaar
approved these changes
Jan 10, 2017
I would like to see the test confirm the packages for both amd64 and arm64 on every platform. So explicitly pass arch into getContainerInstance.
| + {"genisoimage"}, | ||
| + {"libvirt-bin"}, | ||
| + } | ||
| + if runtime.GOARCH == arch.ARM64 { |
howbazaar
Jan 10, 2017
Owner
Perhaps the test should be parameterised so it doesn't depend on runtime.GOARCH, but explicit checking.
reedobrien
Jan 11, 2017
•
Edited 1 time
-
reedobrien
Jan 11, 2017
Contributor
-
Jan 11, 2017
Made it depend on arch.HostArch() rather than runtime as the rest of the test does.
There are tests in container.kvm that explicitly test the package values regardless of the architecture the tests run on.
I took a few minutes and looked at doing it here too. But passing in the arch required also building the container on the said arch and then tools. So I just fixed it to match the rest of the test where it is used.
|
$$merge$$ |
|
Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju |
|
Build failed: Tests failed |
reedobrien commentedJan 8, 2017
ARM64 requires UEFI to run under KVM as there is no support for legacy
BIOS on the platform. This commint ensures the correct package is added
to the "require packages" on that architecture.
QA: I don't yet have access to an arm system to live test on. But there's unit test coverage. So make sure the tests pass in container/kvm.