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

ppc64le: kata-env fails due to missing vendor field #865

Merged
merged 1 commit into from
Nov 28, 2018
Merged

ppc64le: kata-env fails due to missing vendor field #865

merged 1 commit into from
Nov 28, 2018
+82 −41

Conversation

nitkon
Copy link
Contributor

@nitkon nitkon commented Oct 29, 2018

There is no vendor field in /proc/cpuinfo contents
on ppc64le. Make sure the check is only for
arm and amd64.

Fixes: #864

Signed-off-by: Nitesh Konkar niteshkonkar@in.ibm.com

grahamwhaley
grahamwhaley approved these changes Oct 29, 2018
View reviewed changes
Copy link
Contributor

@grahamwhaley grahamwhaley left a comment

Choose a reason for hiding this comment

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

Ah, I see. I think this
lgtm
/cc @jodh-intel

@marcov
Copy link
Contributor

marcov commented Oct 30, 2018

hi @nitkon, the Travis ppc job is failing with:

cli/kata-check.go:55:2:warning: unused variable or constant genericCPUVendorField (varcheck)
cli/kata-check.go:56:2:warning: unused variable or constant genericCPUModelField (varcheck)

Maybe you could move those const in a dedicate file and use // +build amd64 ?

@nitkon
Copy link
Contributor Author

nitkon commented Nov 1, 2018

@jodh-intel @marcov: Fixed Travis ppc job.

@nitkon
Copy link
Contributor Author

nitkon commented Nov 1, 2018

Can someone please start the Jenkins CI job.

@grahamwhaley
Copy link
Contributor

/test

@marcov
Copy link
Contributor

marcov commented Nov 2, 2018

@nitkon: CI errors are because of the failing test TestCheckGetCPUInfo

@sboeuf
Copy link

sboeuf commented Nov 5, 2018

@nitkon this PR is ready to be merged as you got all the needed approvals, but please fix the CI!

@nitkon nitkon force-pushed the kata-env branch 4 times, most recently from 2a5223e to d248659 Compare November 23, 2018 15:04
@nitkon
Copy link
Contributor Author

nitkon commented Nov 23, 2018

Is the failure in CI due to the warnings of func genericTestGetCPUDetails and func genericGetExpectedHostDetails being unused? I see arm64 defining its own
TestGetCPUDetails as well.

@grahamwhaley
Copy link
Contributor

@nitkon yes, those look like the failures in Travis - static code check fails:

/home/travis/gopath/src/github.com/kata-containers/runtime/virtcontainers/utils
cli/kata-check_test.go:140:6:warning: func genericTestGetCPUDetails is unused (U1000) (unused)
cli/kata-env_test.go:244:6:warning: func genericGetExpectedHostDetails is unused (U1000) (unused)

You might need a go-guru to help figure out how to not trigger that for what I suspect are overloaded arch-dep funcs?

@nitkon
Copy link
Contributor Author

nitkon commented Nov 23, 2018

@grahamwhaley : Okay. Also is it okay to put arch specific checks in genericTestGetCPUDetails (I guess no) ?
So the only option is to have a ppc64le specific TestGetCPUDetails and not call genericTestGetCPUDetails and not get a warning as well.

@grahamwhaley
Copy link
Contributor

/me pulls in @jodh-intel @sboeuf - afaik we tend to put arch code into arch postfixed source files (which go understands), like https://github.com/kata-containers/runtime/blob/master/cli/kata-check_amd64.go
We don't normally use golang build constraints afaik.

@nitkon
Copy link
Contributor Author

nitkon commented Nov 23, 2018

Yup, that's why I put the code in cli/kata-check_ppc64le_test.go in this PR.

@marcov
Copy link
Contributor

marcov commented Nov 23, 2018

@nitkon, I see the code for the ppc and generic functions is exactly the same, apart from few variables values, so it's worth doing some refactoring and keep calling the generic functions from the ppc specific code.

You just need to do adapt the generic functions to accept some extra arguments:

the func genericTestGetCPUDetails(t *testing.T, vendorname string, data testdata)
func genericGetExpectedHostDetails(tmpdir string, cpuVendor string) (HostInfo, error)

@nitkon nitkon force-pushed the kata-env branch 7 times, most recently from 50fa81c to 2cd5f02 Compare November 24, 2018 16:14
@nitkon
Copy link
Contributor Author

nitkon commented Nov 24, 2018

The Travis-CI build is now passing. Can someone run the jenkins-ci-ubuntu for me? Thanks.

@nitkon
Copy link
Contributor Author

nitkon commented Nov 25, 2018

CI failing because of

latest: Pulling from library/fedora
d0483bd5a554: Pulling fs layer

Stderr: error pulling image configuration: unknown blob

--- FAIL: TestIntegration (12.75s)
    main_test.go:60: failed to pull docker image: fedora
FAIL

network issue? /retest ?

@caoruidong
Copy link
Member

/retest

@nitkon
Copy link
Contributor Author

nitkon commented Nov 26, 2018

Some of the test failures I am seeing are due to the following:

Stderr: 
Running command '/usr/bin/docker [docker pull fedora]'
command failed error 'exit status 1'
[docker pull fedora]
Timeout: 600 seconds
Exit Code: 1
Stdout: Using default tag: latest

Stderr: Error response from daemon: unauthorized: authentication required

--- FAIL: TestIntegration (12.34s)
    main_test.go:60: failed to pull docker image: fedora
FAIL

Not sure if the CI is broken or its a network issue.

@marcov
Copy link
Contributor

marcov commented Nov 26, 2018

Could be an hiccup with docker hub. I've manually restarted the failed jobs.

@nitkon
Copy link
Contributor Author

nitkon commented Nov 26, 2018

The jenkins are failing due to the following:

14:15:11 ERROR: Found unexpected /usr/libexec/kata-containers/kata-proxy present
14:15:12 
14:15:12 ===== starting test [memory footprint inside container] =====
14:15:12 command: docker: yes
14:15:12 11189
14:15:12 ERROR: Found unexpected /usr/libexec/kata-containers/kata-proxy present
14:15:12 Executing test: boot times image=ubuntu runtime=kata-runtime units=seconds
14:15:12 command: bc: yes
14:15:12 command: awk: yes
14:15:12 
14:15:12 ===== starting test [boot times] =====
14:15:12 command: docker: yes
14:15:13 11189
14:15:13 ERROR: Found unexpected /usr/libexec/kata-containers/kata-proxy present
14:15:13 
14:15:13 ===== starting test [blogbench] =====
14:15:13 command: docker: yes
14:15:13 11189
14:15:13 ERROR: Found unexpected /usr/libexec/kata-containers/kata-proxy present
14:15:13 11189
14:15:14 ERROR: Found unexpected /usr/libexec/kata-containers/kata-proxy present
14:15:14 ~/workspace/workspace/kata-metrics-runtime-ubuntu-16-04-PR/go/src/github.com/kata-containers/tests
14:15:14 ~/workspace/workspace/kata-metrics-runtime-ubuntu-16-04-PR/go/src/github.com/kata-containers/tests/cmd/checkmetrics ~/workspace/workspace/kata-metrics-runtime-ubuntu-16-04-PR/go/src/github.com/kata-containers/tests

@marcov : Can you restart the jobs, probably. Also these don't have a "Required" tag. Does it signify that it not required for a PR to be reviewed/merged .

@marcov
Copy link
Contributor

marcov commented Nov 26, 2018

Restarting one more time. About the Required tag, I am not sure what's the usual way to interpret it in this repo.

@nitkon
Copy link
Contributor Author

nitkon commented Nov 27, 2018

Thank you! Looks all green now. @marcov @grahamwhaley @jodh-intel

jodh-intel
jodh-intel suggested changes Nov 27, 2018
View reviewed changes
Copy link
Contributor

@jodh-intel jodh-intel left a comment

Choose a reason for hiding this comment

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

Hi @nitkon - please can you tidy up this PR by removing all the commented out code. Thanks.

cli/kata-env_amd64_test.go Outdated
expectedVendor := "moi"
expectedModel := "awesome XI"
return genericGetExpectedHostDetails(tmpdir, expectedVendor, expectedModel)
// return genericGetExpectedHostDetails(tmpdir)
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove all the commented out code on this PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jodh-intel : Oops, sorry for that. I have re-pushed the PR.

@jodh-intel
Copy link
Contributor

Added dnm to avoid inadvertent merging - see my comments.

@nitkon
ppc64le: kata-env fails due to missing vendor field
ca58bb4 
There is no vendor field in /proc/cpuinfo contents
on ppc64le. Make sure to return "" for vendor field
for ppc64le and fix all the corresponding testcases
as well.

Fixes: kata-containers#864

Signed-off-by: Nitesh Konkar niteshkonkar@in.ibm.com
@nitkon
Copy link
Contributor Author

nitkon commented Nov 27, 2018

Jenkins needs to be started?

@grahamwhaley
Copy link
Contributor

/test

@nitkon
Copy link
Contributor Author

nitkon commented Nov 28, 2018

jenkins-ci-fedora-27 is failing because of

" name=kata-proxy pid=32201 sandbox=38c5dbd446d132839f003a7ae401460fb3da53e218567b391c3ad09532f6c549 source=agent
time="2018-11-27T16:00:16.387115726Z" level=fatal msg="failed to handle exit signal" error="close unix @->/run/vc/vm/b14c1466ee90361ee323721049456add4219a45c70dca95fae184fb239f2d803/kata.sock: use of closed network connection"

Manually restarting just that test would probably help.

@jodh-intel
Copy link
Contributor

lgtm

@jodh-intel jodh-intel merged commit c2eea35 into kata-containers:master Nov 28, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@jodh-intel jodh-intel jodh-intel approved these changes

@amshinde amshinde amshinde approved these changes

@grahamwhaley grahamwhaley grahamwhaley approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@nitkon @marcov @grahamwhaley @sboeuf @caoruidong @jodh-intel @amshinde