ci/openshift-ci: Enable openshift-ci #2891

wainersm · 2020-08-19T23:34:15Z

Enable openshift-ci on runtime repository.

Fixes: github.com/kata-containers/tests#2527
Depends-on: github.com/kata-containers/tests#2802

Signed-off-by: Wainer dos Santos Moschetta wainersm@redhat.com

devimc · 2020-08-20T14:18:25Z

thanks @wainersm - but I think openshit-ci is not the best name for a CI

jodh-intel · 2020-08-24T11:04:35Z

.ci/openshift-ci/images/entrypoint.sh

+# By using it in an openshift daemonset it should spin forever until an
+# orchestrator kills it.
+#
+while true; do sleep 600; done


We tend to prefer tail -f /dev/null to a loop like this.

jodh-intel · 2020-08-24T11:05:03Z

.ci/openshift-ci/images/entrypoint.sh

+#
+# Synchronize the kata-containers installation with in the host filesystem.
+#
+set -x


set -e (too)?

This script is going to execute in a k8s DaemonSet. If set -e and the script fails then the pod will end up on a "unpredictable state". The mechanism I employ to verify the installation succeeded (or not) is to write the return code of kata-runtime kata-check to /tmp/kata_install_status then put the pod to "sleep" (tail -f /dev/null). One control script will check the value of /tmp/kata_install_status on each pod, if all of them was done then the daemonSet is retired.

Let me know if I am missing something here, or if you think there is a better way to implement it.

jodh-intel · 2020-08-24T11:05:21Z

.ci/openshift-ci/README.md

@@ -0,0 +1,3 @@
+# OpenShift CI files
+
+This directory contain scripts and files used by the openshift-ci operator to test kata. For further details read this [README](https://github.com/kata-containers/tests/tree/master/.ci/openshift-ci/README.md) file.


Nit: very long line.

I deleted the README.md so not need to this. :)

jodh-intel · 2020-08-24T11:06:41Z

.ci/openshift-ci/images/entrypoint.sh

+fi
+
+# Copy the installation over the host filesystem.
+rsync -a * $HOST_MOUNT


Should that be . rather than * in case of hidden files?

Probably better, although installing hidden files on the host is a bad idea.

yeah, I am not expecting hidden files but I changed as suggested in the v4 of this PR.

wainersm · 2020-08-24T12:15:44Z

thanks @wainersm - but I think openshit-ci is not the best name for a CI

@devimc you got eagles eyes! 👍

devimc · 2020-08-24T12:25:30Z

@wainersm thanks, but I still see the same word in your commit message

wainersm · 2020-08-24T12:55:03Z

Changes v1 -> v2:

Fixed the typo in the commit message

devimc · 2020-08-24T13:03:45Z

please add a reference to openshift-ci/README.md in the main README.md

source=check-markdown version=0.0.1
	"virtcontainers/experimental/README.md"
	"CONTRIBUTING.md"
	"virtcontainers/documentation/api/1.0/api.md"
	"virtcontainers/documentation/Developers.md"
	"virtcontainers/pkg/cloud-hypervisor/README.md"
	"virtcontainers/persist/plugin/README.md"
ERROR: Document .ci/openshift-ci/README.md is not referenced
The command ".ci/static-checks.sh" failed and exited with 1 during .

devimc

thanks @wainersm

devimc · 2020-08-24T13:07:43Z

.ci/openshift-ci/images/entrypoint.sh

+mount -o remount,rw $HOST_MOUNT/usr
+
+# The host's '/opt' and '/usr/local' are symlinks to, respectively, '/var/opt'
+# and '/var/usrlocal'. Adjust the installation files accordingly.


typo: usrlocal -> usr/local

Actually not.

[core@worker-0-0 /]$ ls -lha var/ ... drwxr-xr-x. 12 root root 126 Aug 10 20:23 usrlocal ...

That's the structure used by Red Hat CoreOS, which is used by OpenShift.

devimc · 2020-08-24T13:08:41Z

.ci/openshift-ci/images/entrypoint.sh

+
+if [ -d "usr/local" ]; then
+	mkdir -p var
+	mv usr/local var/usrlocal


usrlocal -> usr/local

... and the same here.

devimc · 2020-08-24T16:30:31Z

.ci/openshift-ci/images/entrypoint.sh

+# get copied.
+mount -o remount,rw $HOST_MOUNT/usr
+
+# The host's '/opt' and '/usr/local' are symlinks to, respectively, '/var/opt'


what about here '/usr/local' ?

So, the whole idea about Red Hat CoreOS is to provide the user an immutable system, quite similar to Fedora Silverblue (https://silverblue.fedoraproject.org/) or Fedora CoreOS (https://github.com/coreos/fedora-coreos-tracker).

Those systems do have /usr being mounted as read-only. Apart from a few "workarounds" where wouldn't be to problematic for users to override, such as /opt or /usr/local ... those workarounds consist in having those directories being symlinks to directories under /var, which is not read-only.

wainersm · 2020-08-24T18:21:47Z

please add a reference to openshift-ci/README.md in the main README.md

@devimc , I think I will remove the openshift-ci/README.md file completely if you agree. And the rationale is:

The parent .ci folder doesn't have a readme, and so just adding an entry on the main readme seems weird
This readme is only a mere pointer to github.com/kata-containers/tests/.ci/openshift-ci 's readme

wainersm · 2020-09-01T17:53:46Z

v2 -> v3:

.ci/openshift-ci/images/entrypoint.sh:
- linking all binaries to /usr/local/bin
- sleep infinity with tail -f /dev/null
Deleted .ci/openshift-ci/README.md

devimc · 2020-09-01T18:26:07Z

.ci/openshift-ci/build.sh

+pushd "${tests_repo_dir}"
+# Resolve the kata-container repositories. It relies on branch and
+# kata_repo variables.
+export branch=master


I still don't understand the tests scripts very well, with this you will be testing master and not the pr, is that correct ?

yes @devimc . For kata-1.x I will run these scripts in a nightly job, so not dealing with PRs. I will need to adapt this to kata-2.x because in this case I plan to leverage the openshift CI to test PRs as well.

jodh-intel · 2020-09-08T13:26:46Z

.ci/openshift-ci/images/Dockerfile

+#
+FROM centos:8
+
+RUN yum install -y rsync


Nit: dnf 😄

Not on centos image.

[root@f5018106fe91 /]# yum Loaded plugins: fastestmirror, ovl You need to give some command Usage: yum [options] COMMAND [root@f5018106fe91 /]# dnf bash: dnf: command not found

Looks good to me. Maybe wait for @jodh-intel to ack the replies we gave. I think that @devimc is not available this week.

Thanks @c3d ! I am about to send another version with two small changes as suggested by @jodh-intel . Then we will be ready to go :)

Ah - you get it by default, but clearly not with your image ;)

Not on centos image.

[root@f5018106fe91 /]# yum Loaded plugins: fastestmirror, ovl You need to give some command Usage: yum [options] COMMAND [root@f5018106fe91 /]# dnf bash: dnf: command not found

hmm... interesting.

On v4 of this PR I changed to centos:7 and the reason is: this image will be built in a OpenShift Build process which usually replaces the FROM to whatever the user choose. In the case of this CI, it will use RHEL-7 so the closest we got (for the sake of debugging/development in your workstation) is centos-7.

Ah - right. I'd missed the downgrade to centos 7 ;)

devimc

thanks @wainersm

c3d

Looks good to me. Maybe wait for @jodh-intel to ack the replies we gave. I think that @devimc is not available this week.

jodh-intel

Thanks @wainersm.

lgtm

jodh-intel · 2020-09-14T14:15:17Z

/test

Enable openshift-ci on runtime repository. Fixes: github.com/kata-containers/tests#2527 Depends-on: github.com/kata-containers/tests#2802 Signed-off-by: Wainer dos Santos Moschetta <wainersm@redhat.com>

wainersm · 2020-09-14T14:20:06Z

v3 -> v4:

.ci/openshift-ci/images/entrypoint.sh:
- Replaced rsync -a * with rsync -a .
.ci/openshift-ci/images/Dockerfile:
- Using CentOS-7 instead of 8

wainersm · 2020-09-14T14:31:55Z

/test

codecov · 2020-09-14T15:14:57Z

Codecov Report

Merging #2891 into master will increase coverage by 0.22%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #2891      +/-   ##
==========================================
+ Coverage   51.44%   51.67%   +0.22%     
==========================================
  Files         118      118              
  Lines       17428    17438      +10     
==========================================
+ Hits         8966     9011      +45     
+ Misses       7379     7342      -37     
- Partials     1083     1085       +2

wainersm · 2020-09-14T18:24:59Z

The failure on the jobs aren't related with my changes.

wainersm · 2020-09-15T14:32:24Z

/test jenkins-metrics-ubuntu-18-04

jodh-intel · 2020-09-16T08:29:56Z

Hi @GabyCT - it looks like Jenkins has "stalled" - could you tal please?

wainersm · 2020-09-18T14:19:15Z

/test-ubuntu

wainersm · 2020-09-18T18:54:05Z

/test

wainersm · 2020-09-21T12:10:09Z

/retest-ubuntu

wainersm · 2020-09-21T14:35:12Z

/retest-qemu-metrics

wainersm · 2020-09-22T12:44:58Z

Hi @jodh-intel , this metrics job failing seems not related with this PR. What should I do to get this merged?

amshinde · 2020-09-24T21:59:30Z

/test-ubuntu-qemu-metrics

amshinde · 2020-09-24T22:00:10Z

@wainersm Rerunning the metrics CI, since we have fixed that now.

wainersm requested a review from a team as a code owner August 19, 2020 23:34

wainersm force-pushed the enable_openshift-ci branch from db7298d to 7ecf821 Compare August 19, 2020 23:36

fidencio changed the title ~~ci/openshit-ci: Enable openshift-ci~~ ci/openshift-ci: Enable openshift-ci Aug 24, 2020

jodh-intel reviewed Aug 24, 2020

View reviewed changes

wainersm force-pushed the enable_openshift-ci branch from 7ecf821 to 594da37 Compare August 24, 2020 12:53

devimc reviewed Aug 24, 2020

View reviewed changes

wainersm force-pushed the enable_openshift-ci branch from 594da37 to 9bc3343 Compare September 1, 2020 17:52

devimc reviewed Sep 1, 2020

View reviewed changes

jodh-intel reviewed Sep 8, 2020

View reviewed changes

wainersm mentioned this pull request Sep 10, 2020

On-boarding Kata Containers into OpenShift CI kata-containers/tests#2802

Merged

devimc approved these changes Sep 11, 2020

View reviewed changes

c3d approved these changes Sep 14, 2020

View reviewed changes

jodh-intel approved these changes Sep 14, 2020

View reviewed changes

ci/openshift-ci: Enable openshift-ci

598b4fe

Enable openshift-ci on runtime repository. Fixes: github.com/kata-containers/tests#2527 Depends-on: github.com/kata-containers/tests#2802 Signed-off-by: Wainer dos Santos Moschetta <wainersm@redhat.com>

wainersm force-pushed the enable_openshift-ci branch from 9bc3343 to 598b4fe Compare September 14, 2020 14:17

jodh-intel mentioned this pull request Sep 16, 2020

Jenkins seems to have "stalled" kata-containers/ci#328

Closed

wainersm mentioned this pull request Sep 21, 2020

integration/openshift: Add OpenShift e2e tests runner kata-containers/tests#2861

Merged

c3d mentioned this pull request Sep 23, 2020

metrics CI job is failing, blocking issue merging #2983

Closed

c3d added the blocked-by-metrics Temporary flag to indicate PRs that are blocked by #2983. This will allow us to focus on others. label Sep 24, 2020

c3d merged commit cf5d190 into kata-containers:master Sep 25, 2020

		@@ -0,0 +1,3 @@
		# OpenShift CI files

		This directory contain scripts and files used by the openshift-ci operator to test kata. For further details read this [README](https://github.com/kata-containers/tests/tree/master/.ci/openshift-ci/README.md) file.

ci/openshift-ci: Enable openshift-ci #2891

ci/openshift-ci: Enable openshift-ci #2891

Conversation

wainersm commented Aug 19, 2020 • edited

devimc commented Aug 20, 2020

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

wainersm commented Aug 24, 2020

devimc commented Aug 24, 2020

wainersm commented Aug 24, 2020

devimc commented Aug 24, 2020

devimc left a 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

wainersm commented Aug 24, 2020

wainersm commented Sep 1, 2020

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

wainersm Sep 14, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

devimc left a comment

Choose a reason for hiding this comment

c3d left a comment

Choose a reason for hiding this comment

jodh-intel left a comment

Choose a reason for hiding this comment

jodh-intel commented Sep 14, 2020

wainersm commented Sep 14, 2020

wainersm commented Sep 14, 2020

codecov bot commented Sep 14, 2020 • edited

Codecov Report

wainersm commented Sep 14, 2020

wainersm commented Sep 15, 2020

jodh-intel commented Sep 16, 2020

wainersm commented Sep 18, 2020

wainersm commented Sep 18, 2020

wainersm commented Sep 21, 2020

wainersm commented Sep 21, 2020

wainersm commented Sep 22, 2020

amshinde commented Sep 24, 2020

amshinde commented Sep 24, 2020

wainersm commented Aug 19, 2020 •

edited

wainersm Sep 14, 2020 •

edited

codecov bot commented Sep 14, 2020 •

edited