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

Kata-env and Kata-check are not architecture-independent #349

Closed
Pennyzct opened this issue May 30, 2018 · 9 comments
Closed

Kata-env and Kata-check are not architecture-independent #349

Pennyzct opened this issue May 30, 2018 · 9 comments
Assignees

Comments

@Pennyzct
Copy link
Contributor

Description of problem

For now, the functionality of kata-check and kata-env are missing in Arm architecture. Error outputs are as follows:

# github.com/kata-containers/runtime/cli
./kata-check.go:250:65: undefined: archKernelParamHandler
./kata-check.go:271:27: undefined: archRequiredCPUFlags
./kata-check.go:272:27: undefined: archRequiredCPUAttribs
./kata-check.go:273:27: undefined: archRequiredKernelModules
./kata-check.go:285:10: undefined: archHostCanCreateVMContainer
./kata-env.go:179:26: undefined: archRequiredCPUFlags
./kata-env.go:180:26: undefined: archRequiredCPUAttribs
./kata-env.go:181:26: undefined: archRequiredKernelModules
Makefile:310: recipe for target '/home/Wei/penny/go/src/github.com/kata-containers/runtime/kata-runtime' failed
make: *** [/home/Wei/penny/go/src/github.com/kata-containers/runtime/kata-runtime] Error 2

It could come from following aspects:

  1. Struct vmContainerCapableDetails was architecture-relevant, for example, there were no tag showing virtualization capability in /proc/cpuinfo in aarch64, so variable requiredCPUFlags could be no use. flag filed in /proc/cpuinfo may only amd64-specific.
  2. Struct CPUInfo was also architecture-relevant, e.g. variable vendor and model are meaningless for arm architecture. Because func getCPUDetails() is related to CPUInfo, it may not be included in universal file cli/utils.go
  3. Func hostIsVMContainerCapable() is only suitable for amd64 to check host's capability of creating a VM container

We think that we could move the architecture-relevant variable and func to files with specific architecture suffixes. e.g. func getCPUFlags, checkCPUFlags, hostIsVMContainerCapable and const cpuFlagsTag should be transferred to file kata-check_amd64.go.

@bergwolf @gnawux @jodh-intel @Weichen81

@jodh-intel jodh-intel self-assigned this May 30, 2018
@jodh-intel
Copy link
Contributor

Hi @Pennyzct - thanks for raising. We had recently noticed this in reviewing #286. That PR reworks those commands for PPC64el but also improves the situation for ARM64. I have a local branch that builds on #286 that will fix this bug for ARM64.

However, we really need the Kata Project's CI to support both PPC64el and ARM64. Could you help with the latter maybe?

/cc @egernst, @grahamwhaley, @chavafg.

@Pennyzct
Copy link
Contributor Author

@jodh-intel Hi~I have read the Arm64-relevant code in this PR. Maybe I can refine it after merging. I'm very happy to help with the CI project.😊

@gnawux
Copy link
Member

gnawux commented May 30, 2018

I am setting up an ARM64 instance on packet.net, and if everything goes well, we will use it for the ARM64 integration test @jodh-intel

@jodh-intel
Copy link
Contributor

\o/ - this is great news! Thanks @Pennyzct and @gnawux 😄

@Weichen81
Copy link
Contributor

@gnawux @jodh-intel @Pennyzct Thanks! We will set up a local machine for Kata CI too : )

@jodh-intel
Copy link
Contributor

Thanks @Weichen81!! Ideally, we'd "gate" all PRs to ensure all tests pass on all architectures before the PR is merged.

A local CI would be helpful but you'll then have to raise PRs to fix the build after it's broken which probably isn't ideal for you.

jodh-intel added a commit to jodh-intel/runtimes that referenced this issue Jun 1, 2018
As we still don't have an ARM CI, the ARM64 build broke.

Fixes kata-containers#349.

Signed-off-by: James O. D. Hunt <[email protected]>
@jodh-intel
Copy link
Contributor

Hi @Pennyzct - FYI, I've got a branch that fixes the ARM64 build (https://github.com/jodh-intel/runtimes/tree/fix-arm64-build) but I still need to fix the tests (hopefully later today).

jodh-intel added a commit to jodh-intel/runtimes that referenced this issue Jun 1, 2018
As we still don't have an ARM CI, the ARM64 build broke.

Fixes kata-containers#349.

Signed-off-by: James O. D. Hunt <[email protected]>
jodh-intel added a commit to jodh-intel/runtimes that referenced this issue Jun 1, 2018
As we still don't have an ARM CI, the ARM64 build broke.

Fixes kata-containers#349.

Signed-off-by: James O. D. Hunt <[email protected]>
jodh-intel added a commit to jodh-intel/runtimes that referenced this issue Jun 1, 2018
Fix ARM64 build which silently broken (as we still don't have an ARM CI).

Fixes kata-containers#349.

Signed-off-by: James O. D. Hunt <[email protected]>
@jodh-intel
Copy link
Contributor

jodh-intel commented Jun 1, 2018

PR raised as #362. I'd appreciate it if you could validate on your systems.

@Weichen81
Copy link
Contributor

@jodh-intel Thanks, we will validate it on next Monday : )

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants