Skip to content

Commit

Permalink
Use --exodus-commit=phase1 in pulp hook [RHELDST-20490]
Browse files Browse the repository at this point in the history
When this hook is active, the control flow is:

1. pubtools-exodus: create publish
2. rhsm-pulp: add content to publish
3. pubtools-exodus: commit publish

Problem: step 2 might succeed from Pulp's point of view, but then fail
at step 3 leaving the content not actually published to the CDN storage.
As Pulp believes the publishes were successful, it would then skip
publish of certain files at the next attempt, leading to missing
content.

The newly introduced concept of a phase1 commit should fix this by
allowing step 2 to request that all "phase 1 content" (e.g. RPMs, but
not repodata) are flushed to the CDN storage before proceeding. Go ahead
and start using that via the new related exodus-rsync argument.

While it would appear to make sense to do this at *every* publish, an
env var is added as an escape hatch to go back to the old behavior, just
in case something doesn't work as expected or upgrades are performed in
the wrong order.
  • Loading branch information
rohanpm committed Oct 3, 2023
1 parent 032079e commit d394238
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 2 deletions.
15 changes: 13 additions & 2 deletions pubtools/exodus/_hooks/pulp.py
Expand Up @@ -24,8 +24,8 @@ def __init__(self):
def pulp_repository_pre_publish(self, repository, options):
"""Invoked as the first step in publishing a Pulp repository.
This implementation adds to each config the --exodus-publish argument,
attaching the repository to an exodus-gw publish.
This implementation adds to each config the necessary arguments
to attach this repository's publish task to an exodus-gw publish.
Args:
repository (:class:`~pubtools.pulplib.Repository`):
Expand All @@ -48,6 +48,17 @@ def pulp_repository_pre_publish(self, repository, options):
list(options.rsync_extra_args) if options.rsync_extra_args else []
)
args.append("--exodus-publish=%s" % self.publish["id"])

# 2023-10: by default, the pulp hook should always use phase1 commit.
# But since the functionality is relatively new, this is provided as an
# escape hatch in case it would need to be disabled in some environments
# for unanticipated reasons.
#
# Consider deleting this conditional once the functionality is proven
# in production.
if os.getenv("EXODUS_PULP_HOOK_PHASE1_COMMIT", "1") == "1":
args.append("--exodus-commit=phase1")

return attr.evolve(options, rsync_extra_args=args)

@property
Expand Down
24 changes: 24 additions & 0 deletions tests/test_exodus_pulp_hooks.py
Expand Up @@ -34,6 +34,7 @@ def test_exodus_pulp_typical(
rsync_extra_args=[
"--existing-arg",
"--exodus-publish=497f6eca-6276-4993-bfeb-53cbbbba6f08",
"--exodus-commit=phase1",
],
)

Expand All @@ -55,6 +56,29 @@ def test_exodus_pulp_typical(
)


def test_exodus_pulp_phase1_disabled(
successful_gw_task, monkeypatch: pytest.MonkeyPatch
):
monkeypatch.setenv("EXODUS_PULP_HOOK_PHASE1_COMMIT", "0")

with task_context():
hook_rets = pm.hook.pulp_repository_pre_publish(
repository=None,
options=FakePublishOptions(rsync_extra_args=["--existing-arg"]),
)
hook_rets = [ret for ret in hook_rets if ret is not None]

# The pre-publish hook should've returned options with exodus-publish
# arg appended to existing rsync_extra_args, but this time it should
# NOT have added exodus-commit due to the above env var.
assert hook_rets[0] == FakePublishOptions(
rsync_extra_args=[
"--existing-arg",
"--exodus-publish=497f6eca-6276-4993-bfeb-53cbbbba6f08",
],
)


def test_exodus_pulp_no_publish(patch_env_vars, caplog):
caplog.set_level(logging.DEBUG, "pubtools-exodus")

Expand Down

0 comments on commit d394238

Please sign in to comment.