Skip to content

Commit

Permalink
fixup: rename RA ignore-time setting and add test for second request
Browse files Browse the repository at this point in the history
  • Loading branch information
pedro-psb committed Dec 4, 2024
1 parent edd504c commit 4b4b195
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 9 deletions.
2 changes: 1 addition & 1 deletion pulpcore/app/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -298,7 +298,7 @@
# The time a RemoteArtifact will be ignored after failure.
# In on-demand, if a fetching content from a remote failed due to corrupt data,
# the corresponding RemoteArtifact will be ignored for that time (seconds).
FAILED_REMOTE_ARTIFACT_PROTECTION_TIME = 5 * 60 # 5 minutes
FAILED_REMOTE_ARTIFACT_COOLDOWN_TIME = 5 * 60 # 5 minutes

SPECTACULAR_SETTINGS = {
"SERVE_URLCONF": ROOT_URLCONF,
Expand Down
6 changes: 3 additions & 3 deletions pulpcore/content/handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -830,7 +830,7 @@ async def _stream_content_artifact(self, request, response, content_artifact):
ClientConnectionError,
)

protection_time = settings.FAILED_REMOTE_ARTIFACT_PROTECTION_TIME
protection_time = settings.FAILED_REMOTE_ARTIFACT_COOLDOWN_TIME
remote_artifacts = (
content_artifact.remoteartifact_set.select_related("remote")
.order_by_acs()
Expand Down Expand Up @@ -1151,14 +1151,14 @@ async def finalize():
await remote_artifact.asave()
await downloader.session.close()
close_tcp_connection(request.transport._sock)
FAILED_REMOTE_ARTIFACT_PROTECTION_TIME = settings.FAILED_REMOTE_ARTIFACT_PROTECTION_TIME
FAILED_REMOTE_ARTIFACT_COOLDOWN_TIME = settings.FAILED_REMOTE_ARTIFACT_COOLDOWN_TIME
raise RuntimeError(
f"Pulp tried streaming {remote_artifact.url!r} to "
"the client, but it failed checksum validation.\n\n"
"We can't recover from wrong data already sent so we are:\n"
"- Forcing the connection to close.\n"
"- Marking this Remote to be ignored for "
f"{FAILED_REMOTE_ARTIFACT_PROTECTION_TIME=}s.\n\n"
f"{FAILED_REMOTE_ARTIFACT_COOLDOWN_TIME=}s.\n\n"
"If the Remote is known to be fixed, try resyncing the associated repository.\n"
"If the Remote is known to be permanently corrupted, try removing "
"affected Pulp Remote, adding a good one and resyncing.\n"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,8 +115,13 @@ def test_remote_content_changed_with_on_demand(
):
"""
GIVEN a remote synced on demand with fileA (e.g, digest=123),
WHEN on the remote server, fileA changed its content (e.g, digest=456),
THEN retrieving fileA from the content app will cause a connection-close/incomplete-response.
AND the remote server, fileA changed its content (e.g, digest=456),
WHEN the client first requests that content
THEN the content app will start a response but close the connection before finishing
WHEN the client requests that content again (within the RA cooldown interval)
THEN the content app will return a 404
"""
# GIVEN
basic_manifest_path = write_3_iso_file_fixture_data_factory("basic")
Expand All @@ -128,17 +133,25 @@ def test_remote_content_changed_with_on_demand(
repo = file_bindings.RepositoriesFileApi.read(file_repo_with_auto_publish.pulp_href)
distribution = file_distribution_factory(repository=repo.pulp_href)
expected_file_list = list(get_files_in_manifest(remote.url))

# WHEN
write_3_iso_file_fixture_data_factory("basic", overwrite=True)

# THEN
get_url = urljoin(distribution.base_url, expected_file_list[0][0])

# WHEN (first request)
result = subprocess.run(["curl", "-v", get_url], stdout=subprocess.PIPE, stderr=subprocess.PIPE)

# THEN
assert result.returncode == 18
assert b"* Closing connection 0" in result.stderr
assert b"curl: (18) transfer closed with outstanding read data remaining" in result.stderr

# WHEN (second request)
result = subprocess.run(["curl", "-v", get_url], stdout=subprocess.PIPE, stderr=subprocess.PIPE)

# THEN
assert result.returncode == 0
assert b"< HTTP/1.1 404 Not Found" in result.stderr


@pytest.mark.parallel
def test_handling_remote_artifact_on_demand_streaming_failure(
Expand Down

0 comments on commit 4b4b195

Please sign in to comment.