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

Set up a new test scenario for check pull limit-rate usage #1155

Closed
decko opened this issue Nov 28, 2022 · 7 comments
Closed

Set up a new test scenario for check pull limit-rate usage #1155

decko opened this issue Nov 28, 2022 · 7 comments

Comments

@decko
Copy link
Member

decko commented Nov 28, 2022

Since we were working on the #1133 we noticed that having a functional test wouldn't have been the best way to test it.
So, we're opening this task to set up a manual test to check if after we sync a repo for a second time it would pull the remote again, consuming the pull limit-rate of the Docker registry.

Steps:
1 - Setup a repo and a remote pointing to Docker Registry
2 - Sync it
3 - Use the Docker API to check the Pull Rate Limit (docs here)
4 - Try to sync it again
5 - Check the Docker API again to see if the ratelimit-remaining decreased since the last time. It should not change after 2nd sync.

Useful links:
1 - https://docs.docker.com/docker-hub/download-rate-limit/

@lubosmj
Copy link
Member

lubosmj commented Nov 28, 2022

We may also consider writing our custom server (a new dummy pytest fixture: https://github.com/pulp/pulp-smash/blob/ca6c35d1455f39e5e8a618bc7833624f9d7d5983/pulp_smash/pulp3/pytest_plugin/__init__.py#L196-L203, https://github.com/pulp/pulp_file/blob/3daf462fef83375c14680d71a94d816a5fcadea6/pulp_file/tests/functional/conftest.py#L299) that will act like a registry (returning pre-defined responses) - just to see whether we are sending more GET requests or not after syncing the content.

I was thinking about first syncing the content from a real registry, then updating the remote to start pointing to the dummy fixture and then we can initiate a new sync task. Probably too complicated?

@ipanova
Copy link
Member

ipanova commented Nov 30, 2022

Thanks for filing this test issue. The goal is to automate those steps and tranform them into a test. Steps look good, I made minor adjustments.

@lubosmj I believe it would be best to create such a fixture but it is not as simple as creating a dummy server, docker registry has it's own api with endpoints and response headers

@ipanova
Copy link
Member

ipanova commented Nov 30, 2022

@decko did you happen to perform those steps to ensure the PR you've got merged into master works as you expect? :)

@decko decko self-assigned this Dec 1, 2022
@pulpbot pulpbot moved this to In Progress in RH Pulp Kanban board Dec 1, 2022
@lubosmj lubosmj added the Tests label Dec 5, 2022
@decko
Copy link
Member Author

decko commented Dec 7, 2022

Checking the rate-limit consume before and after the fix for #1047.

The fix was to prevent the usage of methods that could consume the Registry rate-limit when it is not needed, in situations like where we already have the latest manifest or blobs.

Test methodology

The idea is to check the usage of ratelimit-remaining value between each sync operation.

Steps
  1. Create a new Docker Hub user. The service gives 200 pull-requests each 6 hours. If you consume all of them, you got them replenished completely after 6 hours.
  2. Create a new Pulp remote, like: pulp container remote create --name remote_alpine --url https://registry-1.docker.io --upstream-name library/alpine --username username --password password --include-tags '["latest", "3.17*"]' --policy on_demand
    Here we are pointing our remote to the Docker Registry, using an username and password, including the tags latest and any patch under 3.17 of the alpine image.
  3. Create a new Pulp repository, like: pulp container repository create --name repo_alpine --remote container:container:remote_alpine
  4. Obtain a new Docker Hub Token, to be used to verify the rate-limit value: ```TOKEN=$(curl --user 'username:password' "https://auth.docker.io/token?service=registry.docker.io&scope=repository:ratelimitpreview/test:pull" | jq -r .token)
  5. Check the rate-limit value using: curl --head -H "Authorization: Bearer $TOKEN" https://registry-1.docker.io/v2/ratelimitpreview/test/manifests/latest
    Verify the ratelimit-remaining value.
  6. Run the Sync between the remote and the repo: pulp container repository sync --name repo_alpine --remote container:container:remote_alpine
  7. After that, you can see that its gonna consume your ratelimit-remaining value.
  8. Any subsequent run of the sync operation should not consume your ratelimit-remaining value, unless the upstream have any new update.

Conclusion

After applying the fix for #1047, any sync operation should not consume the ratelimit-remaining value if there isn't any new upstream update.

@decko
Copy link
Member Author

decko commented Dec 7, 2022

We may also consider writing our custom server (a new dummy pytest fixture: https://github.com/pulp/pulp-smash/blob/ca6c35d1455f39e5e8a618bc7833624f9d7d5983/pulp_smash/pulp3/pytest_plugin/__init__.py#L196-L203, https://github.com/pulp/pulp_file/blob/3daf462fef83375c14680d71a94d816a5fcadea6/pulp_file/tests/functional/conftest.py#L299) that will act like a registry (returning pre-defined responses) - just to see whether we are sending more GET requests or not after syncing the content.

I was thinking about first syncing the content from a real registry, then updating the remote to start pointing to the dummy fixture and then we can initiate a new sync task. Probably too complicated?

  1. Makes sense a new ticket for it @lubosmj @ipanova??? I could open it.
  2. Use the dummy fixture and checking for the method usage seems pretty good to me. Way cheaper than checking any external registry for its usage.

@decko decko moved this from In Progress to Needs review in RH Pulp Kanban board Dec 26, 2022
@lubosmj
Copy link
Member

lubosmj commented Jan 3, 2023

We can open a ticket for that. However, it might be cumbersome to groom the ticket at the moment.

From the dummy server's perspective, we want to see that Pulp is not trying to download already synced manifests when we serve existing digests. The existing digests will be shared with the server via tests.

@lubosmj
Copy link
Member

lubosmj commented Jul 25, 2024

This is not something we care about right now.

@lubosmj lubosmj closed this as not planned Won't fix, can't repro, duplicate, stale Jul 25, 2024
@github-project-automation github-project-automation bot moved this from Todo to Done in Pulp Container Roadmap Jul 25, 2024
@pulpbot pulpbot moved this to Done in RH Pulp Kanban board Jul 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

No branches or pull requests

3 participants