Repository: cekit/cekit

Issue:
Support maintaining repository tags for release builds 
**Describe the solution you'd like**

We use cekit to build our container images. We publish container images from builds triggered by cekit. We would like to tag the container source repository to indicate which commits correspond to published images.

For example, if we built and shipped a container named foo which had a version: 1.0, I would like to tag the corresponding source repository commit with foo-1.0, or similar.

There are a bunch of issues here, but most fundamentally: cekit doesn't perform the release; that happens externally. So it's not as  simple as adding a tag to the `build --release` workflow. On the other hand, doing it manually (as now) opens up the risk of making mistakes, and once you've published a tag, you can't reasonably un-publish it (I've just found a mistake, which is what motivated me to finally file this enhancement request)

Perhaps cekit could create, but not push, tags in the local clone of the container sources, with a convention like "foo-1.0-candidate" or similar; leaving the user to create the real tag (`git tag foo-1.0 foo-1.0-candidate`) and push that. It would reduce the risk of mistakes at least.

Discussion:
jmtd: I'm really grateful for this feature, thanks! I've reviewed it and I've got a couple of comments but nothing blocking.

 * `--help` output description of `--tag` suggests that it takes an argument in all cases (`--tag TAG                 Use specified tag to tag the dist-git repository after build`), I don't think that's consistent with how it currently works
 * a scratchbuild (`--tag` without `--release`) won't work for tagging; cekit does not catch that at least in early stages of invocation (I'll return to this)
rnc: You are correct regarding the CLI help. I can add 
```
  +    default="{{name}}-{{version}}",
  +    show_default=True,
```
which will give
```
  --commit-message MESSAGE  Custom dist-git commit message.
  --tag TAG                 Use specified tag to tag the dist-git repository after build
                            [default: {{name}}-{{version}}]
```
which matches other CLI arguments that take an optional component with a known default. 

Regarding scratch builds - I thought I wrote it so it would **not** try to tag - the code should output:
```
            LOGGER.warning(
                f"No koji build found - maybe {task_id} was a scratch build?"
            )
```

jmtd: >  I can add

Looks perfect

> Unless - did you mean it doesn't try to catch it earlier ?

Yes, I meant earlier: I think now `--tag` doesn't make sense without also `--release`. In fact, is this a problem? Perhaps there's a use-case for tagging when not doing releases. Or even using OSBS but that's a bigger thing (since this is osbs subsystem specific code). Of course I'm speculating, because I am using it for OSBS release builds... 

I don't think this is a blocking issue fwiw!
jmtd: Example annotated tags composed by this PR: https://github.com/jboss-container-images/openjdk/releases/tag/openjdk-openjdk-11-rhel7-1.15-6 and https://github.com/jboss-container-images/openjdk/releases/tag/openjdk-openjdk-11-rhel7-1.15-8
rnc: > 
> Yes, I meant earlier: I think now `--tag` doesn't make sense without also `--release`. In fact, is this a problem? Perhaps there's a use-case for tagging when not doing releases. Or even using OSBS but that's a bigger thing (since this is osbs subsystem specific code). Of course I'm speculating, because I am using it for OSBS release builds...
> 
> I don't think this is a blocking issue fwiw!

I think we could do this with a callback function in click


Patch:
diff --git a/cekit/builders/osbs.py b/cekit/builders/osbs.py
index 9b011ac..4b42669 100644
--- a/cekit/builders/osbs.py
+++ b/cekit/builders/osbs.py
@@ -40,7 +40,7 @@ class OSBSBuilder(Builder):
 
         self._rhpkg_set_url_repos: List[str] = []
         self.artifacts: List[str] = []
-        self.dist_git: DistGit
+        self.git: Git
         self.dist_git_dir: pathlib.Path
 
         if CONFIG.get("common", "redhat"):
@@ -154,7 +154,7 @@ class OSBSBuilder(Builder):
 
         LOGGER.debug("Using dist-git directory of {}".format(self.dist_git_dir))
 
-        self.dist_git = DistGit(
+        self.git = Git(
             self.dist_git_dir,
             self.target,
             repository,
@@ -164,8 +164,8 @@ class OSBSBuilder(Builder):
             self.params.assume_yes,
         )
 
-        self.dist_git.prepare(self.params.stage, self.params.user)
-        self.dist_git.clean(self.artifacts)
+        self.git.prepare(self.params.stage, self.params.user)
+        self.git.clean(self.artifacts)
 
     def _copy_to_dist_git(self):
         LOGGER.debug(
@@ -175,12 +175,12 @@ class OSBSBuilder(Builder):
 
     def _sync_with_dist_git(self):
         with Chdir(self.dist_git_dir):
-            self.dist_git.add(self.artifacts)
+            self.git.add(self.artifacts)
             self.update_lookaside_cache()
 
-            if self.dist_git.stage_modified():
-                self.dist_git.commit(self.params.commit_message)
-                self.dist_git.push()
+            if self.git.stage_modified():
+                self.git.commit(self.params.commit_message)
+                self.git.push()
             else:
                 LOGGER.info("No changes made to the code, committing skipped")
 
@@ -246,7 +246,6 @@ class OSBSBuilder(Builder):
         LOGGER.info("Updating lookaside cache...")
 
         cache_artifacts = []
-
         for artifact in self.artifacts:
             # In case the artifact is a directory, we don't want to add it.
             # Instead, it will be staged.
@@ -275,7 +274,9 @@ class OSBSBuilder(Builder):
             )
             return
 
-        cmd = [self._koji]
+        build_id: str = ""
+        git_tag: str = ""
+        cmd: List[str] = [self._koji]
 
         if self.params.user:
             cmd += ["--user", self.params.user]
@@ -301,7 +302,7 @@ class OSBSBuilder(Builder):
             # If target was not specified in the image descriptor
             if not target:
                 # Default to computed target based on branch
-                target = "{}-containers-candidate".format(self.dist_git.branch)
+                target = "{}-containers-candidate".format(self.git.branch)
 
             scratch = True
 
@@ -309,7 +310,7 @@ class OSBSBuilder(Builder):
                 scratch = False
 
             kwargs = "{{'src': '{}', 'target': '{}', 'opts': {{'scratch': {}, 'git_branch': '{}', 'yum_repourls': {}}}}}".format(
-                src, target, scratch, self.dist_git.branch, self._rhpkg_set_url_repos
+                src, target, scratch, self.git.branch, self._rhpkg_set_url_repos
             )
 
             cmd.append(kwargs)
@@ -340,15 +341,63 @@ class OSBSBuilder(Builder):
                 LOGGER.info("Image was built successfully in OSBS!")
 
                 if self.params.tag:
-                    tag = Template(self.params.tag).render(self.generator.image)
-                    if "/" in tag:
-                        LOGGER.debug(f"Replacing / in tag with - for {tag}")
-                        tag = tag.replace("/", "-")
-                    self.dist_git.tag(tag)
-                    self.dist_git.push(True)
+                    git_tag, build_id = self._establish_tag_name(task_id)
+                    if git_tag:
+                        self.git.tag(src, git_tag, build_id)
+                        self.git.push(git_tag)
+        if self.params.tag:
+            # This is a bit of code reuse cheat - if tagging is enabled this will also tag
+            # the image source repository (while the above block handles the dist-git source)
+            url = run_wrapper(
+                ["git", "config", "--get", "remote.origin.url"],
+                capture_output=True,
+                check=False,
+            ).stdout
+            if git_tag:
+                self.git.tag(url, git_tag, build_id)
+                self.git.push(git_tag)
+
+    def _establish_tag_name(self, task_id: str) -> Tuple[str, str]:
+        """This calls Brew to establish the NVR from the completed build in order to use that to tag the repositories
+
+        :param task_id: A task_id to use to look up the NVR
+        :returns a Tuple containing a tag and build_id. Maybe blank if the task was a scratch build.
+        """
+        tag: str = ""
+        build_id: str = ""
+
+        task_result = run_wrapper(
+            # Don't need to handle raise_fault as OSBS build must have completed successfully to get to here.
+            [self._koji, "call", "--json-output", "getTaskResult", task_id],
+            True,
+            f"Could not check the task {task_id} result",
+        )
+
+        koji_builds = json.loads(task_result.stdout)["koji_builds"]
+        if koji_builds:
+            build_id = koji_builds[0]
+            build_result = run_wrapper(
+                [self._koji, "call", "--json-output", "getBuild", build_id],
+                True,
+                f"Could not check the build {build_id} result",
+            )
+            nvr: str = json.loads(build_result.stdout)["nvr"]
+            release: str = nvr.rpartition("-")[2]
+            # The default rendered tag (name-version) with release equates to a NVR.
+            tag: str = Template(self.params.tag).render(self.generator.image)
+            if "/" in tag:
+                LOGGER.debug(f"Replacing / in tag with - for {tag}")
+                tag = tag.replace("/", "-")
+            tag = tag + "-" + release
+            LOGGER.info(f"Retrieved NVR {nvr} and generated {tag}")
+        else:
+            LOGGER.warning(
+                f"No koji build found - maybe {task_id} was a scratch build?"
+            )
+        return tag, build_id
 
 
-class DistGit(object):
+class Git(object):
     """Git support for osbs repositories"""
 
     @staticmethod
@@ -393,7 +442,7 @@ class DistGit(object):
             self.source_repo_name,
             self.source_repo_branch,
             self.source_repo_commit,
-        ) = DistGit.repo_info(source)
+        ) = Git.repo_info(source)
 
     def stage_modified(self) -> bool:
         # Check if there are any files in stage (return code 1). If there are no files
@@ -539,16 +588,27 @@ class DistGit(object):
                 },
             )
 
-    def tag(self, tag: str) -> None:
-        LOGGER.info(f"Tagging dist-git repository with {tag}")
-        run_wrapper(["git", "tag", tag], False)
+    def tag(self, url: str, tag: str, build_id) -> None:
+        LOGGER.info(f"Tagging git repository ({url}) with {tag} from build {build_id}")
+        # cgit requires annotated tags.
+        run_wrapper(
+            [
+                "git",
+                "tag",
+                "-a",
+                "-m",
+                f"CEKit automated tag from build {build_id}",
+                tag,
+            ],
+            False,
+        )
 
-    def push(self, tag: bool = False) -> None:
+    def push(self, tag: str = None) -> None:
         if self.noninteractive or tools.decision("Do you want to push the commit?"):
             print("")
             LOGGER.info("Pushing change to the upstream repository...")
             if tag:
-                cmd = ["git", "push", "--tags"]
+                cmd = ["git", "push", "origin", tag]
             else:
                 cmd = ["git", "push", "-q", "origin", self.branch]
             run_wrapper(cmd, False)
diff --git a/cekit/cli.py b/cekit/cli.py
index 5e6d36a..3e1b488 100644
--- a/cekit/cli.py
+++ b/cekit/cli.py
@@ -22,6 +22,14 @@ LOGGER = logging.getLogger("cekit")
 CONFIG = Config()
 
 
+def verify_release_passed(ctx, param, value):
+    if not ctx.params.get("release") and value:
+        raise click.UsageError(
+            "--tag requires --release as scratch builds cannot be tagged"
+        )
+    return value
+
+
 @click.group(context_settings=dict(max_content_width=100))
 @click.option(
     "--descriptor",
@@ -233,7 +241,7 @@ def build_podman(ctx, pull, no_squash, tags, platform):
 
 
 @build.command(name="osbs", short_help="Build using OSBS engine")
-@click.option("--release", help="Execute a release build.", is_flag=True)
+@click.option("--release", help="Execute a release build.", is_flag=True, is_eager=True)
 @click.option("--user", metavar="USER", help="User used to kick the build as.")
 @click.option("--nowait", help="Do not wait for the task to finish.", is_flag=True)
 @click.option("--stage", help="Use stage environment.", is_flag=True)
@@ -249,8 +257,9 @@ def build_podman(ctx, pull, no_squash, tags, platform):
     "--tag",
     is_flag=False,
     flag_value="{{name}}-{{version}}",
-    metavar="TAG",
-    help="Use specified tag to tag the dist-git repository after build",
+    metavar="<TAG>",
+    help="Tag the source and dist-git repositories after build. Default format if not supplied is {{name}}-{{version}}",
+    callback=verify_release_passed,
 )
 @click.option(
     "--assume-yes", "-y", help="Execute build in non-interactive mode.", is_flag=True
diff --git a/cekit/tools.py b/cekit/tools.py
index 3c3d91d..a542c7e 100644
--- a/cekit/tools.py
+++ b/cekit/tools.py
@@ -343,7 +343,11 @@ def run_wrapper(
     :param check: Whether to check the return code (defaults to True)
     :return: a CompletedProcess object
     """
-    logger.debug("Executing '{}'.".format(" ".join(cmd)))
+    logger.debug(
+        "Executing '{}'.".format(
+            " ".join("'" + w + "'" if " " in w else w for w in cmd)
+        )
+    )
     try:
         # While it would be nicer to use
         #   result = subprocess.run(
diff --git a/docs/handbook/building/builder-engines.rst b/docs/handbook/building/builder-engines.rst
index 35be1d3..dbdd952 100644
--- a/docs/handbook/building/builder-engines.rst
+++ b/docs/handbook/building/builder-engines.rst
@@ -148,11 +148,15 @@ Parameters
     ``--tag`` <tag-name>
         .. versionadded:: 4.4.0
 
-        An optional tag to be applied to the dist-git repository after a successful build.
+        An optional annotated tag to be applied to **both** the source and the dist-git repository after a successful
+        non-scratch build.
+
         If the tag parameter is **not** specified then it will be constructed from the image
-        name (with ``/`` changed to ``-``) and the image version. The tag name may be constructed
-        using Jinja template e.g. ``{{name}}-{{version}}`` referencing the keys in the image
-        descriptor.
+        name (with ``/`` changed to ``-``) and the image version. Finally the release increment will be retrieved
+        from the OSBS Brew build and appended. The default tag name equates to the NVR.
+
+        The tag name may be constructed using Jinja template e.g. ``{{name}}-{{version}}`` referencing the
+        keys in the image descriptor.
     ``--sync-only``
         .. versionadded:: 3.4
 
diff --git a/docs/requirements.txt b/docs/requirements.txt
index 640168c..d522d5e 100644
--- a/docs/requirements.txt
+++ b/docs/requirements.txt
@@ -1,4 +1,4 @@
-sphinx>=1.7.6
+sphinx>=1.7.6,<7.0.0
 sphinx-autobuild==0.7.1
 https://github.com/guzzle/guzzle_sphinx_theme/archive/eefd45b79383b1b4aab1607444e41366fd1348a6.zip
 pykwalify>=1.6.0


Test patch:
diff --git a/tests/test_builder.py b/tests/test_builder.py
index 4000f52..8a3a070 100644
--- a/tests/test_builder.py
+++ b/tests/test_builder.py
@@ -154,7 +154,7 @@ def test_merge_container_yaml_limit_arch(mocker, tmpdir):
     assert len(container_yaml["platforms"]["only"]) == 1
 
 
-class DistGitMock(object):
+class GitMock(object):
     def add(self, artifacts):
         pass
 
@@ -191,7 +191,7 @@ def create_builder_object(
 
     builder = BuilderImpl(Map(merge_dicts(common_params, params)))
     builder.dist_git_dir = "/tmp"
-    builder.dist_git = DistGitMock()
+    builder.git = GitMock()
     builder.artifacts = []
     return builder
 
@@ -217,7 +217,7 @@ def test_osbs_builder_run_brew_stage(mocker):
     builder = create_builder_object(mocker, "osbs", params)
     builder.generator = Map({"image": Map({})})
     mocker.patch.object(builder, "_wait_for_osbs_task")
-    builder.dist_git.branch = "some-branch"
+    builder.git.branch = "some-branch"
     builder.run()
 
     run.assert_has_calls(
@@ -276,7 +276,7 @@ def test_osbs_builder_run_brew(mocker):
     builder = create_builder_object(mocker, "osbs", {})
     builder.generator = Map({"image": Map({})})
     mocker.patch.object(builder, "_wait_for_osbs_task")
-    builder.dist_git.branch = "some-branch"
+    builder.git.branch = "some-branch"
     builder.run()
 
     run.assert_has_calls(
@@ -335,7 +335,7 @@ def test_osbs_builder_run_koji(mocker):
     )
     builder.generator = Map({"image": Map({})})
     mocker.patch.object(builder, "_wait_for_osbs_task")
-    builder.dist_git.branch = "some-branch"
+    builder.git.branch = "some-branch"
     builder.run()
 
     run.assert_has_calls(
@@ -394,7 +394,7 @@ def test_osbs_builder_run_brew_nowait(mocker):
     builder = create_builder_object(mocker, "osbs", params)
     builder.generator = Map({"image": Map({})})
     mocker.patch.object(builder, "_wait_for_osbs_task")
-    builder.dist_git.branch = "some-branch"
+    builder.git.branch = "some-branch"
     builder.run()
 
     builder._wait_for_osbs_task.assert_not_called()
@@ -421,7 +421,7 @@ def test_osbs_builder_run_brew_user(mocker):
     builder = create_builder_object(mocker, "osbs", params)
     builder.generator = Map({"image": Map({})})
     mocker.patch.object(builder, "_wait_for_osbs_task")
-    builder.dist_git.branch = "some-branch"
+    builder.git.branch = "some-branch"
     builder.run()
 
     run.assert_called_with(
@@ -464,7 +464,7 @@ def test_osbs_builder_run_brew_target_defined_in_descriptor(mocker):
         {"image": Map({"osbs": Map({"koji_target": "some-target"})})}
     )
     mocker.patch.object(builder, "_wait_for_osbs_task")
-    builder.dist_git.branch = "some-branch"
+    builder.git.branch = "some-branch"
     builder.run()
 
     run.assert_called_with(
@@ -668,8 +668,8 @@ def test_osbs_copy_artifacts_to_dist_git(mocker, tmpdir, artifact, src, target):
     mocker.patch("cekit.descriptor.resource.Resource.copy")
     copy_mock = mocker.patch("cekit.builders.osbs.shutil.copy2")
 
-    dist_git_class = mocker.patch("cekit.builders.osbs.DistGit")
-    dist_git_class.return_value = DistGitMock()
+    dist_git_class = mocker.patch("cekit.builders.osbs.Git")
+    dist_git_class.return_value = GitMock()
 
     config.cfg["common"] = {"redhat": True, "work_dir": str(tmpdir)}
     config.cfg["doc"] = {"addhelp": False}
diff --git a/tests/test_integ_builder_osbs.py b/tests/test_integ_builder_osbs.py
index 6601f21..1d5e50d 100644
--- a/tests/test_integ_builder_osbs.py
+++ b/tests/test_integ_builder_osbs.py
@@ -76,7 +76,7 @@ def run_osbs(
     # We are mocking it, so do not require it at test time
     mocker.patch("cekit.builders.osbs.OSBSBuilder.dependencies", return_value={})
     mocker.patch("cekit.builders.osbs.OSBSBuilder._wait_for_osbs_task")
-    mocker.patch("cekit.builders.osbs.DistGit.prepare")
+    mocker.patch("cekit.builders.osbs.Git.prepare")
 
     side_affect = [
         subprocess.CompletedProcess(
@@ -134,9 +134,20 @@ def run_osbs(
                 "", 0, "3b9283cb26b35511517ff5c0c3e11f490cba8feb"
             ),  # git rev-parse HEAD
             subprocess.CompletedProcess("", 0, "1234"),  # brew call --python...
-            subprocess.CompletedProcess("", 0, "UUU"),
-            # Extra needed for test_osbs_builder_with_fetch_artifacts_url_file_creation_5
-            subprocess.CompletedProcess("", 0, ""),  # git add --all [Optional]
+            # For git tagging
+            subprocess.CompletedProcess(
+                "", 0, """{"koji_builds": ["123456"]}"""
+            ),  # getTaskResult
+            subprocess.CompletedProcess(
+                "", 0, """{"nvr": "org.foo-foobar-1.0-1"}"""
+            ),  # getBuild
+            subprocess.CompletedProcess("", 0, ""),  # git tag [Optional]
+            subprocess.CompletedProcess("", 0, ""),  # git push [Optional]
+            subprocess.CompletedProcess(
+                "", 0, "https://my.cekit.repo/foo"
+            ),  # git config [Optional]
+            subprocess.CompletedProcess("", 0, ""),  # git tag [Optional]
+            subprocess.CompletedProcess("", 0, ""),  # git push [Optional]
         ]
     )
 
@@ -1168,7 +1179,7 @@ def test_osbs_builder_with_fetch_artifacts_plain_file_creation(tmpdir, mocker, c
     mocker.patch(
         "cekit.generator.osbs.get_brew_url", return_value="http://random.url/path"
     )
-    mocker.patch("cekit.builders.osbs.DistGit.push")
+    mocker.patch("cekit.builders.osbs.Git.push")
 
     tmpdir.mkdir("osbs").mkdir("repo")
 
@@ -1217,7 +1228,7 @@ def test_osbs_builder_with_fetch_artifacts_url_file_creation_1(
 
     mocker.patch("cekit.tools.decision", return_value=True)
     mocker.patch("cekit.tools.urlopen")
-    mocker.patch("cekit.builders.osbs.DistGit.push")
+    mocker.patch("cekit.builders.osbs.Git.push")
 
     tmpdir.mkdir("osbs").mkdir("repo")
 
@@ -1263,7 +1274,7 @@ def test_osbs_builder_with_fetch_artifacts_url_file_creation_2(tmpdir, mocker, c
 
     mocker.patch("cekit.tools.decision", return_value=True)
     mocker.patch("cekit.tools.urlopen")
-    mocker.patch("cekit.builders.osbs.DistGit.push")
+    mocker.patch("cekit.builders.osbs.Git.push")
 
     descriptor = copy.deepcopy(image_descriptor)
 
@@ -1302,7 +1313,7 @@ def test_osbs_builder_with_fetch_artifacts_url_file_creation_3(tmpdir, mocker, c
 
     mocker.patch("cekit.tools.decision", return_value=True)
     mocker.patch("cekit.tools.urlopen")
-    mocker.patch("cekit.builders.osbs.DistGit.push")
+    mocker.patch("cekit.builders.osbs.Git.push")
 
     tmpdir.mkdir("osbs").mkdir("repo")
 
@@ -1346,7 +1357,7 @@ def test_osbs_builder_with_fetch_artifacts_url_file_creation_4(tmpdir, mocker, c
 
     mocker.patch("cekit.tools.urlopen", return_value=res)
     mocker.patch("cekit.tools.decision", return_value=True)
-    mocker.patch("cekit.builders.osbs.DistGit.push")
+    mocker.patch("cekit.builders.osbs.Git.push")
 
     tmpdir.mkdir("osbs").mkdir("repo")
 
@@ -1382,7 +1393,7 @@ def test_osbs_builder_with_fetch_artifacts_url_file_creation_5(tmpdir, mocker, c
 
     mocker.patch("cekit.tools.decision", return_value=True)
     mocker.patch("cekit.tools.urlopen")
-    mocker.patch("cekit.builders.osbs.DistGit.push")
+    mocker.patch("cekit.builders.osbs.Git.push")
 
     tmpdir.mkdir("osbs").mkdir("repo")
 
@@ -1467,7 +1478,7 @@ def test_osbs_builder_with_fetch_artifacts_url_file_creation_multiple_hash(
 
     mocker.patch("cekit.tools.urlopen", return_value=res)
     mocker.patch("cekit.tools.decision", return_value=True)
-    mocker.patch("cekit.builders.osbs.DistGit.push")
+    mocker.patch("cekit.builders.osbs.Git.push")
 
     tmpdir.mkdir("osbs").mkdir("repo")
 
@@ -1512,7 +1523,7 @@ def test_osbs_builder_with_fetch_artifacts_url_file_creation_naming(
 
     mocker.patch("cekit.tools.urlopen", return_value=res)
     mocker.patch("cekit.tools.decision", return_value=True)
-    mocker.patch("cekit.builders.osbs.DistGit.push")
+    mocker.patch("cekit.builders.osbs.Git.push")
 
     tmpdir.mkdir("osbs").mkdir("repo")
 
@@ -1557,7 +1568,7 @@ def test_osbs_builder_with_fetch_artifacts_url_file_creation_naming_with_target(
 
     mocker.patch("cekit.tools.urlopen", return_value=res)
     mocker.patch("cekit.tools.decision", return_value=True)
-    mocker.patch("cekit.builders.osbs.DistGit.push")
+    mocker.patch("cekit.builders.osbs.Git.push")
 
     tmpdir.mkdir("osbs").mkdir("repo")
 
@@ -1607,7 +1618,7 @@ def test_osbs_builder_with_fetch_artifacts_url_validate_dockerfile(
 
     mocker.patch("cekit.tools.urlopen", return_value=res)
     mocker.patch("cekit.tools.decision", return_value=True)
-    mocker.patch("cekit.builders.osbs.DistGit.push")
+    mocker.patch("cekit.builders.osbs.Git.push")
 
     tmpdir.mkdir("osbs").mkdir("repo")
 
@@ -1661,7 +1672,7 @@ def test_osbs_builder_with_fetch_artifacts_url_file_removal(tmpdir, mocker, capl
     mocker.patch(
         "cekit.generator.osbs.get_brew_url", return_value="http://random.url/path"
     )
-    mocker.patch("cekit.builders.osbs.DistGit.push")
+    mocker.patch("cekit.builders.osbs.Git.push")
 
     tmpdir.mkdir("osbs").mkdir("repo")
 
@@ -1683,7 +1694,7 @@ def test_osbs_builder_with_fetch_artifacts_url_file_source_fail(tmpdir, mocker,
     """
     mocker.patch("cekit.tools.decision", return_value=True)
     mocker.patch("cekit.tools.urlopen")
-    mocker.patch("cekit.builders.osbs.DistGit.push")
+    mocker.patch("cekit.builders.osbs.Git.push")
 
     tmpdir.mkdir("osbs").mkdir("repo")
 
@@ -1728,7 +1739,7 @@ def test_osbs_builder_with_fetch_artifacts_url_file_source_1(tmpdir, mocker, cap
 
     mocker.patch("cekit.tools.decision", return_value=True)
     mocker.patch("cekit.tools.urlopen")
-    mocker.patch("cekit.builders.osbs.DistGit.push")
+    mocker.patch("cekit.builders.osbs.Git.push")
 
     tmpdir.mkdir("osbs").mkdir("repo")
 
@@ -1804,7 +1815,7 @@ def test_osbs_builder_with_fetch_artifacts_url_file_source_2(tmpdir, mocker, cap
 
     mocker.patch("cekit.tools.decision", return_value=True)
     mocker.patch("cekit.tools.urlopen")
-    mocker.patch("cekit.builders.osbs.DistGit.push")
+    mocker.patch("cekit.builders.osbs.Git.push")
 
     tmpdir.mkdir("osbs").mkdir("repo")
 
@@ -1886,7 +1897,7 @@ def test_osbs_builder_with_fetch_artifacts_pnc_file_removal(tmpdir, mocker, capl
     mocker.patch(
         "cekit.generator.osbs.get_brew_url", return_value="http://random.url/path"
     )
-    mocker.patch("cekit.builders.osbs.DistGit.push")
+    mocker.patch("cekit.builders.osbs.Git.push")
 
     tmpdir.mkdir("osbs").mkdir("repo")
 
@@ -1918,7 +1929,7 @@ def test_osbs_builder_container_yaml_existence(tmpdir, mocker, caplog, flag):
     mocker.patch(
         "cekit.generator.osbs.get_brew_url", return_value="http://random.url/path"
     )
-    mocker.patch("cekit.builders.osbs.DistGit.push")
+    mocker.patch("cekit.builders.osbs.Git.push")
 
     tmpdir.mkdir("osbs").mkdir("repo")
 
@@ -1958,7 +1969,7 @@ def test_osbs_builder_with_cachito_enabled(tmpdir, mocker, caplog):
     mocker.patch(
         "cekit.generator.osbs.get_brew_url", return_value="http://random.url/path"
     )
-    mocker.patch("cekit.builders.osbs.DistGit.push")
+    mocker.patch("cekit.builders.osbs.Git.push")
 
     tmpdir.mkdir("osbs").mkdir("repo")
 
@@ -2396,7 +2407,7 @@ def test_osbs_builder_with_fetch_artifacts_pnc_file_creation_1(tmpdir, mocker, c
 
     mocker.patch("cekit.tools.decision", return_value=True)
     mocker.patch("cekit.tools.urlopen")
-    mocker.patch("cekit.builders.osbs.DistGit.push")
+    mocker.patch("cekit.builders.osbs.Git.push")
 
     tmpdir.mkdir("osbs").mkdir("repo")
 
@@ -2428,7 +2439,7 @@ def test_osbs_builder_with_fetch_artifacts_pnc_file_creation_2(tmpdir, mocker, c
 
     mocker.patch("cekit.tools.decision", return_value=True)
     mocker.patch("cekit.tools.urlopen")
-    mocker.patch("cekit.builders.osbs.DistGit.push")
+    mocker.patch("cekit.builders.osbs.Git.push")
 
     tmpdir.mkdir("osbs").mkdir("repo")
 
@@ -2513,7 +2524,7 @@ def test_osbs_builder_with_brew_and_lookaside(tmpdir, mocker, caplog):
     mocker.patch(
         "cekit.generator.osbs.get_brew_url", return_value="http://random.url/path"
     )
-    mocker.patch("cekit.builders.osbs.DistGit.push")
+    mocker.patch("cekit.builders.osbs.Git.push")
 
     work_dir = str(tmpdir.mkdir("work_dir"))
     image_dir = str(tmpdir)
@@ -2597,7 +2608,7 @@ def test_osbs_builder_kick_build_with_tag_1(tmpdir, mocker, caplog):
         descriptor,
         str(source_dir),
         mocker,
-        build_command=["build", "osbs", "--tag", "FOOBAR"],
+        build_command=["build", "osbs", "--release", "--tag", "FOOBAR"],
     )
 
     assert os.path.exists(str(repo_dir.join("Dockerfile"))) is True
@@ -2640,7 +2651,14 @@ def test_osbs_builder_kick_build_with_tag_1(tmpdir, mocker, caplog):
         in caplog.text
     )
     assert "Image was built successfully in OSBS!" in caplog.text
-    assert "Tagging dist-git repository with FOOBAR" in caplog.text
+    assert (
+        "Tagging git repository (git://somehost.com/containers/somerepo#3b9283cb26b35511517ff5c0c3e11f490cba8feb) with FOOBAR-1 from build 123456"
+        in caplog.text
+    )
+    assert (
+        "Tagging git repository (https://my.cekit.repo/foo) with FOOBAR-1 from build 123456"
+        in caplog.text
+    )
 
 
 def test_osbs_builder_kick_build_with_tag_2(tmpdir, mocker, caplog):
@@ -2661,7 +2679,7 @@ def test_osbs_builder_kick_build_with_tag_2(tmpdir, mocker, caplog):
         descriptor,
         str(source_dir),
         mocker,
-        build_command=["build", "osbs", "--tag"],
+        build_command=["build", "osbs", "--release", "--tag"],
     )
 
     assert os.path.exists(str(repo_dir.join("Dockerfile"))) is True
@@ -2704,4 +2722,28 @@ def test_osbs_builder_kick_build_with_tag_2(tmpdir, mocker, caplog):
         in caplog.text
     )
     assert "Image was built successfully in OSBS!" in caplog.text
-    assert "Tagging dist-git repository with test-image-1.0" in caplog.text
+    assert (
+        "Tagging git repository (git://somehost.com/containers/somerepo#3b9283cb26b35511517ff5c0c3e11f490cba8feb) with test-image-1.0-1 from build 123456"
+        in caplog.text
+    )
+    assert (
+        "Tagging git repository (https://my.cekit.repo/foo) with test-image-1.0-1 from build 123456"
+        in caplog.text
+    )
+
+
+def test_osbs_builder_kick_build_with_tag_3(tmpdir):
+    """
+    Verify tag and release flags.
+    """
+    run_cekit(
+        str(tmpdir),
+        parameters=[
+            "-v",
+            "build",
+            "osbs",
+            "--tag",
+        ],
+        message="--tag requires --release as scratch builds cannot be tagged",
+        return_code=2,
+    )