Cirrus: Fix release dependencies

The release-task ***must*** always execute last, in order to guarantee a
consistent cache of release archives from dependent tasks.  It
accomplishes this by verifying it's task-number matches one-less than
the total number of tasks.  Previous to this commit, a YAML anchor/alias
was used to avoid duplication of the dependency list between 'success'
and 'release'

However, it's been observed that this opens the possibility for
'release' and 'success' tasks to race when running on a PR.  Because
YAML anchor/aliases cannot be used to modify lists, duplication is
required to make 'release' actually depend upon 'success'.

This duplication will introduce an additional maintenance burden.
Though when adding a new task, it's already very easy to forget to
update the 'depends_on' list.  Assist both cases by the addition
unit-tests to verify ``.cirrus.yml`` dependency contents and structure.

Signed-off-by: Chris Evich <cevich@redhat.com>
This commit is contained in:
Chris Evich
2019-07-31 09:48:14 -04:00
parent cb2ea1a27b
commit 3e3afb942a
3 changed files with 87 additions and 2 deletions

View File

@ -504,13 +504,14 @@ success_task:
only_if: $CIRRUS_BRANCH != $DEST_BRANCH only_if: $CIRRUS_BRANCH != $DEST_BRANCH
# ignores any dependent task conditions, include everything except 'release' # ignores any dependent task conditions, include everything except 'release'
depends_on: &alltasks depends_on:
- "gating" - "gating"
- "vendor" - "vendor"
- "varlink_api" - "varlink_api"
- "build_each_commit" - "build_each_commit"
- "build_without_cgo" - "build_without_cgo"
- "meta" - "meta"
- "image_prune"
- "testing" - "testing"
- "special_testing_rootless" - "special_testing_rootless"
- "special_testing_in_podman" - "special_testing_in_podman"
@ -540,7 +541,22 @@ release_task:
# allow_failures: $CI == "true" # allow_failures: $CI == "true"
# skip_notifications: $CI == "true" # skip_notifications: $CI == "true"
depends_on: *alltasks # Must include everything (YAML anchor/alias cannot be used here)
depends_on:
- "gating"
- "vendor"
- "varlink_api"
- "build_each_commit"
- "build_without_cgo"
- "meta"
- "image_prune"
- "testing"
- "special_testing_rootless"
- "special_testing_in_podman"
- "special_testing_cross"
- "test_build_cache_images"
- "verify_test_built_images"
- "success"
gce_instance: gce_instance:
image_name: "${IMAGE_BUILDER_CACHE_IMAGE_NAME}" image_name: "${IMAGE_BUILDER_CACHE_IMAGE_NAME}"

View File

@ -232,6 +232,7 @@ localunit: test/goecho/goecho varlink_generate
--succinct --succinct
$(MAKE) -C contrib/cirrus/packer test $(MAKE) -C contrib/cirrus/packer test
./contrib/cirrus/lib.sh.t ./contrib/cirrus/lib.sh.t
./contrib/cirrus/cirrus_yaml_test.py
ginkgo: ginkgo:
ginkgo -v -tags "$(BUILDTAGS)" $(GINKGOTIMEOUT) -cover -flakeAttempts 3 -progress -trace -noColor -nodes 3 -debug test/e2e/. ginkgo -v -tags "$(BUILDTAGS)" $(GINKGOTIMEOUT) -cover -flakeAttempts 3 -progress -trace -noColor -nodes 3 -debug test/e2e/.

View File

@ -0,0 +1,68 @@
#!/usr/bin/env python3
"""
Verify contents of .cirrus.yml meet specific expectations
"""
import sys
import os
import unittest
import yaml
# Assumes directory structure of this file relative to repo.
SCRIPT_DIRPATH = os.path.dirname(os.path.realpath(__file__))
REPO_ROOT = os.path.realpath(os.path.join(SCRIPT_DIRPATH, '../', '../'))
class TestCaseBase(unittest.TestCase):
CIRRUS_YAML = None
def setUp(self):
with open(os.path.join(REPO_ROOT, '.cirrus.yml')) as cirrus_yaml:
self.CIRRUS_YAML = yaml.safe_load(cirrus_yaml.read())
class TestDependsOn(TestCaseBase):
ALL_TASK_NAMES = None
SUCCESS_RELEASE = set(['success', 'release'])
def setUp(self):
super().setUp()
self.ALL_TASK_NAMES = set([key.replace('_task', '')
for key, _ in self.CIRRUS_YAML.items()
if key.endswith('_task')])
def test_dicts(self):
"""Expected dictionaries are present and non-empty"""
for name in ('success_task', 'release_task'):
# tests all names then show specific failures
with self.subTest(name=name):
self.assertIn(name, self.CIRRUS_YAML)
self.assertIn(name.replace('_task', ''), self.ALL_TASK_NAMES)
self.assertIn('depends_on', self.CIRRUS_YAML[name])
self.assertGreater(len(self.CIRRUS_YAML[name]['depends_on']), 0)
def _check_dep(self, name, task_name, deps):
# name includes '_task' suffix, task_name does not
msg=('Please add "{0}" to the "depends_on" list in "{1}"'
"".format(task_name, name))
self.assertIn(task_name, deps, msg=msg)
def test_depends(self):
"""Success and Release tasks depend on all other tasks"""
for name in ('success_task', 'release_task'):
deps = set(self.CIRRUS_YAML[name]['depends_on'])
for task_name in self.ALL_TASK_NAMES - self.SUCCESS_RELEASE:
with self.subTest(name=name, task_name=task_name):
self._check_dep(name, task_name, deps)
def test_release(self):
"""Release task must always execute last"""
deps = set(self.CIRRUS_YAML['release_task']['depends_on'])
self._check_dep('release_task', 'success', deps)
if __name__ == "__main__":
unittest.main()