Skip to content

🧪⇪ Bump galaxy_ng to v4.9.2 in tests - #82396

Open
webknjaz wants to merge 4 commits into
ansible:develfrom
webknjaz:testing/integration-galaxy-ng-4.9.0
Open

webknjaz wants to merge 4 commits into
ansible:develfrom
webknjaz:testing/integration-galaxy-ng-4.9.0

Conversation

@webknjaz
webknjaz requested a review from sivel December 11, 2023 13:52
@webknjaz webknjaz self-assigned this Dec 11, 2023
@ansibot ansibot added test This PR relates to tests. needs_triage Needs a first human triage before being processed. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Dec 11, 2023
@webknjaz

webknjaz commented Dec 11, 2023 •

Copy link
Copy Markdown
Member Author

The ansible-galaxy-collection integration test is broken in the CI runs:

03:20 TASK [ansible-galaxy-collection : ensure we can download the published collection - pulp_v2] ***
03:21 fatal: [testhost]: FAILED! => {"changed": true, "cmd": ["ansible-galaxy", "collection", "install", "-s", "pulp_v2", "-p", "/tmp/ansible.mk27mu05.test/publish/pulp_v2", "ansible_test.my_collection==1.0.0", "-v"], "delta": "0:00:00.470485", "end": "2023-12-11 13:56:52.273680", "msg": "non-zero return code", "rc": 1, "start": "2023-12-11 13:56:51.803195", "stderr": "\u001b[1;35m[WARNING]: The specified \"api_version\" configuration for the galaxy server\u001b[0m\n\u001b[1;35m\"pulp_v2\" is not a public configuration, and may be removed at any time without\u001b[0m\n\u001b[1;35mwarning.\u001b[0m\n\u001b[1;35m[WARNING]: The specified collections path\u001b[0m\n\u001b[1;35m'/tmp/ansible.mk27mu05.test/publish/pulp_v2' is not part of the configured\u001b[0m\n\u001b[1;35mAnsible collections paths\u001b[0m\n\u001b[1;35m'/root/.ansible/collections:/usr/share/ansible/collections'. The installed\u001b[0m\n\u001b[1;35mcollection will not be picked up in an Ansible run, unless within a playbook-\u001b[0m\n\u001b[1;35madjacent collections directory.\u001b[0m\n\u001b[0;31mERROR! Error when getting the collection info for ansible_test.my_collection from pulp_v2 (http://galaxy-pulp/pulp_ansible/galaxy/primary/api/) (HTTP Code: 500, Message: Internal Server Error Code: Unknown)\u001b[0m", "stderr_lines": ["\u001b[1;35m[WARNING]: The specified \"api_version\" configuration for the galaxy server\u001b[0m", "\u001b[1;35m\"pulp_v2\" is not a public configuration, and may be removed at any time without\u001b[0m", "\u001b[1;35mwarning.\u001b[0m", "\u001b[1;35m[WARNING]: The specified collections path\u001b[0m", "\u001b[1;35m'/tmp/ansible.mk27mu05.test/publish/pulp_v2' is not part of the configured\u001b[0m", "\u001b[1;35mAnsible collections paths\u001b[0m", "\u001b[1;35m'/root/.ansible/collections:/usr/share/ansible/collections'. The installed\u001b[0m", "\u001b[1;35mcollection will not be picked up in an Ansible run, unless within a playbook-\u001b[0m", "\u001b[1;35madjacent collections directory.\u001b[0m", "\u001b[0;31mERROR! Error when getting the collection info for ansible_test.my_collection from pulp_v2 (http://galaxy-pulp/pulp_ansible/galaxy/primary/api/) (HTTP Code: 500, Message: Internal Server Error Code: Unknown)\u001b[0m"], "stdout": "\u001b[0;34mUsing /tmp/ansible.mk27mu05.test/galaxy/ansible.cfg as config file\u001b[0m\nStarting galaxy collection install process\nProcess install dependency map", "stdout_lines": ["\u001b[0;34mUsing /tmp/ansible.mk27mu05.test/galaxy/ansible.cfg as config file\u001b[0m", "Starting galaxy collection install process", "Process install dependency map"]}

The unescaped stderr:

[WARNING]: The specified "api_version" configuration for the galaxy server
"pulp_v2" is not a public configuration, and may be removed at any time without
warning.
[WARNING]: The specified collections path
'/tmp/ansible.mk27mu05.test/publish/pulp_v2' is not part of the configured
Ansible collections paths
'/root/.ansible/collections:/usr/share/ansible/collections'. The installed
collection will not be picked up in an Ansible run, unless within a playbook-
adjacent collections directory.
ERROR! Error when getting the collection info for ansible_test.my_collection from pulp_v2 (http://galaxy-pulp/pulp_ansible/galaxy/primary/api/) (HTTP Code: 500, Message: Internal Server Error Code: Unknown)

@webknjaz webknjaz added the ci_verified Changes made in this PR are causing tests to fail. label Dec 11, 2023
@webknjaz

Copy link
Copy Markdown
Member Author

Sticking a sleep 60 doesn't fix it, so the problem with the new Galaxy NG is deeper.

cc @sivel FYI

@sivel

sivel commented Dec 11, 2023

Copy link
Copy Markdown
Member

Seems like the only way to know for sure what is failing, is to attach to the container logs for the galaxy container, and see the traceback causing the 500 ISE and then go from there.

@webknjaz

Copy link
Copy Markdown
Member Author

Yeah, that's what I thought.

@webknjaz

Copy link
Copy Markdown
Member Author

I attached and saw it streaming things like

pulp [a0feea53400f4988b47590376543557d]: django.request:ERROR: Internal Server Error: /api/galaxy/v3/plugin/ansible/content/primary/collections/index/
Traceback (most recent call last):
  File "/usr/local/lib/python3.8/site-packages/django/core/handlers/exception.py", line 55, in inner
    response = get_response(request)
  File "/usr/local/lib/python3.8/site-packages/django/core/handlers/base.py", line 197, in _get_response
    response = wrapped_callback(request, *callback_args, **callback_kwargs)
  File "/usr/local/lib/python3.8/site-packages/django/views/decorators/csrf.py", line 56, in wrapper_view
    return view_func(*args, **kwargs)
  File "/usr/local/lib/python3.8/site-packages/rest_framework/viewsets.py", line 125, in view
    return self.dispatch(request, *args, **kwargs)
  File "/usr/local/lib/python3.8/site-packages/rest_framework/views.py", line 509, in dispatch
    response = self.handle_exception(exc)
  File "/usr/local/lib/python3.8/site-packages/rest_framework/views.py", line 469, in handle_exception
    self.raise_uncaught_exception(exc)
  File "/usr/local/lib/python3.8/site-packages/rest_framework/views.py", line 480, in raise_uncaught_exception
    raise exc
  File "/usr/local/lib/python3.8/site-packages/rest_framework/views.py", line 497, in dispatch
    self.initial(request, *args, **kwargs)
  File "/usr/local/lib/python3.8/site-packages/rest_framework/views.py", line 415, in initial
    self.check_permissions(request)
  File "/usr/local/lib/python3.8/site-packages/rest_framework/views.py", line 332, in check_permissions
    if not permission.has_permission(request, self):
  File "/usr/local/lib/python3.8/site-packages/rest_access_policy/access_policy.py", line 69, in has_permission
    allowed = self._evaluate_statements(statements, request, view, action)
  File "/usr/local/lib/python3.8/site-packages/rest_access_policy/access_policy.py", line 113, in _evaluate_statements
    matched = self._get_statements_matching_conditions(
  File "/usr/local/lib/python3.8/site-packages/rest_access_policy/access_policy.py", line 262, in _get_statements_matching_conditions
    passed = self._check_condition(condition, request, view, action)
  File "/usr/local/lib/python3.8/site-packages/rest_access_policy/access_policy.py", line 288, in _check_condition
    result = method(request, view, action)
  File "/usr/local/lib/python3.8/site-packages/galaxy_ng/app/access_control/access_policy.py", line 253, in v3_can_view_repo_content
    distro = ansible_models.AnsibleDistribution.objects.get(base_path=path)
  File "/usr/local/lib/python3.8/site-packages/django/db/models/manager.py", line 87, in manager_method
    return getattr(self.get_queryset(), name)(*args, **kwargs)
  File "/usr/local/lib/python3.8/site-packages/django/db/models/query.py", line 637, in get
    raise self.model.DoesNotExist(
pulp_ansible.app.models.AnsibleDistribution.DoesNotExist: AnsibleDistribution matching query does not exist.
pulp [a0feea53400f4988b47590376543557d]: 127.0.0.1 - admin [11/Dec/2023:18:14:34 +0000] "GET /api/galaxy/v3/plugin/ansible/content/primary/collections/index/?limit=100&offset=0 HTTP/1.0" 500 145 "-" "ansible-httpget"
pulp [f01fb13dcfa242b4b291212793ad06b6]: django.request:ERROR: Internal Server Error: /api/galaxy/v3/plugin/ansible/content/secondary/collections/index/
Traceback (most recent call last):
  File "/usr/local/lib/python3.8/site-packages/django/core/handlers/exception.py", line 55, in inner
    response = get_response(request)
  File "/usr/local/lib/python3.8/site-packages/django/core/handlers/base.py", line 197, in _get_response
    response = wrapped_callback(request, *callback_args, **callback_kwargs)
  File "/usr/local/lib/python3.8/site-packages/django/views/decorators/csrf.py", line 56, in wrapper_view
    return view_func(*args, **kwargs)
  File "/usr/local/lib/python3.8/site-packages/rest_framework/viewsets.py", line 125, in view
    return self.dispatch(request, *args, **kwargs)
  File "/usr/local/lib/python3.8/site-packages/rest_framework/views.py", line 509, in dispatch
    response = self.handle_exception(exc)
  File "/usr/local/lib/python3.8/site-packages/rest_framework/views.py", line 469, in handle_exception
    self.raise_uncaught_exception(exc)
  File "/usr/local/lib/python3.8/site-packages/rest_framework/views.py", line 480, in raise_uncaught_exception
    raise exc
  File "/usr/local/lib/python3.8/site-packages/rest_framework/views.py", line 497, in dispatch
    self.initial(request, *args, **kwargs)
  File "/usr/local/lib/python3.8/site-packages/rest_framework/views.py", line 415, in initial
    self.check_permissions(request)
  File "/usr/local/lib/python3.8/site-packages/rest_framework/views.py", line 332, in check_permissions
    if not permission.has_permission(request, self):
  File "/usr/local/lib/python3.8/site-packages/rest_access_policy/access_policy.py", line 69, in has_permission
    allowed = self._evaluate_statements(statements, request, view, action)
  File "/usr/local/lib/python3.8/site-packages/rest_access_policy/access_policy.py", line 113, in _evaluate_statements
    matched = self._get_statements_matching_conditions(
  File "/usr/local/lib/python3.8/site-packages/rest_access_policy/access_policy.py", line 262, in _get_statements_matching_conditions
    passed = self._check_condition(condition, request, view, action)
  File "/usr/local/lib/python3.8/site-packages/rest_access_policy/access_policy.py", line 288, in _check_condition
    result = method(request, view, action)
  File "/usr/local/lib/python3.8/site-packages/galaxy_ng/app/access_control/access_policy.py", line 253, in v3_can_view_repo_content
    distro = ansible_models.AnsibleDistribution.objects.get(base_path=path)
  File "/usr/local/lib/python3.8/site-packages/django/db/models/manager.py", line 87, in manager_method
    return getattr(self.get_queryset(), name)(*args, **kwargs)
  File "/usr/local/lib/python3.8/site-packages/django/db/models/query.py", line 637, in get
    raise self.model.DoesNotExist(
pulp_ansible.app.models.AnsibleDistribution.DoesNotExist: AnsibleDistribution matching query does not exist.

even before that download test...

@webknjaz

Copy link
Copy Markdown
Member Author

And this is the end of the log (I paused the test playbook after running that failing download command):

pulp [0c53ee324d834cfaa90d4d605c486e87]: 127.0.0.1 - admin [11/Dec/2023:18:14:50 +0000] "POST /api/galaxy/v3/auth/token/ HTTP/1.0" 200 52 "-" "ansible-httpget"
pulp [8e4c4fd15e4544329b7495a59bce908d]: 127.0.0.1 - admin [11/Dec/2023:18:15:34 +0000] "POST /pulp_ansible/galaxy/primary/api/v2/collections/ HTTP/1.0" 202 78 "-" "ansible-galaxy/2.17.0.dev0 (Linux; python:3.12.0)"
pulp [8e4c4fd15e4544329b7495a59bce908d]: pulpcore.tasking.tasks:INFO: Starting task 018c5a17-6f71-7930-a58b-e0f94b80e573
pulp [8e4c4fd15e4544329b7495a59bce908d]: pulp_ansible.app.tasks.collections:INFO: Processing collection from tmp/files/01/8c5a17-6f65-73e8-9649-33a56a764ef7
pulp [8e4c4fd15e4544329b7495a59bce908d]: pulp_ansible.app.tasks.utils:INFO: Reading MANIFEST.json from tmp/files/01/8c5a17-6f65-73e8-9649-33a56a764ef7
pulp [8e4c4fd15e4544329b7495a59bce908d]: pulp_ansible.app.tasks.utils:INFO: Reading FILES.json from tmp/files/01/8c5a17-6f65-73e8-9649-33a56a764ef7
/usr/lib64/python3.8/tarfile.py:2230: RuntimeWarning: The default behavior of tarfile extraction has been changed to disallow common exploits (including CVE-2007-4559). By default, absolute/parent paths are disallowed and some mode bits are cleared. See https://access.redhat.com/articles/7004769 for more details.
  warnings.warn(
pulp [6edadb4393614c8aa74faf59532b5b89]: 127.0.0.1 - admin [11/Dec/2023:18:15:35 +0000] "GET /pulp_ansible/galaxy/primary/api/v2/collection-imports/018c5a17-6f71-7930-a58b-e0f94b80e573/ HTTP/1.0" 200 649 "-" "ansible-galaxy/2.17.0.dev0 (Linux; python:3.12.0)"
pulp [8e4c4fd15e4544329b7495a59bce908d]: pulpcore.tasking.tasks:INFO: Task completed 018c5a17-6f71-7930-a58b-e0f94b80e573
pulp [565b91ac991b45ddbdeece69b4e8edaa]: 127.0.0.1 - admin [11/Dec/2023:18:15:37 +0000] "GET /pulp_ansible/galaxy/primary/api/v2/collection-imports/018c5a17-6f71-7930-a58b-e0f94b80e573/ HTTP/1.0" 200 676 "-" "ansible-galaxy/2.17.0.dev0 (Linux; python:3.12.0)"
pulp [be92b47f2f5c4616b9d34eb0a74694e7]: django.request:ERROR: Internal Server Error: /pulp_ansible/galaxy/primary/api/v2/collections/ansible_test/my_collection/
Traceback (most recent call last):
  File "/usr/local/lib/python3.8/site-packages/django/core/handlers/exception.py", line 55, in inner
    response = get_response(request)
  File "/usr/local/lib/python3.8/site-packages/django/core/handlers/base.py", line 197, in _get_response
    response = wrapped_callback(request, *callback_args, **callback_kwargs)
  File "/usr/local/lib/python3.8/site-packages/django/views/decorators/csrf.py", line 56, in wrapper_view
    return view_func(*args, **kwargs)
  File "/usr/local/lib/python3.8/site-packages/django/views/generic/base.py", line 104, in view
    return self.dispatch(request, *args, **kwargs)
  File "/usr/local/lib/python3.8/site-packages/rest_framework/views.py", line 509, in dispatch
    response = self.handle_exception(exc)
  File "/usr/local/lib/python3.8/site-packages/rest_framework/views.py", line 469, in handle_exception
    self.raise_uncaught_exception(exc)
  File "/usr/local/lib/python3.8/site-packages/rest_framework/views.py", line 480, in raise_uncaught_exception
    raise exc
  File "/usr/local/lib/python3.8/site-packages/rest_framework/views.py", line 506, in dispatch
    response = handler(request, *args, **kwargs)
  File "/usr/local/lib/python3.8/site-packages/pulp_ansible/app/galaxy/views.py", line 154, in get
    return response.Response(GalaxyCollectionSerializer(collection, context=context).data)
  File "/usr/local/lib/python3.8/site-packages/rest_framework/serializers.py", line 555, in data
    ret = super().data
  File "/usr/local/lib/python3.8/site-packages/rest_framework/serializers.py", line 253, in data
    self._data = self.to_representation(self.instance)
  File "/usr/local/lib/python3.8/site-packages/rest_framework/serializers.py", line 522, in to_representation
    ret[field.field_name] = field.to_representation(attribute)
  File "/usr/local/lib/python3.8/site-packages/rest_framework/fields.py", line 1838, in to_representation
    return method(value)
  File "/usr/local/lib/python3.8/site-packages/pulp_ansible/app/galaxy/serializers.py", line 116, in get_latest_version
    "version": rv.version,
AttributeError: 'NoneType' object has no attribute 'version'
pulp [be92b47f2f5c4616b9d34eb0a74694e7]: 127.0.0.1 - admin [11/Dec/2023:18:15:38 +0000] "GET /pulp_ansible/galaxy/primary/api/v2/collections/ansible_test/my_collection/ HTTP/1.0" 500 145 "-" "ansible-galaxy/2.17.0.dev0 (Linux; python:3.12.0)"

@sivel

sivel commented Dec 11, 2023

Copy link
Copy Markdown
Member

Some of those may be expected. See:

def get_repo_collections(repository, module):
collections_uri = module.params['galaxy_ng_server'] + 'v3/plugin/ansible/content/' + repository + '/collections/index/'
# status code 500 isn't really expected, an unhandled exception is causing this instead of a 404
# See https://issues.redhat.com/browse/AAH-2329
info = invoke_api(module, collections_uri + '?limit=100&offset=0', status_codes=[200, 500])
if not info:
return []
return [module.params['pulp_api'] + c['href'] for c in info['data']]

@webknjaz

Copy link
Copy Markdown
Member Author

jeez

@webknjaz

Copy link
Copy Markdown
Member Author

I think this is the relevant bit

  File "/usr/local/lib/python3.8/site-packages/pulp_ansible/app/galaxy/serializers.py", line 116, in get_latest_version
    "version": rv.version,
AttributeError: 'NoneType' object has no attribute 'version'

which suggests another bug in pulp_ansible again.

@bcoca bcoca removed the needs_triage Needs a first human triage before being processed. label Dec 12, 2023
@ansibot ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Dec 19, 2023
Comment thread test/lib/ansible_test/_internal/commands/integration/cloud/galaxy.py Outdated
@ansibot ansibot added the stale_pr This PR has not been pushed to for more than one year. label Jan 28, 2025
@webknjaz
webknjaz force-pushed the testing/integration-galaxy-ng-4.9.0 branch from 8a74e33 to ecf2426 Compare February 5, 2025 02:13
@ansibot ansibot removed stale_pr This PR has not been pushed to for more than one year. ci_verified Changes made in this PR are causing tests to fail. stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. labels Feb 5, 2025
@ansibot ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Feb 19, 2025
@webknjaz
webknjaz force-pushed the testing/integration-galaxy-ng-4.9.0 branch from ecf2426 to e1b0878 Compare February 27, 2025 22:55
@ansibot ansibot removed the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Feb 27, 2025
webknjaz pushed a commit to webknjaz/ansible that referenced this pull request Mar 3, 2025
This patch adjusts the metadata processing to swallow 404 and handle
other cases as a cache reset request.

Ref: ansible#82396 (comment)
webknjaz and others added 3 commits March 3, 2025 17:05
It updates the underlying `pulp_ansible` to v0.20.2, which fixes a
race condition that is occasionally observable in our CI when
provisioning the initial dummy collections for testing.

Refs:

  * https://github.com/ansible/galaxy_ng/releases/tag/4.9.0
  * ansible/galaxy_ng#1950
  * ansible/galaxy#3267
  * pulp/pulp_ansible#1571
  * pulp/pulp_ansible#1572
  * pulp/pulp_ansible#1595
  * pulp/pulp_ansible#1623
  * pulp/pulp_ansible#1624
Co-authored-by: Sloane Hertel <19572925+s-hertel@users.noreply.github.com>
This patch adjusts the metadata processing to swallow 404 and handle
other cases as a cache reset request.

Ref: ansible#82396 (comment)
@webknjaz
webknjaz force-pushed the testing/integration-galaxy-ng-4.9.0 branch from 9a9aba3 to cd32765 Compare March 3, 2025 16:05
@webknjaz webknjaz changed the title 🧪⇪ Bump galaxy_ng to v4.9.0 in tests 🧪⇪ Bump galaxy_ng to v4.9.2 in tests Mar 3, 2025
@webknjaz
webknjaz requested a review from s-hertel March 3, 2025 16:05
@webknjaz webknjaz moved this from 📑 Ready 👌 to 🫸In review🫷 in 📅 Procrastinating in public 😵‍💫 Mar 3, 2025
@ansibot ansibot removed the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. label Mar 3, 2025
Comment thread lib/ansible/galaxy/api.py
Comment on lines +856 to +867
if err.http_code == 404:
# No collection found, return an empty list to keep things consistent with the various APIs
return []
elif err.http_code != 500 or 'v3' in self.available_api_versions:
raise
# No collection found, return an empty list to keep things consistent with the various APIs
return []

# v2 appears to have a bug on some versions, where the metadata URL is an HTTP error 500
# since this URL is only used to handle the cache, proceed as if the cache needs to be refreshed.
modified_date = None
if '%s.%s' % (namespace, name) in modified_cache:
del modified_cache['%s.%s' % (namespace, name)]
self._set_cache()

@s-hertel s-hertel Mar 17, 2025 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@webknjaz Since v2 GalaxyAPI is deprecated and close to removal, I'm second-guessing this. Can we just remove the v2 tests early, instead of adding code to support the lack of fully functional v2 in 4.9.2?

Or, could we keep using an older galaxy ng for v2 coverage until 2.20 development starts and we can delete everything v2?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@s-hertel I think that coverage is a good reason to keep this. But also, I was looking forward to backporting this to reduce the CI flakiness. I feel like this might be okay to backport, but removals wouldn't be.

Should we merge+backport this and then delete whatever in follow-ups when 2.20 comes? The main value in this for me is the CI stability, which spans across several branches IIRC.

cc @mattclay would this be acceptable to backport for the sake of CI?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this is a bug fix for V2, it seems like a good idea to backport it. We should continue testing V2 for versions that support it. Once we branch for 2.20 we'll be able to rip out all the V2 code and tests.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If v2 was used by anything other than old galaxy and custom servers mimicking the v2 endpoint, I'd be more in favor of backporting this. This seems featureish for 3rd party galaxy servers, which should conform with https://old-galaxy.ansible.com v2 or GalaxyNG v3.

IMO we should not be adding more code for v2, unless it fixes issues with https://old-galaxy.ansible.com.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@s-hertel doesn't it fix issues with our own CI where it uses v2 in old branches? How else would we address that?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@webknjaz How will we know fixes on old branches are compatible with https://old-galaxy.ansible.com/ if we no longer test against a compatible v2? Specifying the api version is internal, so if someone's using a recent version of GalaxyNG, they'll be using v3 via the public API, not v2. We should keep backwards compat with old galaxy since that's how people use v2.

Or, could we keep using an older galaxy ng for v2 coverage until 2.20 development starts and we can delete everything v2?

Have you taken a look at which GalaxyNG version it actually breaks in and why? Can the image can be updated to a lesser extent to pull in some of the linked fixes and still work with v2?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dunno, I haven't looked deeper. I was originally hoping for just an image update, but these changes grew on top, as you and sivel suggested.

@ansibot ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Mar 17, 2025
@webknjaz
webknjaz requested a review from mattclay March 18, 2025 15:39

@mattclay mattclay left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this looks good overall. Since we're changing code, not just tests, this should be turned into a bugfix and have a changelog fragment.

Comment thread lib/ansible/galaxy/api.py
Comment on lines +856 to +867
if err.http_code == 404:
# No collection found, return an empty list to keep things consistent with the various APIs
return []
elif err.http_code != 500 or 'v3' in self.available_api_versions:
raise
# No collection found, return an empty list to keep things consistent with the various APIs
return []

# v2 appears to have a bug on some versions, where the metadata URL is an HTTP error 500
# since this URL is only used to handle the cache, proceed as if the cache needs to be refreshed.
modified_date = None
if '%s.%s' % (namespace, name) in modified_cache:
del modified_cache['%s.%s' % (namespace, name)]
self._set_cache()

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this is a bug fix for V2, it seems like a good idea to backport it. We should continue testing V2 for versions that support it. Once we branch for 2.20 we'll be able to rip out all the V2 code and tests.

@mattclay

Copy link
Copy Markdown
Member

@webknjaz Have you confirmed that the issue this is trying to fix is still occurring in the devel branch? #84763 included a work-around for one of the galaxy issues we were seeing while working on DT.

@webknjaz

Copy link
Copy Markdown
Member Author

@mattclay no, I haven't. I guess I missed that PR. It looks like it might've fixed it. I'll keep an eye out for this test failures.
Are you going to backport that into older branches?

@webknjaz

Copy link
Copy Markdown
Member Author

@mattclay

Copy link
Copy Markdown
Member

@webknjaz I wasn't planning on backporting the entire PR, but it makes sense to backport the galaxy test fixes. Can you open backports for just those fixes?

@webknjaz

webknjaz commented Apr 1, 2025

Copy link
Copy Markdown
Member Author

Looks like devel still has problems with these tests: https://dev.azure.com/ansible/ansible/_build/results?buildId=141717&view=logs&j=02bf57c0-d919-5e2d-1c94-7b4ef705afdf&t=a9b57ab5-2f0b-5a77-0352-06fbabf684da&l=5867 (it's from @s-hertel's PR #84911). Should we still consider this PR, then?

@webknjaz

Copy link
Copy Markdown
Member Author

@webknjaz webknjaz moved this from 🫸In review🫷 to 🚧 In progress 🚧 in 📅 Procrastinating in public 😵‍💫 Apr 15, 2025
@mattclay

Copy link
Copy Markdown
Member

@webknjaz Thanks for monitoring this issue. Since the random sleep only improves the situation, rather than eliminating the it completely, it occurred to me that we could simply disable the parallel publishing in the test. If the parallel publishing is the source of this issue, then #85000 should resolve it.

@webknjaz

Copy link
Copy Markdown
Member Author

@mattclay thanks! It doesn't seem to have helped: #85000 (comment). Pretty sure that it is failing on cleanup, not on publishing.

@mattclay

Copy link
Copy Markdown
Member

Yeah, this looks like a different error though -- not related to the publishing problems we were seeing.

@webknjaz

Copy link
Copy Markdown
Member Author

Yeah, this looks like a different error though -- not related to the publishing problems we were seeing.

@mattclay perhaps we were talking about different things, then. I checked a few log links in my previous comments above, and all of them exhibit the same race condition in the pulp reset task.

Here's one from today's nightly: https://dev.azure.com/ansible/ansible/_build/results?buildId=143192&view=logs&j=02bf57c0-d919-5e2d-1c94-7b4ef705afdf&t=a9b57ab5-2f0b-5a77-0352-06fbabf684da&l=5473

@mattclay

Copy link
Copy Markdown
Member

@webknjaz I think part of the problem is that there are different errors for the initial run and the retry. To make things easier to discuss and troubleshoot, I'm disabling the retry on that integration test: #85013

@mattclay

Copy link
Copy Markdown
Member

Now that the test isn't retrying, and isn't publishing in parallel, we're now seeing the publishing step time out instead:

https://dev.azure.com/ansible/ansible/_build/results?buildId=143289&view=logs&jobId=70eeb688-fc21-5491-4420-b60b604f62ec&j=70eeb688-fc21-5491-4420-b60b604f62ec&t=d6f17d19-0bb3-5e63-8427-e2ae23c55414

This timeout was occurring previously, but it was more difficult to notice because the test was being retried, and the retry failure was due to concurrency issues -- most likely because the previous publish was still running.

I've opened #85016 to increase the publish timeout, to see if that's sufficient to get the tests to pass consistently.

@webknjaz

Copy link
Copy Markdown
Member Author

I've opened #85016 to increase the publish timeout, to see if that's sufficient to get the tests to pass consistently.

Looks like that did help. And you're right: I was confusing normal runs vs. reruns.

@webknjaz

Copy link
Copy Markdown
Member Author

Here stable-2.18 hit that unique_is_highest bug earlier today:

{
  "rc": 1,
  "stderr": "\u001b[1;35m[WARNING]: The specified \"api_version\" configuration for the galaxy server\u001b[0m\n\u001b[1;35m\"pulp_v2\" is not a public configuration, and may be removed at any time without\u001b[0m\n\u001b[1;35mwarning.\u001b[0m\n\u001b[1;35m[WARNING]: Galaxy import warning message: No changelog found. Add a\u001b[0m\n\u001b[1;35mCHANGELOG.rst, CHANGELOG.md, or changelogs/changelog.yaml file.\u001b[0m\n\u001b[0;31mERROR! Galaxy import process failed: duplicate key value violates unique constraint \"unique_is_highest\"\u001b[0m\n\u001b[0;31mDETAIL:  Key (collection_id, is_highest)=(839aeb95-f5e3-4a2e-a853-1a73a8e4767c, t) already exists.\u001b[0m\n\u001b[0;31m (Code: UNKNOWN)\u001b[0m\n",
  "stdout": "Publishing collection artifact '/root/.ansible/tmp/ansible-moduletmp-1745565311.5552013-pjw5iaof/namespace1-name1-0.0.7/namespace1-name1-0.0.7.tar.gz' to galaxy_ng [http://galaxy-pulp/api/galaxy/content/primary/\nCollection](http://galaxy-pulp/api/galaxy/content/primary//nCollection) has been published to the Galaxy server galaxy_ng [http://galaxy-pulp/api/galaxy/content/primary/\nWaiting](http://galaxy-pulp/api/galaxy/content/primary//nWaiting) until Galaxy import task http://galaxy-pulp/api/galaxy/content/primary/v3/imports/collections/2b6dc0bb-d568-4d8f-bd9e-3d23598fa3ab/ has completed\n"
}

(https://dev.azure.com/ansible/ansible/_build/results?buildId=144050&view=logs&j=70eeb688-fc21-5491-4420-b60b604f62ec&t=d6f17d19-0bb3-5e63-8427-e2ae23c55414&l=1014)

@webknjaz

Copy link
Copy Markdown
Member Author

@ansibot ansibot added the stale_pr This PR has not been pushed to for more than one year. label Mar 7, 2026
@ansibot ansibot added the needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html label Mar 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. stale_pr This PR has not been pushed to for more than one year. test This PR relates to tests.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants