Tag release branch (#18413)

* Tag release branch

* add changelog

* Apply suggestions from code review

Co-authored-by: dkirov-dd <[email protected]>

* Some more feedback, fixes, features

* reduce duplication and complexity

---------

Co-authored-by: dkirov-dd <[email protected]>

ddev/changelog.d/18413.added

@@ -0,0 +1 @@
+Add command to tag the Agent release branch. It supports both RC and final tags, see `ddev release branch tag --help` for more details.

ddev/src/ddev/cli/release/branch/__init__.py

@@ -4,6 +4,7 @@
 import click
 
 from ddev.cli.release.branch.create import create
+from ddev.cli.release.branch.tag import tag
 
 
 @click.group(short_help='Manage release branches')
@@ -14,3 +15,4 @@ def branch():
 
 
 branch.add_command(create)
+branch.add_command(tag)

ddev/src/ddev/cli/release/branch/tag.py

@@ -0,0 +1,74 @@
+import re
+
+import click
+from packaging.version import Version
+
+from .create import BRANCH_NAME_REGEX
+
+
+@click.command
+@click.option(
+    '--final/--rc',
+    default=False,
+    show_default=True,
+    help="Whether we're tagging the final release or a release candidate (rc).",
+)
+@click.pass_obj
+def tag(app, final):
+    """
+    Tag the release branch either as release candidate or final release.
+    """
+    branch_name = app.repo.git.current_branch()
+    release_branch = BRANCH_NAME_REGEX.match(branch_name)
+    if release_branch is None:
+        app.abort(
+            f'Invalid branch name: {branch_name}. Branch name must match the pattern {BRANCH_NAME_REGEX.pattern}.'
+        )
+    click.echo(app.repo.git.pull(branch_name))
+    click.echo(app.repo.git.fetch_tags())
+    major_minor_version = branch_name.replace('.x', '')
+    this_release_tags = sorted(
+        (
+            Version(t)
+            for t in set(app.repo.git.tags(glob_pattern=major_minor_version + '.*'))
+            # We take 'major.minor.x' as the branch name pattern and replace 'x' with 'patch-rc.number'.
+            # We make the RC component optional so that final tags also match our filter.
+            if re.match(BRANCH_NAME_REGEX.pattern.replace('x', r'\d+(\-rc\.\d+)?'), t)
+        ),
+        reverse=True,
+    )
+    last_patch, last_rc = _extract_patch_and_rc(this_release_tags)
+    last_tag_was_final = last_rc is None
+    new_patch = last_patch + 1 if last_tag_was_final else last_patch
+    new_tag = f'{major_minor_version}.{new_patch}'
+    if not final:
+        new_rc_guess = 1 if last_tag_was_final else last_rc + 1
+        next_rc = click.prompt(
+            'What RC number are we tagging? (hit ENTER to accept suggestion)', type=int, default=new_rc_guess
+        )
+        if next_rc < 1:
+            app.abort('RC number must be at least 1.')
+        new_tag += f'-rc.{next_rc}'
+        if Version(new_tag) in this_release_tags:
+            app.abort(f'Tag {new_tag} already exists. Switch to git to overwrite it.')
+        if not last_tag_was_final and next_rc < last_rc:
+            click.secho('!!! WARNING !!!')
+            if not click.confirm(
+                f'The latest RC is {last_rc}. '
+                'You are about to go back in time by creating an RC with a number less than that. Are you sure? [y/N]'
+            ):
+                app.abort('Did not get confirmation, aborting. Did not create or push the tag.')
+    if not click.confirm(f'Create and push this tag: {new_tag}?'):
+        app.abort('Did not get confirmation, aborting. Did not create or push the tag.')
+    click.echo(app.repo.git.tag(new_tag, message=new_tag))
+    click.echo(app.repo.git.push(new_tag))
+
+
+def _extract_patch_and_rc(version_tags):
+    if not version_tags:
+        return 0, 0
+    latest = version_tags[0]
+    patch = latest.micro
+    # latest.pre is None for final releases and a tuple ('rc', <NUM>) for RC.
+    rc = latest.pre if latest.pre is None else latest.pre[1]
+    return patch, rc

ddev/src/ddev/utils/git.py

@@ -40,11 +40,26 @@ def latest_commit(self) -> GitCommit:
         sha, subject = self.capture('log', '-1', '--format=%H%n%s').splitlines()
         return GitCommit(sha, subject=subject)
 
+    def pull(self, ref):
+        return self.capture('pull', 'origin', ref)
+
+    def push(self, ref):
+        return self.capture('push', 'origin', ref)
+
+    def tag(self, value, message=None):
+        """
+        Create a tag with an optional message.
+        """
+        cmd = ['tag', value]
+        if message is not None:
+            cmd.extend(['--message', value])
+        return self.capture(*cmd)
+
     def tags(self, glob_pattern=None) -> list[str]:
         """
         List the repo's tags and sort them.
 
-        If provided, `glob_pattern` filters tags just like the pattern argument to `git tag --list`.
+        If not None, we pass `glob_pattern` as the pattern argument to `git tag --list`.
         """
 
         cmd = ['tag', '--list']

ddev/tests/cli/release/branch/test_tag.py

@@ -0,0 +1,226 @@
+# (C) Datadog, Inc. 2024-present
+# All rights reserved
+# Licensed under a 3-clause BSD style license (see LICENSE)
+from unittest.mock import call as c
+
+import pytest
+
+from ddev.utils.git import GitRepository
+
+NO_CONFIRMATION_SO_ABORT = 'Did not get confirmation, aborting. Did not create or push the tag.'
+RC_NUMBER_PROMPT = 'What RC number are we tagging? (hit ENTER to accept suggestion) [{}]'
+
+EXAMPLE_TAGS = [
+    '7.56.0-rc.1',
+    # Random RC tag from DBM. We should make sure we ignore it.
+    '7.56.0-rc.1-dbm-agent-jobs',
+    # Icluding RC 11 is interesting because it makes sure we we parse the versions before we sort them.
+    # The naive sort will think RC 11 is earlier than RC 2.
+    '7.56.0-rc.11',
+    '7.56.0-rc.2',
+    # Skipping RCs, we go from 2 to 5.
+    '7.56.0-rc.5',
+    '7.56.0-rc.6',
+    '7.56.0-rc.7',
+    '7.56.0-rc.8',
+]
+
+
+@pytest.fixture
+def basic_git(mocker):
+    mock_git = mocker.create_autospec(GitRepository)
+    # We're patching the creation of the GitRepository class.
+    # That's why we need a function that returns the mock.
+    mocker.patch('ddev.repo.core.GitRepository', lambda _: mock_git)
+    return mock_git
+
+
+@pytest.fixture
+def git(basic_git):
+    basic_git.current_branch.return_value = '7.56.x'
+    basic_git.tags.return_value = EXAMPLE_TAGS[:]
+    yield basic_git
+    assert basic_git.method_calls[:4] == [
+        c.current_branch(),
+        c.pull('7.56.x'),
+        c.fetch_tags(),
+        c.tags(glob_pattern='7.56.*'),
+    ]
+
+
+def _assert_tag_pushed(git, result, tag):
+    assert result.exit_code == 0, result.output
+    assert git.method_calls[-2:] == [
+        c.tag(tag, message=tag),
+        c.push(tag),
+    ]
+    assert f'Create and push this tag: {tag}?' in result.output
+
+
+def test_wrong_branch(ddev, basic_git):
+    """
+    Given a branch that doesn't match the release branch pattern we should abort.
+    """
+    name = 'foo'
+    basic_git.current_branch.return_value = name
+
+    result = ddev('release', 'branch', 'tag')
+
+    assert result.exit_code == 1, result.output
+    assert rf'Invalid branch name: {name}. Branch name must match the pattern ^\d+\.\d+\.x$' in result.output
+    assert basic_git.method_calls == [c.current_branch()]
+
+
+def test_middle_of_release_next_rc(ddev, git):
+    """
+    We're in the middle of a release, some RCs are already done. We want to create the next RC.
+    """
+    result = ddev('release', 'branch', 'tag', input='\ny\n')
+
+    _assert_tag_pushed(git, result, '7.56.0-rc.12')
+    assert RC_NUMBER_PROMPT.format('12') in result.output
+
+
+@pytest.mark.parametrize(
+    'no_confirm',
+    [
+        pytest.param('n', id='explicit abort'),
+        pytest.param('', id='abort by default'),
+        pytest.param('x', id='abort on any other input'),
+    ],
+)
+@pytest.mark.parametrize('rc_num', ['3', '10'])
+@pytest.mark.parametrize('last_rc', [11, 12])
+def test_do_not_confirm_non_sequential_rc(ddev, git, rc_num, no_confirm, last_rc):
+    """
+    We're in the middle of a release, some RCs are already done. User wants to create the next RC.
+
+    However the user asks to create an RC that's less than the latest RC number.
+    This is unusual, so we give a warning and ask user to confirm. If they don't, we abort.
+
+    Important: we are not overwriting an existing RC tag.
+    """
+
+    git.tags.return_value.append(f'7.56.0-rc.{last_rc}')
+    result = ddev('release', 'branch', 'tag', input=f'{rc_num}\n{no_confirm}\n')
+
+    assert RC_NUMBER_PROMPT.format(str(last_rc + 1)) in result.output
+    assert (
+        '!!! WARNING !!!\n'
+        f'The latest RC is {last_rc}. You are about to go back in time by creating an RC with a number less than that. '
+        'Are you sure? [y/N]'
+    ) in result.output
+    assert result.exit_code == 1, result.output
+    assert NO_CONFIRMATION_SO_ABORT in result.output
+
+
+@pytest.mark.parametrize('rc_num', ['3', '10'])
+def test_confirm_non_sequential_rc(ddev, git, rc_num):
+    """
+    We're in the middle of a release, some RCs are already done. User wants to create the next RC.
+
+    However the user asks to create an RC that's less than the latest RC number.
+    This is unusual, so we give a warning and ask user to confirm. If they do, we create the tag.
+
+    Important: we are not overwriting an existing RC tag.
+    """
+    result = ddev('release', 'branch', 'tag', input=f'{rc_num}\ny\ny\n')
+
+    assert RC_NUMBER_PROMPT.format('12') in result.output
+    assert (
+        '!!! WARNING !!!\n'
+        'The latest RC is 11. You are about to go back in time by creating an RC with a number less than that. '
+        'Are you sure? [y/N]'
+    ) in result.output
+    _assert_tag_pushed(git, result, f'7.56.0-rc.{rc_num}')
+
+
+@pytest.mark.parametrize('rc_num', ['1', '5', '11'])
+def test_abort_if_rc_tag_exists(ddev, git, rc_num):
+    """
+    We're in the middle of a release, some RCs are already done. User wants to create the next RC.
+
+    However the user asks to create an RC for which we already have a tag. This requires special git flags to clobber
+    the local and remote tags. To keep our logic simple we give up here and leave a hint how the user can proceed.
+    """
+
+    result = ddev('release', 'branch', 'tag', input=f'{rc_num}\ny\n')
+
+    assert result.exit_code == 1, result.output
+    assert RC_NUMBER_PROMPT.format('12') in result.output
+    assert f'Tag 7.56.0-rc.{rc_num} already exists. Switch to git to overwrite it.' in result.output
+
+
+def test_abort_if_tag_less_than_one(ddev, git):
+    """
+    RC numbers less than 1 don't make any sense, so we abort if we get one.
+    """
+    result = ddev('release', 'branch', 'tag', input='0\ny\n')
+
+    assert RC_NUMBER_PROMPT.format('12') in result.output
+    assert result.exit_code == 1, result.output
+    assert 'RC number must be at least 1.' in result.output
+
+
+@pytest.mark.parametrize(
+    'no_confirm',
+    [
+        pytest.param('n', id='explicit abort'),
+        pytest.param('', id='abort by default'),
+        pytest.param('x', id='abort on any other input'),
+    ],
+)
+def test_abort_valid_rc(ddev, git, no_confirm):
+    """
+    The RC is fine but we don't confirm in the end, so we abort.
+    """
+    git.tags.return_value = []
+
+    result = ddev('release', 'branch', 'tag', input='\n{no_confirm}\n')
+
+    assert RC_NUMBER_PROMPT.format('1') in result.output
+    assert result.exit_code == 1, result.output
+    assert NO_CONFIRMATION_SO_ABORT in result.output
+
+
+@pytest.mark.parametrize(
+    'rc_num_input, rc_num',
+    [
+        pytest.param('', '1', id='implicit sequential'),
+        pytest.param('', '1', id='explicit sequential'),
+        pytest.param('2', '2', id='explicit non-sequential'),
+    ],
+)
+@pytest.mark.parametrize('tags, patch', [([], '0'), (EXAMPLE_TAGS + ['7.56.0'], '1')])
+def test_first_rc(ddev, git, rc_num_input, rc_num, tags, patch):
+    """
+    First RC for a new release.
+
+    We support starting with a number other than 1, though that's very unlikely to happen in practice.
+    """
+    git.tags.return_value = tags
+
+    result = ddev('release', 'branch', 'tag', input=f'{rc_num_input}\ny\n')
+
+    _assert_tag_pushed(git, result, f'7.56.{patch}-rc.{rc_num}')
+    assert RC_NUMBER_PROMPT.format('1') in result.output
+
+
+@pytest.mark.parametrize(
+    'latest_final_tag, expected_new_final_tag',
+    [
+        pytest.param('', '7.56.0', id='no final tag yet'),
+        pytest.param('7.56.0', '7.56.1', id='final tag present, so we are making a bugfix release'),
+    ],
+)
+def test_final(ddev, git, latest_final_tag, expected_new_final_tag):
+    """
+    Create final release tag.
+    """
+    git.tags.return_value.append(latest_final_tag)
+    result = ddev('release', 'branch', 'tag', '--final', input='y\n')
+
+    _assert_tag_pushed(git, result, expected_new_final_tag)
+
+
+# TODO: test for adding RCs for a bugfix release