Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refresh tokens when needed, not on every download #406

Merged
merged 1 commit into from
Nov 4, 2020

Conversation

bmbouter
Copy link
Member

@bmbouter bmbouter commented Nov 2, 2020

closes #7643

@pulpbot
Copy link
Member

pulpbot commented Nov 2, 2020

Attached issue: https://pulp.plan.io/issues/7643

@bmbouter bmbouter force-pushed the fix-token-refresh-more-efficiently branch 2 times, most recently from b790e1b to 92b7f69 Compare November 3, 2020 20:58
Comment on lines +152 to +157
if AUTH_TOKEN:
return AUTH_TOKEN
async with TOKEN_LOCK:
if AUTH_TOKEN:
return AUTH_TOKEN
Copy link
Member

Choose a reason for hiding this comment

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

how the refresh happens even with these ifs?

Copy link
Member

Choose a reason for hiding this comment

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

actually, I was not giving enough time for the tokens to expire:

Nov 03 21:51:00 pulp3-source-fedora31.localhost.example.com rq[35999]: Backing off _run_with_token_refresh_and_401_retry(...) for 0.0s (aiohttp.client_exceptions.ClientResponseError: 401, message='Unauthorized', url=URL('https://cloud.redhat.com/api/automation-hub/v3/collections/check_point/mgmt/versions/?offset=0&limit=100'))
Nov 03 21:51:00 pulp3-source-fedora31.localhost.example.com rq[35999]: pulp: backoff:INFO: Backing off _run_with_token_refresh_and_401_retry(...) for 0.0s (aiohttp.client_exceptions.ClientResponseError: 401, message='Unauthorized', url=URL('https://cloud.redhat.com/api/automation-hub/v3/collections/check_point/mgmt/versions/?offset=0&limit=100'))
Nov 03 21:51:00 pulp3-source-fedora31.localhost.example.com rq[35999]: Backing off _run_with_token_refresh_and_401_retry(...) for 0.0s (aiohttp.client_exceptions.ClientResponseError: 401, message='Unauthorized', url=URL('https://cloud.redhat.com/api/automation-hub/v3/collections/check_point/mgmt/versions/?offset=0&limit=100'))
Nov 03 21:51:00 pulp3-source-fedora31.localhost.example.com rq[35999]: pulp: backoff:INFO: Backing off _run_with_token_refresh_and_401_retry(...) for 0.0s (aiohttp.client_exceptions.ClientResponseError: 401, message='Unauthorized', url=URL('https://cloud.redhat.com/api/automation-hub/v3/collections/check_point/mgmt/versions/?offset=0&limit=100'))
Nov 03 21:51:01 pulp3-source-fedora31.localhost.example.com rq[35999]: Backing off _run_with_token_refresh_and_401_retry(...) for 0.0s (aiohttp.client_exceptions.ClientResponseError: 401, message='Unauthorized', url=URL('https://cloud.redhat.com/api/automation-hub/v3/collections/check_point/mgmt/versions/?offset=0&limit=100'))
Nov 03 21:51:01 pulp3-source-fedora31.localhost.example.com rq[35999]: pulp: backoff:INFO: Backing off _run_with_token_refresh_and_401_retry(...) for 0.0s (aiohttp.client_exceptions.ClientResponseError: 401, message='Unauthorized', url=URL('https://cloud.redhat.com/api/automation-hub/v3/collections/check_point/mgmt/versions/?offset=0&limit=100'))
Nov 03 21:51:01 pulp3-source-fedora31.localhost.example.com rq[35999]: Backing off _run_with_token_refresh_and_401_retry(...) for 0.0s (aiohttp.client_exceptions.ClientResponseError: 401, message='Unauthorized', url=URL('https://cloud.redhat.com/api/automation-hub/v3/collections/check_point/mgmt/versions/?offset=0&limit=100'))
Nov 03 21:51:01 pulp3-source-fedora31.localhost.example.com rq[35999]: pulp: backoff:INFO: Backing off _run_with_token_refresh_and_401_retry(...) for 0.0s (aiohttp.client_exceptions.ClientResponseError: 401, message='Unauthorized', url=URL('https://cloud.redhat.com/api/automation-hub/v3/collections/check_point/mgmt/versions/?offset=0&limit=100'))
Nov 03 21:51:01 pulp3-source-fedora31.localhost.example.com rq[35999]: Backing off _run_with_token_refresh_and_401_retry(...) for 0.0s (aiohttp.client_exceptions.ClientResponseError: 401, message='Unauthorized', url=URL('https://cloud.redhat.com/api/automation-hub/v3/collections/check_point/mgmt/versions/?offset=0&limit=100'))
Nov 03 21:51:01 pulp3-source-fedora31.localhost.example.com rq[35999]: pulp: backoff:INFO: Backing off _run_with_token_refresh_and_401_retry(...) for 0.0s (aiohttp.client_exceptions.ClientResponseError: 401, message='Unauthorized', url=URL('https://cloud.redhat.com/api/automation-hub/v3/collections/check_point/mgmt/versions/?offset=0&limit=100'))
Nov 03 21:51:01 pulp3-source-fedora31.localhost.example.com rq[35999]: Backing off _run_with_token_refresh_and_401_retry(...) for 0.0s (aiohttp.client_exceptions.ClientResponseError: 401, message='Unauthorized', url=URL('https://cloud.redhat.com/api/automation-hub/v3/collections/check_point/mgmt/versions/?offset=0&limit=100'))
Nov 03 21:51:01 pulp3-source-fedora31.localhost.example.com rq[35999]: pulp: backoff:INFO: Backing off _run_with_token_refresh_and_401_retry(...) for 0.0s (aiohttp.client_exceptions.ClientResponseError: 401, message='Unauthorized', url=URL('https://cloud.redhat.com/api/automation-hub/v3/collections/check_point/mgmt/versions/?offset=0&limit=100'))
Nov 03 21:51:01 pulp3-source-fedora31.localhost.example.com gunicorn[36003]: 127.0.0.1 - admin [03/Nov/2020:21:51:01 +0000] "GET /pulp/api/v3/tasks/279b9126-02c6-4f88-a35a-829123583bca/ HTTP/1.1" 200 1237 "-" "OpenAPI-Generator//python"
Nov 03 21:51:02 pulp3-source-fedora31.localhost.example.com rq[35999]: Backing off _run_with_token_refresh_and_401_retry(...) for 0.0s (aiohttp.client_exceptions.ClientResponseError: 401, message='Unauthorized', url=URL('https://cloud.redhat.com/api/automation-hub/v3/collections/check_point/mgmt/versions/?offset=0&limit=100'))
.
.
.
Nov 03 21:53:50 pulp3-source-fedora31.localhost.example.com rq[35999]: Backing off _run_with_token_refresh_and_401_retry(...) for 0.0s (aiohttp.client_exceptions.ClientResponseError: 401, message='Unauthorized', url=URL('https://cloud.redhat.com/api/automation-hub/v3/collections/check_point/mgmt/versions/?offset=0&limit=100'))
Nov 03 21:53:50 pulp3-source-fedora31.localhost.example.com rq[35999]: pulp: backoff:INFO: Backing off _run_with_token_refresh_and_401_retry(...) for 0.0s (aiohttp.client_exceptions.ClientResponseError: 401, message='Unauthorized', url=URL('https://cloud.redhat.com/api/automation-hub/v3/collections/check_point/mgmt/versions/?offset=0&limit=100'))
Nov 03 21:53:50 pulp3-source-fedora31.localhost.example.com rq[35999]: Backing off _run_with_token_refresh_and_401_retry(...) for 0.0s (aiohttp.client_exceptions.ClientResponseError: 401, message='Unauthorized', url=URL('https://cloud.redhat.com/api/automation-hub/v3/collections/check_point/mgmt/versions/?offset=0&limit=100'))
Nov 03 21:53:50 pulp3-source-fedora31.localhost.example.com rq[35999]: pulp: backoff:INFO: Backing off _run_with_token_refresh_and_401_retry(...) for 0.0s (aiohttp.client_exceptions.ClientResponseError: 401, message='Unauthorized', url=URL('https://cloud.redhat.com/api/automation-hub/v3/collections/check_point/mgmt/versions/?offset=0&limit=100'))
Nov 03 21:53:50 pulp3-source-fedora31.localhost.example.com rq[35999]: Backing off _run_with_token_refresh_and_401_retry(...) for 0.0s (aiohttp.client_exceptions.ClientResponseError: 401, message='Unauthorized', url=URL('https://cloud.redhat.com/api/automation-hub/v3/collections/check_point/mgmt/versions/?offset=0&limit=100'))

diff:

diff --git a/pulp_ansible/app/tasks/collections.py b/pulp_ansible/app/tasks/collections.py
index 3e1db78..342ff81 100644
--- a/pulp_ansible/app/tasks/collections.py
+++ b/pulp_ansible/app/tasks/collections.py
@@ -4,6 +4,7 @@ from gettext import gettext as _
 import json
 import logging
 import math
+import time
 import tarfile
 from urllib.parse import urljoin, urlparse, urlunparse
 
@@ -408,6 +409,7 @@ class CollectionSyncFirstStage(Stage):
                 page_count = math.ceil(float(count) / float(PAGE_SIZE))
 
             for page in range(1, page_count + 1):
+                time.sleep(120)
                 url = await _get_url(page, versions_url)
                 downloader = remote.get_downloader(url=url)
                 not_done.add(downloader.run())
diff --git a/pulp_ansible/tests/functional/cli/test_collection_install.py b/pulp_ansible/tests/functional/cli/test_collection_install.py
index c40af10..c8bb3f6 100644
--- a/pulp_ansible/tests/functional/cli/test_collection_install.py
+++ b/pulp_ansible/tests/functional/cli/test_collection_install.py
@@ -94,7 +94,6 @@ class InstallCollectionTestCase(unittest.TestCase):
         """Test whether ansible-galaxy can install a Collection hosted by Pulp."""
         body = gen_ansible_remote(
             url=AUTOMATION_HUB_URL,
-            requirements_file="collections:\n  - ansible.posix",
             auth_url=AH_AUTH_URL,
             token=os.environ["AUTOMATION_HUB_TOKEN_AUTH"],
             tls_validation=False,

@pep8speaks
Copy link

pep8speaks commented Nov 4, 2020

Hello @bmbouter! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2020-11-04 21:15:23 UTC

@bmbouter bmbouter force-pushed the fix-token-refresh-more-efficiently branch 2 times, most recently from 0954bf1 to c9196c5 Compare November 4, 2020 20:12
try:
return await self._run_with_additional_headers(headers)
except ClientResponseError as exc:
if exc.status is 401:
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if exc.status is 401:
if exc.status == 401:

Copy link
Member Author

Choose a reason for hiding this comment

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

fixing in next push along w/ CI. Thanks!

@bmbouter bmbouter force-pushed the fix-token-refresh-more-efficiently branch from c9196c5 to 7faf2d3 Compare November 4, 2020 21:15
Copy link
Member

@fao89 fao89 left a comment

Choose a reason for hiding this comment

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

Thank you Brian!
Sorry for all this testing back and forth

@bmbouter bmbouter merged commit 57f5775 into pulp:master Nov 4, 2020
@bmbouter bmbouter deleted the fix-token-refresh-more-efficiently branch November 4, 2020 22:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants