CI: Fix tests #40

jcvenegas · 2018-01-22T19:24:16Z

This PR adds some fixes to make work travis.

The tests were only running for fedora image creation
centos and euler os need golang to allow build agent
Minor changes after scripts: Arch dependent repo url for fedora #22 to use fedora version defined in config.sh and not the version in host.
/cc @harche

Not all the distros were executed due to typo Signed-off-by: Jose Carlos Venegas Munoz <[email protected]>

Signed-off-by: Jose Carlos Venegas Munoz <[email protected]>

Use OS_VERSION provided by user configuration and not use host version. Also add retries before fail on a request. Signed-off-by: Jose Carlos Venegas Munoz <[email protected]>

harche · 2018-01-23T05:50:17Z

lgtm.

jodh-intel · 2018-01-23T09:43:26Z

rootfs-builder/centos/Dockerfile

@@ -1 +1,7 @@
 FROM centos:7
+
+ADD https://storage.googleapis.com/golang/go1.9.2.linux-amd64.tar.gz /tmp


This looks fine, but we're hard-coding more into these Dockerfile's which is going to make them more difficult to maintain.

I'd rather we fix #31 first (maybe as one of the commits on this PR) to avoid this. These 2 chunks of code are identical too which lends weight to doing that as you could add something like:

FROM centos:@centos_version@ @centos_install_go@

And,

from euleros:@euleros_version@ # EulerOS is similar enough to CentOS to re-use this code. @centos_install_go@

Lastly, there is the issue of the version of golang. The CI systems for the various code repos are using go 1.8.3 so we're in danger of repeating clearcontainers/packaging#257.

Could you create a top-level ./versions.txt containing a go_version= so that it's defined in a single location? It maybe be best to default to 1.8.3 for now and raise an issue to discuss go version support separately.

that is a good point, let me add Dockerfile templates.

jcvenegas · 2018-01-23T20:25:48Z

@jodh-intel please take a look now dockerfile generations is done rootfs. I will send a follow up PR for image-builder dockerfile.

jodh-intel · 2018-01-24T09:19:33Z

rootfs-builder/clearlinux/Dockerfile.am

@@ -0,0 +1,6 @@
+From fedora:27


This line is a bit of a problem. If we leave it like this, it'll probably break at some future date.

We could add one extra variable to each config.sh that specifies the version for that particular distro. For example, fedora/config.sh would become something like:

FEDORA_OS_VERSION=27 OS_VERSION=${OS_VERSION:-${FEDORA_OS_VERSION}}

But we'd then need to do some rework to allow the clearlinux rootfs build to use $FEDORA_OS_VERSION so that the clearlinux Dockerfile could specify:

From @FEDORA_OS_VERSION@

But this all seems rather odd. Clear Linux is the only distro that currently requires a different distro to build its rootfs. So why can't we just build a clearlinux rootfs using... clearlinux:

From clearlinux:@OS_VERSION@

Where @OS_VERSION@ would probably be just "latest"?

Issue raised as #48.

jodh-intel · 2018-01-24T09:20:01Z

rootfs-builder/centos/Dockerfile.am

@@ -0,0 +1,4 @@
+From centos:@OS_VERSION@


Nice. Can you rename these files Dockerfile.in though (as the .am extension implies they are in automake format).

liyingjun · 2018-01-25T06:21:19Z

rootfs-builder/rootfs.sh

+	dir="$1"
+
+	readonly install_go="
+ADD https://storage.googleapis.com/golang/go${GO_VERSION}.linux-amd64.tar.gz /tmp


Would be nice to make the golang URL configurable

Agreed - similar to kata-containers/agent#112.

This could be handled by a follow-on PR though as it would be great to get this PR landed today. WDYT @jcvenegas ?

@liangchenye @jodh-intel yes, I prefer to have this PR merged today, and then fix the URL and clearlinux container usage.

Captured at #47 if someone wants to pick that up.

bergwolf · 2018-01-25T09:37:18Z

travis still fails

jodh-intel · 2018-01-25T10:00:05Z

Yep. It made me laugh actually seeing these two lines:

Step 2/6 : RUN yum -y update && dnf install -y yum git make gcc
   :
/bin/sh: dnf: command not found

To simplify maintaince, create dockerfiles based on templates. This way when golang version is updated it will be done in one place versions.txt. This also allow to allways intall the same version of golang in any dockerfile. Signed-off-by: Jose Carlos Venegas Munoz <[email protected]>

jcvenegas · 2018-01-25T15:27:59Z

@jodh-intel @bergwolf @liyingjun , travis passing.

jodh-intel · 2018-01-25T15:38:33Z

lgtm

jodh-intel · 2018-01-25T15:39:07Z

@devimc - could you review please?

liangchenye · 2018-01-26T10:19:14Z

tests/image_creation.bats

@@ -45,5 +45,9 @@ function build_image()
 }

 @test "Can create euleros image" {
+	if [ "$TRAVIS" = true ]
+	then
+		skip "travis timout, see: https://github.com/kata-containers/osbuilder/issues/46"


I asked a colleague from our company, we don't have a fast server. It is embarrassing.
I have tried the host server of our isula (a containerOS) community. It works good.
I'll use that repo URL instead.

@liangchenye I check in travis docuementation, I added travis_wait now seems all is OK.

Use travis configuration to wait more than 10 min Signed-off-by: Jose Carlos Venegas Munoz <[email protected]>

jodh-intel · 2018-01-26T17:51:01Z

Still...

lgtm

jcvenegas · 2018-01-26T19:27:06Z

@devimc @bergwolf @sameo ready to merge, can you review it and merge it

bergwolf · 2018-01-27T03:09:58Z

lgtm, thanks!

jcvenegas added 3 commits January 22, 2018 14:56

test: Fix tests - run test for all distros

b946770

Not all the distros were executed due to typo Signed-off-by: Jose Carlos Venegas Munoz <[email protected]>

dockerfiles: Add golang to centos anda euleros

a617dd8

Signed-off-by: Jose Carlos Venegas Munoz <[email protected]>

fedora: use OS_VERSION instead of host version

6dcaef7

Use OS_VERSION provided by user configuration and not use host version. Also add retries before fail on a request. Signed-off-by: Jose Carlos Venegas Munoz <[email protected]>

jcvenegas force-pushed the fixes branch from c0f7478 to d549e72 Compare January 22, 2018 21:01

jcvenegas changed the title ~~Fix ci tests~~ CI: Fix tests Jan 22, 2018

jcvenegas force-pushed the fixes branch from d549e72 to 6dcaef7 Compare January 22, 2018 21:19

jodh-intel suggested changes Jan 23, 2018

View reviewed changes

jodh-intel suggested changes Jan 24, 2018

View reviewed changes

jcvenegas force-pushed the fixes branch 2 times, most recently from 3b41b8c to fd77114 Compare January 25, 2018 06:10

liyingjun mentioned this pull request Jan 25, 2018

Add missing packages for centos Dockerfile #44

Closed

liyingjun reviewed Jan 25, 2018

View reviewed changes

liyingjun mentioned this pull request Jan 25, 2018

/osbuilder/rootfs.sh: line 154: make: command not found #43

Closed

jcvenegas force-pushed the fixes branch from fd77114 to 03fa417 Compare January 25, 2018 13:56

jcvenegas force-pushed the fixes branch from 03fa417 to 612754f Compare January 25, 2018 15:03

jodh-intel mentioned this pull request Jan 25, 2018

Build clearlinux rootfs using clearlinux #48

Closed

jodh-intel approved these changes Jan 25, 2018

View reviewed changes

jcvenegas mentioned this pull request Jan 25, 2018

CI: Enable checkcommits #13

Merged

liangchenye mentioned this pull request Jan 26, 2018

travis stop build when building euleros #46

Closed

liangchenye reviewed Jan 26, 2018

View reviewed changes

travis: Wait more than 10 min

45d4e5f

Use travis configuration to wait more than 10 min Signed-off-by: Jose Carlos Venegas Munoz <[email protected]>

bergwolf merged commit 1295bd5 into kata-containers:master Jan 27, 2018

jodh-intel mentioned this pull request Mar 21, 2018

auto-generating the dockerfile #31

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CI: Fix tests #40

CI: Fix tests #40

jcvenegas commented Jan 22, 2018 •

edited

Loading

harche commented Jan 23, 2018

jodh-intel Jan 23, 2018

jcvenegas Jan 23, 2018

jcvenegas commented Jan 23, 2018

jodh-intel Jan 24, 2018

jodh-intel Jan 25, 2018

jodh-intel Jan 24, 2018

liyingjun Jan 25, 2018

jodh-intel Jan 25, 2018

jodh-intel Jan 25, 2018

jcvenegas Jan 25, 2018

jodh-intel Jan 25, 2018

bergwolf commented Jan 25, 2018

jodh-intel commented Jan 25, 2018

jcvenegas commented Jan 25, 2018

jodh-intel commented Jan 25, 2018 •

edited by amshinde

Loading

jodh-intel commented Jan 25, 2018

liangchenye Jan 26, 2018

jcvenegas Jan 26, 2018

jodh-intel commented Jan 26, 2018

jcvenegas commented Jan 26, 2018

bergwolf commented Jan 27, 2018 •

edited by amshinde

Loading

		@@ -1 +1,7 @@
		FROM centos:7

		ADD https://storage.googleapis.com/golang/go1.9.2.linux-amd64.tar.gz /tmp

CI: Fix tests #40

CI: Fix tests #40

Conversation

jcvenegas commented Jan 22, 2018 • edited Loading

harche commented Jan 23, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jcvenegas commented Jan 23, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bergwolf commented Jan 25, 2018

jodh-intel commented Jan 25, 2018

jcvenegas commented Jan 25, 2018

jodh-intel commented Jan 25, 2018 • edited by amshinde Loading

jodh-intel commented Jan 25, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jodh-intel commented Jan 26, 2018

jcvenegas commented Jan 26, 2018

bergwolf commented Jan 27, 2018 • edited by amshinde Loading

jcvenegas commented Jan 22, 2018 •

edited

Loading

jodh-intel commented Jan 25, 2018 •

edited by amshinde

Loading

bergwolf commented Jan 27, 2018 •

edited by amshinde

Loading