Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CI - various fixes #7070

Merged
merged 1 commit into from
Jul 27, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .cirrus.yml
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ env:
UBUNTU_NAME: "ubuntu-20"
PRIOR_UBUNTU_NAME: "ubuntu-19"

_BUILT_IMAGE_SUFFIX: "podman-5869602141896704"
_BUILT_IMAGE_SUFFIX: "podman-6439450735542272"
Copy link
Member

Choose a reason for hiding this comment

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

👏 you made it through the "magic VM building workflow", yay 👏

(I found this out because the number in the name is a $CIRRUS_BUILD_ID...which maps back to this PR)

FEDORA_CACHE_IMAGE_NAME: "${FEDORA_NAME}-${_BUILT_IMAGE_SUFFIX}"
PRIOR_FEDORA_CACHE_IMAGE_NAME: "${PRIOR_FEDORA_NAME}-${_BUILT_IMAGE_SUFFIX}"
UBUNTU_CACHE_IMAGE_NAME: "${UBUNTU_NAME}-${_BUILT_IMAGE_SUFFIX}"
Expand Down
3 changes: 0 additions & 3 deletions contrib/cirrus/check_image.sh
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,6 @@ item_test 'Minimum available memory' $MEM_FREE -ge $MIN_MEM_MB || let "NFAILS+=1
remove_packaged_podman_files
item_test "remove_packaged_podman_files() does it's job" -z "$(type -P podman)" || let "NFAILS+=1"

# Integration Tests require varlink in Fedora
item_test "The varlink executable is present" -x "$(type -P varlink)" || let "NFAILS+=1"

MIN_ZIP_VER='3.0'
VER_RE='.+([[:digit:]]+\.[[:digit:]]+).+'
ACTUAL_VER=$(zip --version 2>&1 | egrep -m 1 "Zip$VER_RE" | sed -r -e "s/$VER_RE/\\1/")
Expand Down
2 changes: 2 additions & 0 deletions contrib/cirrus/container_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ if [ "${ID}" != "fedora" ] || [ "${CONTAINER_RUNTIME}" != "" ]; then
INTEGRATION_TEST_ENVS="SKIP_USERNS=1"
fi

echo "$(date --rfc-3339=seconds) $(basename $0) started with '$*' and TEST_REMOTE_CLIENT='${TEST_REMOTE_CLIENT}'"

pwd

# -i install
Expand Down
2 changes: 1 addition & 1 deletion contrib/cirrus/integration_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ fi
# but pr2947 intends to add 'system'.
TESTSUITE=$(expr $(basename $0) : '\(.*\)_test')
if [[ -z $TESTSUITE ]]; then
die 1 "Script name is not of the form xxxx_test.sh"
die 1 "Script name ($basename $0) is not of the form xxxx_test.sh"
fi

cd "$GOSRC"
Expand Down
2 changes: 1 addition & 1 deletion contrib/cirrus/networking.sh
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ while read host port
do
if [[ "$port" -eq "443" ]]
then
item_test "SSL/TLS to $host:$port" "$(echo -n '' | openssl s_client -quiet -no_ign_eof -connect $host:$port &> /dev/null; echo $?)" -eq "0"
item_test "SSL/TLS to $host:$port" "$(echo -n '' | timeout 60 openssl s_client -quiet -no_ign_eof -connect $host:$port &> /dev/null; echo $?)" -eq "0"
Copy link
Member

Choose a reason for hiding this comment

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

Goods idea

else
item_test "Connect to $host:$port" "$(nc -zv -w 13 $host $port &> /dev/null; echo $?)" -eq 0
fi
Expand Down
89 changes: 89 additions & 0 deletions contrib/cirrus/packer/README.how-to-update-cirrus-vms
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
This document briefly describes how to update VMs on Cirrus.
cevich marked this conversation as resolved.
Show resolved Hide resolved

Examples of when you need to do this:

- to update crun, conmon, or some other package(s)
- to add and/or remove an OS (eg drop f31, add f33)
Copy link
Member

Choose a reason for hiding this comment

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

The above isn't entirely true, that requires a third "phase" which isn't worth documenting here (it's complex): Namely (FYI) google does not have ready-made Fedora images. We use packer (again) as part of a semi-automated process, involving libpod_base_images.yml + Makefile. The complexity mainly comes from the google-side (image-format stuffs).

- to change system config (eg containers.conf or other /etc files)
edsantiago marked this conversation as resolved.
Show resolved Hide resolved
- to change kernel command-line (boot time) options

This is a TWO-STEP process: you need to submit a PR with a magic [CI:IMG]
description string, wait for it to finish, grab a magic string from the
results, then resubmit without [CI:IMG].

Procedure, Part One of Two:

1) Create a working branch:

$ git co -b my_branch_name

2) Make your changes. Typically, zero or more of the following files:

.cirrus.yml
contrib/cirrus/packer/*_packaging.sh

I said zero because sometimes you just want to update VMs
with the latest in dnf or ubuntu repos. That doesn't require
changing anything here, simply running new dnf/apt installs.

3) Commit your changes. Be sure to include the magic [CI:IMG] string:

$ git commit -asm'[CI:IMG] this is my commit message'

4) Submit your PR:

$ gh pr create --fill --web


-------------------------- INTERMISSION --------------------------
...in which we wait for CI to turn green. In particular, although
we only really need 'test_build_cache_images' (45 minutes or so)
to get the required magic number strings, please be a decent
human being and wait for 'verify_test_built_images' (another hour)
so we can all have confidence in our process. Thank you.
-------------------------- INTERMISSION --------------------------
cevich marked this conversation as resolved.
Show resolved Hide resolved


Procedure, Part Two of Two:

1) When 'test_build_cache_images' completes, click it, then click
'View more details on Cirrus CI', then expand the 'Run build_vm_image'
accordion. This gives you a garishly colorful display of lines.
Each color is a different VM.
Copy link
Member

Choose a reason for hiding this comment

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

and there's no way to solarize them 😆


2) Verify that each VM has the packages you require. (The garish log
doesn't actually list this for all packages, so you may need to
look in the 'verify_test_built_images' log for each individual
VM. Click the 'package_versions' accordion.)
Copy link
Member

Choose a reason for hiding this comment

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

accordingly?


3) At the bottom of this log you will see a block like:

Builds finished. The artifacts of successful builds are:
ubuntu-19: A disk image was created: ubuntu-19-podman-6439450735542272
fedora-31: A disk image was created: fedora-31-podman-6439450735542272
.....

The long numbers at the end should (MUST!) be all identical.
Copy link
Member

Choose a reason for hiding this comment

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

must...something is really, really, horribly broken if they're not.


4) Edit .cirrus.yml locally. Find '_BUILT_IMAGE_SUFFIX' near the
top. Copy that long number ("6439450735542272", above) and paste
it here, replacing the previous long number.

5) Wait for CI to turn green. I know you might have skipped that,
because 'test_build_cache_images' finishes long before 'verify',
and maybe you're in a hurry, but come on. Be responsible.

Copy link
Member

Choose a reason for hiding this comment

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

hehe

6) Edit the PR description in github: remove '[CI:IMG]' from the
title. Again, *in github*, in the web UI, use the 'Edit' button
at top right next to the PR title. Remove the '[CI:IMG]' string
from the PR title, press Save. If you forget to do this, the
VM-building steps will run again (taking a long time) but it
will be a waste of time.

7) Update your PR:

$ git add .cirrus.yml (to get the new magic IMAGE_SUFFIX string)
$ git commit --amend (remove [CI:IMG] for consistency with 6)
$ git push --force

You can probably take it from here.
9 changes: 9 additions & 0 deletions contrib/cirrus/packer/fedora_packaging.sh
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,15 @@ DOWNLOAD_PACKAGES=(\
echo "Installing general build/test dependencies for Fedora '$OS_RELEASE_VER'"
$BIGTO ooe.sh $SUDO dnf install -y ${INSTALL_PACKAGES[@]}

# AD-HOC CODE FOR SPECIAL-CASE SITUATIONS!
# On 2020-07-23 we needed this code to upgrade crun on f31, a build
# that is not yet in stable. Since CI:IMG PRs are a two-step process,
# the key part is that we UN-COMMENT-THIS-OUT during the first step,
# then re-comment it on the second (once we have the built images).
# That way this will be dead code in future CI:IMG PRs but will
# serve as an example for anyone in a similar future situation.
# $BIGTO ooe.sh $SUDO dnf --enablerepo=updates-testing -y upgrade crun

Copy link
Member

Choose a reason for hiding this comment

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

Nice comment, I hope that will be helpful to someone.

[[ ${#REMOVE_PACKAGES[@]} -eq 0 ]] || \
$LILTO ooe.sh $SUDO dnf erase -y ${REMOVE_PACKAGES[@]}

Expand Down
4 changes: 3 additions & 1 deletion contrib/cirrus/rootless_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,13 @@

set -e

echo "$(date --rfc-3339=seconds) $(basename $0) started with '$*'"

cevich marked this conversation as resolved.
Show resolved Hide resolved
source $(dirname $0)/lib.sh

if [[ "$UID" == "0" ]]
then
echo "Error: Expected to be running as a regular user"
echo "$(basename $0): Error: Expected to be running as a regular user"
exit 1
fi

Expand Down
3 changes: 0 additions & 3 deletions contrib/cirrus/setup_environment.sh
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,6 @@ case "${OS_RELEASE_ID}" in

workaround_bfq_bug

# HACK: Need Conmon 2.0.17, currently in updates-testing on F31.
dnf update -y --enablerepo=updates-testing conmon

Copy link
Member

Choose a reason for hiding this comment

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

Oh good, yes, thank you for confirming this can be removed.

if [[ "$ADD_SECOND_PARTITION" == "true" ]]; then
bash "$SCRIPT_BASE/add_second_partition.sh"
fi
Expand Down
4 changes: 2 additions & 2 deletions contrib/cirrus/timestamp.awk
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
BEGIN {
STARTTIME=systime()
printf "[%s] START", strftime("%T")
printf " - All [+xxxx] lines that follow are relative to right now.\n"
printf " - All [+xxxx] lines that follow are relative to %s.\n", strftime("%FT%T")
Copy link
Member

Choose a reason for hiding this comment

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

ahh yes...much more helpful-ing this way. Thanks.

}

{
Expand All @@ -16,5 +16,5 @@ BEGIN {

END {
printf "[%s] END", strftime("%T")
printf " - [%+05ds] total duration since START\n", systime()-STARTTIME
printf " - [%+05ds] total duration since %s\n", systime()-STARTTIME, strftime("%FT%T")
}