CI: Test remaining install methods by jodh-intel · Pull Request #280 · kata-containers/documentation

jodh-intel · 2018-10-23T08:26:40Z

Add new CI tests to ensure that the following installation methods are
also tested:

"Automatic" method (kata-manager)
"Scripted" method (kata-doc-to-script)

Note: the "Automatic" method is not the same as the existing
kata-manager test: the existing test executes the "Manual"
installation method (which runs kata-manager to execute the
appropriate distro-specific install guide). However, this new test
executes the install/installing-with-kata-manager.md document, which
subsequently calls the kata-manager script.

Since the "Automatic" and "Scripted" installation methods are designed
to run "standalone" (without requiring any local git repo clones), the
script which runs these new tests has to take care to ensure the
environment they run in is clean. It does this by using the following
approach:

Removes any local Kata github repos from the standard GOPATH
locations (to ensure the scripts do not inadvertently access local
files) [1].
Creates a temporary directory containing:
A copy of itself.
The scripts it generated from the "Automatic" and "Scripted" installation documents.
Re-exec's itself to run the version in the temporary directory,
passing an option that tells itself to simply execute the scripts in
the specified directory.
It then runs the scripts in the directory specified.

[1] - Since the recursive delete of all local Kata github repos is
potentially dangerous, the test will immediately fail if the standard
KATA_DEV_MODE variable is set (since this denotes a developer system)
and will also fail unless the standard CI variable is set (denoting
the script is running in a Continuous Integration environment, such as
JenkinsCI.

Also includes commits to refactor the existing install doc test code and drop the dependency on using golang.

Fixes #278.

Signed-off-by: James O. D. Hunt james.o.hunt@intel.com

jodh-intel · 2018-10-23T08:27:34Z

/cc @marcov, @grahamwhaley.

jodh-intel · 2018-10-23T09:49:25Z

/test

jodh-intel · 2018-10-23T09:56:34Z

This is weird. Even though kata-containers/tests#839 has now landed, Travis is showing:

INFO: Installing xurls utility
package mvdan.cc/xurls/v2: cannot find package "mvdan.cc/xurls/v2" in any of:
	/home/travis/.gimme/versions/go1.7.4.linux.amd64/src/mvdan.cc/xurls/v2 (from $GOROOT)
	/home/travis/gopath/src/mvdan.cc/xurls/v2 (from $GOPATH)

I don't know where that is coming from:

$ go get -v -u "github.com/mvdan/xurls/cmd/xurls"
github.com/mvdan/xurls (download)
Fetching https://mvdan.cc/xurls?go-get=1
Parsing meta tags from https://mvdan.cc/xurls?go-get=1 (status code 200)
get "mvdan.cc/xurls": found meta tag get.metaImport{Prefix:"mvdan.cc/xurls", VCS:"git", RepoRoot:"https://github.com/mvdan/xurls"} at https://mvdan.cc/xurls?go-get=1
mvdan.cc/xurls (download)

jodh-intel · 2018-10-23T14:22:11Z

/test

jodh-intel · 2018-10-23T15:07:19Z

/test

jodh-intel · 2018-10-23T15:46:09Z

Hopefully, the Travis failure will be fixed by #282...

jodh-intel · 2018-10-23T16:33:21Z

#282 now merged.

Let's see what falls over now...

/test

jodh-intel · 2018-10-23T16:50:19Z

Ping @kata-containers/documentation - the doc change on this PR is actually a NOP (specifically to trigger the code changes on this PR :)

However, it would be good to get this test code reviewed and hopefully landed today/tomorrow so that we have a safety net on the remaining install doc methods.

/cc @egernst.

jodh-intel · 2018-10-23T16:50:37Z

/test

Converted the plain code blocks in `install/installing-with-kata-doc-to-script.md` to bash code blocks so that they are executable by... `kata-doc-to-script.sh`. Also, removed the backslashes to let github render scroll bars for consistency with other docs. Signed-off-by: James O. D. Hunt <james.o.hunt@intel.com>

jodh-intel · 2018-10-24T08:31:16Z

/test

jodh-intel · 2018-10-24T10:50:35Z

/test

jodh-intel · 2018-10-24T11:00:56Z

CI passing! Please review folks :)

chavafg · 2018-10-24T12:28:22Z

lgtm

grahamwhaley

Some minor queries, but don't need to be blocking, so
lgtm

grahamwhaley · 2018-10-24T12:29:55Z

.ci/test-install-docs.sh

+
+	[ ! -e "$GOPATH" ] && die "cannot find $mgr"
+
+	source /etc/os-release


Expand to cover the multiple potential source files as discussed on kata-containers/tests#589, maybe like:

source /etc/os-release || source /usr/lib/os-release

grahamwhaley · 2018-10-24T12:32:44Z

.ci/test-install-docs.sh

+{
+	source /etc/os-release || source /usr/lib/os-release
+
+	mkdir -p "${GOPATH}"


I dunno if this will fail if GOPATH already exists? If so, a || true type fix I guess?

Actually, no, -p won't fail if it exists.

grahamwhaley · 2018-10-24T12:35:07Z

.ci/test-install-docs.sh

+# Grab a copy of the tests repository
+get_tests_repo()
+{
+	[ -d "${test_repo_dir}" ] && return


I doubt we need to, but if it already exists, should we do a pull/fetch on it to make sure it is upto date?

Good question but this test is only designed to run under the CI where everything is fresh.

I tend to agree with Graham, sometimes I use CI scripts locally, not required in this PR we can let the user update test repository manually.

See my comments - #280 (comment).

grahamwhaley · 2018-10-24T12:42:36Z

.ci/test-install-docs.sh

+
+	local tmp_dir
+
+	tmp_dir=$(mktemp -d)


Do we ever finally delete the tmp_dir? Maybe a TRAP hook like we use elsewhere might do it?

Yes, I'll fix that for when running with -t...

If it is fixed can you remove the FIXME?

marcov · 2018-10-24T11:47:26Z

install/installing-with-kata-doc-to-script.md

+$ bash "./${ID}-install.sh"
 ```

 For example, if your distribution is CentOS, the previous example will generate a runnable shell script called `centos-install.sh`.


Would it make sense to separate the bash "./${ID}... command in a dedicate session?

E.g. For example, if your distribution is CentOS, the previous example will generate a runnable shell script called centos-install.sh. To proceed wit the installation, simply run:

$ bash "./${ID}-install.sh"

Sure - I can do that. We'll have to source the os-release file yet again of course... ;)

marcov · 2018-10-24T11:51:08Z

.ci/test-install-docs.sh

 {
-	[ -n "$TRAVIS" ] && info "Not testing install guide as Travis lacks modern distro support and VT-x" && return 
+	if [ -n "$TRAVIS" ]
+	then


about the code style :), isn't if [...]; then more widely used in kata scripts than having then on its own line?

We don't prescribe a coding style atm. However, we could if anyone wants to work on a new doc (and fight all the fights to get it landed :), or maybe it could be an update to:

https://github.com/kata-containers/documentation/blob/master/Documentation-Requirements.md

marcov · 2018-10-24T12:08:06Z

.ci/test-install-docs.sh

+
+	pushd "${kata_project_dir}"
+	git clone "${test_repo_url}"
+	popd


just fyi :) you could also do git clone "${test_repo_url}" "${kata_project_dir}"

Sure. Trying to keep the code as simple as possible.

Oh - sorry, yes - I'll do that...

marcov · 2018-10-24T12:13:21Z

.ci/test-install-docs.sh

+delete_kata_repos()
+{
+	[ -n "${KATA_DEV_MODE}" ] && die "Not continuing - this is a dev system"
+	[ -z "${CI}" ] && die "Not continuing - not running in a CI environment"


not running in a non-CI environment

jodh-intel · 2018-10-24T13:31:23Z

Branch updated.

/test

jodh-intel · 2018-10-24T15:30:09Z

One ack required and we can sleep easy knowing all the install methods are being tested... :)

jodh-intel · 2018-10-24T15:33:08Z

BTW, @egernst, @gnawux, @jcvenegas - what do you think about us re-running all these tests on master after we've created a new release to ensure all the installation methods work at that point?

If so, I'll raise a new PR to update https://github.com/kata-containers/documentation/blob/master/Release-Checklist.md. Ideally, we'd tag the branch or similar to show that it passed all CI tests for the already-released set of packages.

jcvenegas · 2018-10-24T16:17:35Z

@jodh-intel I would like to have a test like that, I was wondering if OBS allow to know if a new build was done, and have a jenkins pipeline to autotest the installation

jodh-intel · 2018-10-24T16:18:47Z

@jcvenegas - pipeline sounds like a good idea!

@chavafg - did you make any progress on reworking our CI code to support pipelines?

jcvenegas

LGTM a few comments but I think nothing that blocks this PR

jcvenegas · 2018-10-24T16:38:49Z

.ci/test-install-docs.sh

+# Grab a copy of the tests repository
+get_tests_repo()
+{
+	[ -d "${test_repo_dir}" ] && return


I tend to agree with Graham, sometimes I use CI scripts locally, not required in this PR we can let the user update test repository manually.

jcvenegas · 2018-10-24T16:39:52Z

.ci/test-install-docs.sh

+
+	local cwd="$PWD"
+
+	info "Deleting all local kata repositories below ${kata_project_dir}"


This should be a warning with a big red button :D

As shown in the code, we only do this if running under a CI and developer mode is not enabled.

jcvenegas · 2018-10-24T16:47:30Z

.ci/test-install-docs.sh

+
+	local tmp_dir
+
+	tmp_dir=$(mktemp -d)


If it is fixed can you remove the FIXME?

jcvenegas · 2018-10-24T16:52:43Z

.ci/test-install-docs.sh

+		bash "${doc_to_script}" "${file_path}" "${script_file_path}"
+	done
+
+	reexec_in_tmpdir "${tmp_dir}"


This is confusing :P thanks for document it

jodh-intel · 2018-10-24T16:59:04Z

@jcvenegas - regarding your comment on get_tests_repo() - I understand but as you've seen, we really do not want users running this script as we delete all local Kata github repos!

The script is only designed to run in a clean CI environment so that's why I don't think it's worth checking if the repo needs updating.

chavafg · 2018-10-24T21:50:43Z

@jcvenegas @jodh-intel yes, we can create a pipeline for this. Would this be for the complete release process? or just part of it?
As for triggering a job when a new build is created, not sure what would be the best way, but searching the web found this plugin: https://wiki.jenkins.io/display/JENKINS/URLTrigger+Plugin
which states that
* This plugin enables users to check if new artifacts (such as binaries) have been deployed in a repository (managed for example by a repository manager such as Sonatype Nexus, JFrog Artifactory, Apache Archiva and so on).

seems like it might help?

jcvenegas · 2018-10-24T22:14:39Z

@chavafg I would be great all the release process :D but for now just testing the packages is good. @marcov do you know if OBS support a way to trigger a hook after a build is done?

jodh-intel · 2018-10-25T07:37:31Z

FIXME removed and branch updated.

/test

jodh-intel · 2018-10-25T09:42:33Z

Ping @egernst, @bergwolf.

jodh-intel · 2018-10-25T14:37:38Z

@egernst - could you tal when you get a chance? It would be great to get these tests running :)

Update the `kata-doc-to-script` install document to actually execute the generated scripts, allowing the entire installation to be tested by the CI. Signed-off-by: James O. D. Hunt <james.o.hunt@intel.com>

Make the variable in the `info()` function a `local` one. Signed-off-by: James O. D. Hunt <james.o.hunt@intel.com>

Folded the overly-long Travis check line in `check_install_guides()`. Signed-off-by: James O. D. Hunt <james.o.hunt@intel.com>

Rename the `check_install_guides()` function to `check_install_docs()` and clean up: - Improve messages. - Add more braces around variables. Signed-off-by: James O. D. Hunt <james.o.hunt@intel.com>

Rework the logic in `check_install_docs()` to make the intention clearer and support adding additional tests. Signed-off-by: James O. D. Hunt <james.o.hunt@intel.com>

Create a `setup()` function in the test script used to test the install documents. Signed-off-by: James O. D. Hunt <james.o.hunt@intel.com>

The script used to test the install docs does not actually use the golang binary (it only uses the `GOPATH` variables) so remove the unnecessary call to `go`. Signed-off-by: James O. D. Hunt <james.o.hunt@intel.com>

Split out a function to create a container from `test_distro_install_guide() in the script used to test install docs. Signed-off-by: James O. D. Hunt <james.o.hunt@intel.com>

Make `setup()` clone the tests repo and check for the `kata-manager` script. Signed-off-by: James O. D. Hunt <james.o.hunt@intel.com>

Wrap the function calls in the doc test script in a `main()` function to simplify future changes. Signed-off-by: James O. D. Hunt <james.o.hunt@intel.com>

Add new CI tests to ensure that the following installation methods are also tested: - "Automatic" method ([`kata-manager`](https://github.com/kata-containers/tests/tree/master/cmd/kata-manager)) - "Scripted" method ([`kata-doc-to-script`](https://github.com/kata-containers/tests/blob/master/.ci/kata-doc-to-script.sh)) **Note:** the "Automatic" method is **not** the same as the existing `kata-manager` test: the existing test executes the "Manual" installation method (which runs `kata-manager` to execute the appropriate distro-specific install guide). However, this new test executes the `install/installing-with-kata-manager.md` document, which subsequently calls the `kata-manager` script. Since the "Automatic" and "Scripted" installation methods are designed to run "standalone" (without requiring any local git repo clones), the script which runs these new tests has to take care to ensure the environment they run in is clean. It does this by using the following approach: - Removes any local Kata github repos from the standard `GOPATH` locations (to ensure the scripts do not inadvertently access local files) [1]. - Creates a temporary directory containing: - A copy of *itself*. - The scripts it generated from the "Automatic" and "Scripted" installation documents. - Re-exec's itself to run the version in the temporary directory, passing an option that tells itself to simply execute the scripts in the specified directory. - It then runs the scripts in the directory specified. --- [1] - Since the recursive delete of all local Kata github repos is potentially dangerous, the test will immediately fail if the standard `KATA_DEV_MODE` variable is set (since this denotes a developer system) and will also fail unless the standard `CI` variable is set (denoting the script is running in a Continuous Integration environment, such as JenkinsCI. Fixes kata-containers#278. Signed-off-by: James O. D. Hunt <james.o.hunt@intel.com>

jodh-intel · 2018-10-25T15:30:03Z

All comments responded to I think.

jodh-intel · 2018-10-25T15:31:05Z

/test

jodh-intel · 2018-10-25T16:03:37Z

lgtm

/test

ping @egernst.

jodh-intel · 2018-10-25T16:04:02Z

Ah - we have full approval - a miracle! ;)

jodh-intel · 2018-10-25T16:22:10Z

/test

jcvenegas · 2018-10-25T21:46:10Z

all passing merging ...

Chrony service is not started because it requires a private temporal directory, these directories can't be created in read-only filesystems. Create a symlink to /tmp in /var allowing systemd to create private temporal directories. fixes kata-containers#280 Signed-off-by: Julio Montes <julio.montes@intel.com>

jodh-intel force-pushed the ci-add-tests-for-other-install-doc-methods branch from 83dad45 to 788329b Compare October 23, 2018 09:48

grahamwhaley mentioned this pull request Oct 23, 2018

CI: Make static check ignore local URLs kata-containers/tests#838

Merged

jodh-intel force-pushed the ci-add-tests-for-other-install-doc-methods branch from 788329b to 0e2b18f Compare October 23, 2018 14:21

jodh-intel mentioned this pull request Oct 23, 2018

kata-manager: do not depend on git,go or local repos kata-containers/tests#829

Merged

jodh-intel force-pushed the ci-add-tests-for-other-install-doc-methods branch from 63be14d to e144a6e Compare October 23, 2018 15:07

jodh-intel force-pushed the ci-add-tests-for-other-install-doc-methods branch from e144a6e to 4a4f72e Compare October 23, 2018 16:32

jodh-intel force-pushed the ci-add-tests-for-other-install-doc-methods branch from 4a4f72e to 13f1821 Compare October 24, 2018 08:31

jodh-intel force-pushed the ci-add-tests-for-other-install-doc-methods branch from 13f1821 to acac089 Compare October 24, 2018 10:50

grahamwhaley approved these changes Oct 24, 2018

View reviewed changes

marcov reviewed Oct 24, 2018

View reviewed changes

jodh-intel force-pushed the ci-add-tests-for-other-install-doc-methods branch from acac089 to 940c32c Compare October 24, 2018 13:30

jcvenegas approved these changes Oct 24, 2018

View reviewed changes

devimc approved these changes Oct 24, 2018

View reviewed changes

James O. D. Hunt added 11 commits October 25, 2018 16:28

docs: Add instructions to run the kata-doc-to-script generated scripts

7dba230

Update the `kata-doc-to-script` install document to actually execute the generated scripts, allowing the entire installation to be tested by the CI. Signed-off-by: James O. D. Hunt <james.o.hunt@intel.com>

CI: Make info() variable local

f4351e4

Make the variable in the `info()` function a `local` one. Signed-off-by: James O. D. Hunt <james.o.hunt@intel.com>

CI: Fold long Travis check line

3d08ccd

Folded the overly-long Travis check line in `check_install_guides()`. Signed-off-by: James O. D. Hunt <james.o.hunt@intel.com>

CI: Rename+cleanup install doc test function

d21eaaf

Rename the `check_install_guides()` function to `check_install_docs()` and clean up: - Improve messages. - Add more braces around variables. Signed-off-by: James O. D. Hunt <james.o.hunt@intel.com>

CI: Refactor check_install_guides()

e74c431

Rework the logic in `check_install_docs()` to make the intention clearer and support adding additional tests. Signed-off-by: James O. D. Hunt <james.o.hunt@intel.com>

CI: Add setup function to doc test script

8334d90

Create a `setup()` function in the test script used to test the install documents. Signed-off-by: James O. D. Hunt <james.o.hunt@intel.com>

CI: Remove golang dependency on doc test script

d9c91d3

The script used to test the install docs does not actually use the golang binary (it only uses the `GOPATH` variables) so remove the unnecessary call to `go`. Signed-off-by: James O. D. Hunt <james.o.hunt@intel.com>

CI: Refactor function in install doc test

4b1839f

Split out a function to create a container from `test_distro_install_guide() in the script used to test install docs. Signed-off-by: James O. D. Hunt <james.o.hunt@intel.com>

CI: Improve doc test script setup function

83c38c2

Make `setup()` clone the tests repo and check for the `kata-manager` script. Signed-off-by: James O. D. Hunt <james.o.hunt@intel.com>

CI: Add main function to doc test script

78692a1

Wrap the function calls in the doc test script in a `main()` function to simplify future changes. Signed-off-by: James O. D. Hunt <james.o.hunt@intel.com>

jodh-intel force-pushed the ci-add-tests-for-other-install-doc-methods branch from 940c32c to 2f07105 Compare October 25, 2018 15:28

egernst approved these changes Oct 25, 2018

View reviewed changes

jcvenegas merged commit 1d6c296 into kata-containers:master Oct 25, 2018


		[ ! -e "$GOPATH" ] && die "cannot find $mgr"

		source /etc/os-release


		local cwd="$PWD"

		info "Deleting all local kata repositories below ${kata_project_dir}"

Conversation

jodh-intel commented Oct 23, 2018

Uh oh!

jodh-intel commented Oct 23, 2018

Uh oh!

jodh-intel commented Oct 23, 2018

Uh oh!

jodh-intel commented Oct 23, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jodh-intel commented Oct 23, 2018

Uh oh!

jodh-intel commented Oct 23, 2018

Uh oh!

jodh-intel commented Oct 23, 2018

Uh oh!

jodh-intel commented Oct 23, 2018

Uh oh!

jodh-intel commented Oct 23, 2018

Uh oh!

jodh-intel commented Oct 23, 2018

Uh oh!

jodh-intel commented Oct 24, 2018

Uh oh!

jodh-intel commented Oct 24, 2018

Uh oh!

jodh-intel commented Oct 24, 2018

Uh oh!

chavafg commented Oct 24, 2018

Uh oh!

grahamwhaley left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jodh-intel commented Oct 24, 2018

Uh oh!

jodh-intel commented Oct 24, 2018

Uh oh!

jodh-intel commented Oct 24, 2018

jodh-intel commented Oct 23, 2018 •

edited

Loading