Skip to content
This repository has been archived by the owner on Jun 28, 2024. It is now read-only.

Adding ppc64le support for CI #884

Merged
merged 1 commit into from
Nov 5, 2018

Conversation

sudeeshjohn
Copy link
Contributor

Fixes: #883

Signed-off-by: Sudeesh John [email protected]

@nitkon
Copy link
Contributor

nitkon commented Nov 2, 2018

Lgtm.

@grahamwhaley
Copy link
Contributor

Changes lgtm. The CI is noting your PR does not meet all of our formatting requirements - and gives you a hint and link :-) ...

Found 1 commit between commit 9841d31d7426fe2ba5729ca5074ff9abaafca976 and branch master
ERROR: Commit 9841d31d7426fe2ba5729ca5074ff9abaafca976: Failed to find subsystem in subject: "Adding ppc64le support for CI"
ERROR: checkcommits failed. See the document below for help on formatting
commits for the project.
https://github.com/kata-containers/community/blob/master/CONTRIBUTING.md#patch-format

@sudeeshjohn
Copy link
Contributor Author

@grahamwhaley I guess this can group under "build" ?

@grahamwhaley
Copy link
Contributor

@sudeeshjohn yep, that would be fine.
I don't remember if checkcommits is going to complain that you don't have much 'body' to your commit - you may wish to add a single line explanation just above the 'Fixes' line. I know, with a small patch there is not much to say, but a one line summary, even if it reflects the same as the subject line, is useful when looking at git logs.
And.. .thanks for the contribution! :-)

@sudeeshjohn
Copy link
Contributor Author

updated commit message.

.ci/setup.sh Outdated
@@ -67,6 +67,9 @@ enable_nested_virtualization() {
aarch64)
info "CI running in bare machine"
;;
ppc64le)
Copy link
Contributor

Choose a reason for hiding this comment

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

just a hint, to avoid repeating the same info message you could do:

aarch64 | ppc64le)
		info "CI running in bare machine"
		;;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@marcov thanks for the hint .. updated the patch accordingly

@marcov
Copy link
Contributor

marcov commented Nov 5, 2018

Once we have this merge, it would be useful to have a bare metal server set up that could run tests for PPC, similarly with what we are doing for ARM (reference: kata-containers/ci#30)
lgtm

Approved with PullApprove

@marcov
Copy link
Contributor

marcov commented Nov 5, 2018

..and thanks @sudeeshjohn for your contribution.

@jodh-intel
Copy link
Contributor

jodh-intel commented Nov 5, 2018

lgtm

Approved with PullApprove

@nitkon
Copy link
Contributor

nitkon commented Nov 5, 2018

@marcov: Yes, we are in the process of getting a bare metal Power server with a public IP for our CI. Are there any other specific requirements for a server that we may miss for CI? (going through the link you provided as well).

including ppc64le arch too into the ci scripts

Fixes: kata-containers#883

Signed-off-by: Sudeesh John [email protected]
@marcov
Copy link
Contributor

marcov commented Nov 5, 2018

@marcov: Yes, we are in the process of getting a bare metal Power server with a public IP for our CI. Are there any other specific requirements for a server that we may miss for CI? (going through the link you provided as well).

That's cool! I can't see any specific requirements, but @chavafg, @grahamwhaley and @jodh-intel may have a better insight about it.

@marcov
Copy link
Contributor

marcov commented Nov 5, 2018

Running tests just to get green checks, even if this PR shoudn't have broken anything
/test

@grahamwhaley
Copy link
Contributor

There are no really hard/strict requirements that I'm aware of. You need the 'slave' set up enough to be a Jenkins slave, and be able to launch our jenkins script.

We should maybe clarify if this is a single slave machine that will be tied into our kata jenkins master, of if this is maybe a single or pair of machines that will run a complete separate Jenkins instance just for ppc64.
Also, we should clarify if this will run the QA CI (tests), or the metrics CI (performance), or both. I think just the QA CI to begin with at least.

We currently run the x86 QA CI on oneshot cloud instances. That gives us a nice degree of isolation from a test run corrupting somehow a machine (it does happen). I wrote up some notes over here a while back. I also wrote up and coded a way to use oneshot VMs on the machines to alleviate this here. Although I put that together for running the metrics CI, I believe that would work for QA CI as well.
The ARM folks could do neither of those (cloud instance or VMs), so have put together some scripts to try to clean up the machine back to a known state after each CI run. To note, I am about to utilise those on the x86 metrics machines, as I've come to the conclusion that the nested VMs create to much variance in the metrics results to be useful. They (the nested VMs) should still be useable for QA CI though (which are not performance sensitive).

I am also about to post some ansible scripts that can deploy a machine with enough bits to run either a jenkins slave directly, or setup the nested VMs. Not quite there yet - but if you could do with them, nudge me, and I could push a prelim branch you can stare at.

@chavafg
Copy link
Contributor

chavafg commented Nov 5, 2018

lgtm

Hi @nitkon, in addition to the good summary of @grahamwhaley, our current VMs are launched with 4 vCPUs and 16 GB of memory. I remember that CPU tests failed when the machine had less than 4 CPUs, so it would be good to at least match that number. I the case of the memory, I think 8GB has also worked in the past. Please let me know when you have the resource, I can help you set up as a slave.

Approved with PullApprove

@chavafg chavafg merged commit 46f4039 into kata-containers:master Nov 5, 2018
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.

6 participants