Skip to content
This repository was archived by the owner on May 7, 2021. It is now read-only.

Enable kola on arm64 machines#661

Merged
dm0- merged 5 commits into
coreos:masterfrom
glevand:for-merge-arm64-kola
Aug 22, 2017
Merged

Enable kola on arm64 machines#661
dm0- merged 5 commits into
coreos:masterfrom
glevand:for-merge-arm64-kola

Conversation

@glevand

@glevand glevand commented Aug 3, 2017

Copy link
Copy Markdown
Contributor

Allows kola to run on arm64 hosts. Also includes a patch that increases the debug output of dnsmasq.

Comment thread platform/local/dnsmasq.go Outdated
`
)

var plog = capnslog.NewPackageLogger("github.com/coreos/mantle", "local")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The second argument should be platform/local.

Comment thread kola/harness.go Outdated
// architecture returns the machine architecture of the given platform.
func architecture(pltfrm string) string {
nativeArch := "amd64"
nativeArch := system.PortageArch()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If kola is running on arm64 but pltfrm is e.g. aws, architecture should still return amd64. It looks like the right place to fix the default selection is in sdk.DefaultBoard(). That will fix QEMUOptions.Board via defaultTargetBoard in cmd/kola/options.go.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

OK, please check what I have in the updated patch.

"qemu-system-aarch64",
"-machine", "virt",
"-cpu", "cortex-a57",
"-m", "2048",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Just to double-check, this is necessary with images built with ./build_image prod? On amd64, dev images seem to need more than 1024 MB (coreos/bugs#1594) but prod images don't.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, the prod image will run out of memory. Strange behavior and unexpected errors start to occur.

@glevand glevand force-pushed the for-merge-arm64-kola branch from cc5f601 to acdff44 Compare August 4, 2017 20:15
@glevand

glevand commented Aug 4, 2017

Copy link
Copy Markdown
Contributor Author

Rebased to latest, addressed comments above.

@glevand glevand force-pushed the for-merge-arm64-kola branch from acdff44 to edce199 Compare August 17, 2017 17:09
@glevand

glevand commented Aug 17, 2017

Copy link
Copy Markdown
Contributor Author

Rebase to latest.

Comment thread kola/harness.go Outdated
nativeArch := "amd64"
if pltfrm == "qemu" && QEMUOptions.Board != "" {
nativeArch = strings.SplitN(QEMUOptions.Board, "-", 2)[0]
if pltfrm == "qemu" {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why does this drop QEMUOptions.Board != ""?

Comment thread kola/harness.go Outdated
return boardToArch(QEMUOptions.Board)
}
return nativeArch
return boardToArch(sdk.DefaultBoard())

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This will still return arm64 on an ARM host if pltfrm is e.g. aws, which is incorrect. My point in #661 (comment) was that the sdk.DefaultBoard() change should be sufficient without altering kola.architecture(), since kola running on ARM will then default QEMUOptions.Board to arm64-usr. That will fix the default on qemu without affecting platforms which are amd64-only.

Though, come to think of it, architecture() should also special-case packet to look at PacketOptions.Board.

To aid in debugging dnsmasq problems configure dnsmasq to use
verbose logging when the command line option '--log-level=DEBUG'
is passed (plog level is at capnslog.DEBUG).

The previous dnsmasq logging verbosity (quiet) is retained for
lower plog levels.

Signed-off-by: Geoff Levand <geoff@infradead.org>
In preparation for running kola on other architectures.

Signed-off-by: Geoff Levand <geoff@infradead.org>
Signed-off-by: Geoff Levand <geoff@infradead.org>
arm64 guest machines seem to run out of memory with 1024 MiB of
RAM, so increase to 2048 MiB.

Signed-off-by: Geoff Levand <geoff@infradead.org>
To allow kola to run on arm64 machines.

Signed-off-by: Geoff Levand <geoff@infradead.org>
@glevand glevand force-pushed the for-merge-arm64-kola branch from edce199 to b8a4579 Compare August 21, 2017 19:46
@glevand

glevand commented Aug 21, 2017

Copy link
Copy Markdown
Contributor Author

In the current patch set the only change I've made is for sdk.DefaultBoard() to use system.PortageArch() for its default value, which will be the host machine's arch (from runtime.GOARCH). QEMUOptions.Board should now be set to either a user specified --board value, or a default of the host machine.

kola.architecture() should now return amd64 for gce, aws, and esx, and either the --board value or the host machine's arch for qemu.

With my new patch kola/harness: Use PacketOptions.Board packet will have the same value as qemu, either the --board value or the host machine's arch. Does it make more sense to have a default of amd64 for packet? It doesn't seem it would be easy to arrange that though.

@bgilbert

Copy link
Copy Markdown
Contributor

Does it make more sense to have a default of amd64 for packet? It doesn't seem it would be easy to arrange that though.

I think it's fine to default Packet to the host platform. We should arguably switch the default --packet-plan to baremetal_2a on ARM, but we can leave that for another day.

@bgilbert bgilbert left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good, thanks.

@dm0- dm0- left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@dm0- dm0- merged commit 4fb9701 into coreos:master Aug 22, 2017
@glevand glevand deleted the for-merge-arm64-kola branch August 22, 2017 23:09
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants