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

Fix proxy statistics #4045

Merged
merged 1 commit into from Nov 20, 2023

Conversation

dimitar-kostadinov
Copy link
Contributor

If registry is configured as pull through cache the reported proxy statistics are incorrect.
Fixes #4044.

@milosgajdos
Copy link
Member

@dimitar-kostadinov mind rebasing?

@ialidzhikov
Copy link
Contributor

@milosgajdos PR is rebased. Can you have a look? Thank you in advance!

@milosgajdos
Copy link
Member

@ialidzhikov sorry, been swamped with work, aiming to have a proper look this week, but can't promise anything I'm afraid

@ialidzhikov
Copy link
Contributor

@milosgajdos , a friendly ping. We would like to proceed with the monitoring story on our side.

@milosgajdos
Copy link
Member

this needs a rebase @dimitar-kostadinov

@Jamstah
Copy link
Collaborator

Jamstah commented Oct 26, 2023

Again, no tests and not a well tested area, so I'd appreciate a quick "proof of test" message.

@dimitar-kostadinov dimitar-kostadinov force-pushed the fix/proxy_stats branch 2 times, most recently from 73e0529 to d2b369f Compare October 31, 2023 08:54
Signed-off-by: Dimitar Kostadinov <dimitar.kostadinov@sap.com>
@dimitar-kostadinov
Copy link
Contributor Author

Again, no tests and not a well tested area, so I'd appreciate a quick "proof of test" message.

Basic unit tests added.

Here are the manual steps for validation:

  • Checkout this PR gh pr checkout 4045
  • Start registry as pull through cache
# build
% make binaries
# prepare configuration
% mkdir -p ~/registry-test
% export REGISTRY_STORAGE_FILESYSTEM_ROOTDIRECTORY=~/registry-test
% export REGISTRY_PROXY_REMOTEURL=https://registry-1.docker.io
% export REGISTRY_HTTP_ADDR=":5500"
% export REGISTRY_HTTP_DEBUG_ADDR=":5501"
# start the registry as proxy cache to docker.io
% ./bin/registry serve cmd/registry/config-dev.yml
  • Configure docker to use registry mirror
{
  "registry-mirrors": [
    "http://test.registry.mirror:5500"
  ]
}
  • Update /etc/hosts file with following record
echo '127.0.0.1 test.registry.mirror' >> /etc/host
  • Pull nginx image % docker pull nginx
  • Get the expvars metrics
% curl http://test.registry.mirror:5501/debug/vars | jq '.registry.proxy'
{
  "blobs": {
    "Requests": 8,
    "Hits": 0,
    "Misses": 8,
    "BytesPulled": 67238076,
    "BytesPushed": 67238076
  },
  "manifests": {
    "Requests": 3,
    "Hits": 1,
    "Misses": 2,
    "BytesPulled": 3640,
    "BytesPushed": 5502
  }
}
  • Remove nginx from docker % docker rmi nginx and pull it again % docker pull nginx (from mirror)
  • Get the expvars metrics again
% curl http://test.registry.mirror:5501/debug/vars | jq '.registry.proxy'
{
  "blobs": {
    "Requests": 16,
    "Hits": 8,
    "Misses": 8,
    "BytesPulled": 67238076,
    "BytesPushed": 134476152
  },
  "manifests": {
    "Requests": 6,
    "Hits": 4,
    "Misses": 2,
    "BytesPulled": 3640,
    "BytesPushed": 11004
  }
}

@ialidzhikov
Copy link
Contributor

@Jamstah, can you check again?

@ialidzhikov
Copy link
Contributor

HI folks,

Can someone else have a look so that we proceed with the PR? Thank in advance!

Copy link
Collaborator

@Jamstah Jamstah left a comment

Choose a reason for hiding this comment

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

Thanks for the test results (and the rest of the PR!). Code looks good.

@milosgajdos milosgajdos merged commit 13fe08d into distribution:main Nov 20, 2023
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/proxy Related to registry as a pull-through cache
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Incorrect proxy statistics
4 participants