PR#606: osbs: Make git_branch required option - pungi

pungi

#606 osbs: Make git_branch required option

Merged 7 years ago by lsedlar. Opened 7 years ago by lsedlar.

lsedlar/pungi osbs-git-branch into master

osbs: Make git_branch required option

Lubomír Sedlář • 7 years ago

bab2a12

doc/configuration.rst

file modified

		`@@ -1291,6 +1291,11 @@`
		* ``url`` -- (str) URL pointing to a Git repository with ``Dockerfile``.
		Please see :ref:`git-urls` section for more details.
		* ``target`` -- (str) A Koji target to build the image for.
		+ * ``git_branch`` -- (str) A branch in SCM for the ``Dockerfile``. This is
		`+ required by OSBS to avoid race conditions when multiple builds from the`
		+ same repo are submitted at the same time. Please note that ``url`` should
		`+ contain the branch or tag name as well, so that it can be resolved to a`
		`+ particular commit hash.`

		Optionally you can specify ``failable``. If it has a truthy value, failure
		`to create the image will not abort the whole compose.`
		`@@ -1319,6 +1324,7 @@`
		`# required`
		`"url": "git://example.com/dockerfiles.git?#HEAD",`
		`"target": "f24-docker-candidate",`
		`+ "git_branch": "f24-docker",`

		`# optional`
		`"name": "fedora-docker-base",`

pungi/checks.py

file modified

+2 -1

		`@@ -999,8 +999,9 @@`
		`"append": "repo_from",`
		`},`
		`"gpgkey": {"type": "string"},`
		`+ "git_branch": {"type": "string"},`
		`},`
		`- "required": ["url", "target"]`
		`+ "required": ["url", "target", "git_branch"]`
		`}`
		`},`
		`"additionalProperties": False,`

tests/test_config.py

file modified

		`@@ -250,6 +250,7 @@`
		`osbs={"^Server$": {`
		`'url': 'http://example.com',`
		`'target': 'f25-build',`
		`+ 'git_branch': 'f25',`
		`}}`
		`)`

tests/test_osbs_phase.py

file modified

+25 -4

		`@@ -257,13 +257,14 @@`
		`cfg = {`
		`'url': 'git://example.com/repo?#HEAD',`
		`'target': 'f24-docker-candidate',`
		`+ 'git_branch': 'f24-docker',`
		`}`
		`self._setupMock(KojiWrapper, resolve_git_url)`
		`self._assertConfigCorrect(cfg)`

		`self.t.process((self.compose, self.compose.variants['Server'], cfg), 1)`

		`- self._assertCorrectCalls({})`
		`+ self._assertCorrectCalls({'git_branch': 'f24-docker'})`
		`self._assertCorrectMetadata()`
		`self._assertRepoFile()`

		`@@ -273,6 +274,7 @@`
		`cfg = {`
		`'url': 'git://example.com/repo?#HEAD',`
		`'target': 'f24-docker-candidate',`
		`+ 'git_branch': 'f24-docker',`
		`'failable': ['*']`
		`}`
		`self._setupMock(KojiWrapper, resolve_git_url)`
		`@@ -280,7 +282,7 @@`

		`self.t.process((self.compose, self.compose.variants['Server'], cfg), 1)`

		`- self._assertCorrectCalls({})`
		`+ self._assertCorrectCalls({'git_branch': 'f24-docker'})`
		`self._assertCorrectMetadata()`
		`self._assertRepoFile()`

		`@@ -290,6 +292,7 @@`
		`cfg = {`
		`'url': 'git://example.com/repo?#HEAD',`
		`'target': 'f24-docker-candidate',`
		`+ 'git_branch': 'f24-docker',`
		`'name': 'my-name',`
		`'version': '1.0',`
		`}`
		`@@ -298,7 +301,7 @@`

		`self.t.process((self.compose, self.compose.variants['Server'], cfg), 1)`

		`- self._assertCorrectCalls({'name': 'my-name', 'version': '1.0'})`
		`+ self._assertCorrectCalls({'name': 'my-name', 'version': '1.0', 'git_branch': 'f24-docker'})`
		`self._assertCorrectMetadata()`
		`self._assertRepoFile()`

		`@@ -308,6 +311,7 @@`
		`cfg = {`
		`'url': 'git://example.com/repo?#HEAD',`
		`'target': 'f24-docker-candidate',`
		`+ 'git_branch': 'f24-docker',`
		`'name': 'my-name',`
		`'version': '1.0',`
		`'repo': ['Everything', 'http://pkgs.example.com/my.repo']`
		`@@ -320,6 +324,7 @@`
		`options = {`
		`'name': 'my-name',`
		`'version': '1.0',`
		`+ 'git_branch': 'f24-docker',`
		`'yum_repourls': [`
		`'http://root/work/global/tmp-Server/compose-rpms-1.repo',`
		`'http://root/work/global/tmp-Everything/compose-rpms-1.repo',`
		`@@ -336,6 +341,7 @@`
		`cfg = {`
		`'url': 'git://example.com/repo?#HEAD',`
		`'target': 'f24-docker-candidate',`
		`+ 'git_branch': 'f24-docker',`
		`'name': 'my-name',`
		`'version': '1.0',`
		`'repo': ['Everything', 'Client', 'http://pkgs.example.com/my.repo'],`
		`@@ -348,6 +354,7 @@`
		`options = {`
		`'name': 'my-name',`
		`'version': '1.0',`
		`+ 'git_branch': 'f24-docker',`
		`'yum_repourls': [`
		`'http://root/work/global/tmp-Server/compose-rpms-1.repo',`
		`'http://root/work/global/tmp-Everything/compose-rpms-1.repo',`
		`@@ -366,6 +373,7 @@`
		`cfg = {`
		`'url': 'git://example.com/repo?#HEAD',`
		`'target': 'f24-docker-candidate',`
		`+ 'git_branch': 'f24-docker',`
		`'name': 'my-name',`
		`'version': '1.0',`
		`'repo': ['Everything', 'Client', 'http://pkgs.example.com/my.repo'],`
		`@@ -384,6 +392,7 @@`
		`cfg = {`
		`'url': 'git://example.com/repo?#HEAD',`
		`'target': 'f24-docker-candidate',`
		`+ 'git_branch': 'f24-docker',`
		`'name': 'my-name',`
		`'version': '1.0',`
		`'repo': 'Gold',`
		`@@ -399,6 +408,7 @@`
		`def test_run_with_missing_url(self):`
		`cfg = {`
		`'target': 'f24-docker-candidate',`
		`+ 'git_branch': 'f24-docker',`
		`'name': 'my-name',`
		`}`
		`self._assertConfigMissing(cfg, 'url')`
		`@@ -406,16 +416,25 @@`
		`def test_run_with_missing_target(self):`
		`cfg = {`
		`'url': 'git://example.com/repo?#HEAD',`
		`+ 'git_branch': 'f24-docker',`
		`'name': 'my-name',`
		`}`
		`self._assertConfigMissing(cfg, 'target')`

		`+ def test_run_with_missing_git_branch(self):`
		`+ cfg = {`
		`+ 'url': 'git://example.com/repo?#HEAD',`
		`+ 'target': 'f24-docker-candidate',`
		`+ }`
		`+ self._assertConfigMissing(cfg, 'git_branch')`
		`+`
		`@mock.patch('pungi.util.resolve_git_url')`
		`@mock.patch('pungi.phases.osbs.kojiwrapper.KojiWrapper')`
		`def test_failing_task(self, KojiWrapper, resolve_git_url):`
		`cfg = {`
		`'url': 'git://example.com/repo?#HEAD',`
		`'target': 'fedora-24-docker-candidate',`
		`+ 'git_branch': 'f24-docker',`
		`}`
		`self._assertConfigCorrect(cfg)`
		`self._setupMock(KojiWrapper, resolve_git_url)`
		`@@ -432,6 +451,7 @@`
		`cfg = {`
		`'url': 'git://example.com/repo?#HEAD',`
		`'target': 'fedora-24-docker-candidate',`
		`+ 'git_branch': 'f24-docker',`
		`'failable': ['*']`
		`}`
		`self._assertConfigCorrect(cfg)`
		`@@ -446,6 +466,7 @@`
		`cfg = {`
		`'url': 'git://example.com/repo?#HEAD',`
		`'target': 'f24-docker-candidate',`
		`+ 'git_branch': 'f24-docker',`
		`'scratch': True,`
		`}`
		`self._setupMock(KojiWrapper, resolve_git_url, scratch=True)`
		`@@ -453,7 +474,7 @@`

		`self.t.process((self.compose, self.compose.variants['Server'], cfg), 1)`

		`- self._assertCorrectCalls({}, scratch=True)`
		`+ self._assertCorrectCalls({'git_branch': 'f24-docker'}, scratch=True)`
		`self._assertCorrectMetadata(scratch=True)`
		`self._assertRepoFile()`

lsedlar commented 7 years ago

In order to avoid conflicting tags, OSBS allows only one build for a repo/branch pair at the same time. To avoid race conditions, we should make sure we always pass in the branch. This commit makes it a required option.

This changes the config schema and updates documentations. The tests are updated too to supply the new required option.