Feature delete tag - #43
Merged
Merged
Conversation
A lot of improvement is required.
- Generated certificate with same info as old one, added `CN=distribution`. - This commit resolves the following error: `httpx.ConnectError: [SSL: CERTIFICATE_VERIFY_FAILED] certificate verify failed: Hostname mismatch, certificate is not valid for 'distribution'`.
This commit introduces the previously missing `remove_tag` method. Commit consistency has been an issue since `#19656252a0e936b45b5f3ed47a926dbd7ac28841`. I'll give my best to keep things consistent in the future.
Pytest error: "Failed: DID NOT RAISE `NotFoundException`". This test, though seemingly illogical and unnecessary, is currently failing and appears incorrect. It has no correlation with the feature I'm working on, and I've made no changes that should affect it. Curiously, this test previously passed, even with GitHub's validation. It was introduced in the `feature-repository-stars` branch, the last one merged into `develop`. Now, months later, running tests locally on an unchanged `develop` branch, only this test fails. Some troubleshooting steps taken: - Deleted all Docker volumes and restarted the project to clear any state. - Set `RUN_TESTS` to `true` in `compose.yaml` and ran tests with `docker-compose up backend --build`. Despite these efforts, the test continued to fail, compelling me to modify it. I was torn between adjusting or completely removing the test due to its questionable validity.
I was unsure about the syntax style. I was considering the following options:
- docker tag {image-name}[:tag] localhost:5000/{image-name}[:tag]
- docker tag <image-name>[:<tag>] localhost:5000/<image-name>[:<tag>]
- docker tag image-name[:tag] localhost:5000/image-name[:tag]
I've made some changes to how certificates are handled. Previously, users encountered a `FileNotFoundError: [Errno 2] No such file or directory` during tests because certificates weren't being unpacked by default. To make this more intuitive, I've now set unpacking certificates as the default operation. This should prevent the error from occurring. Optionally, users can regenerate certificates if they want or if needed. Additionally, I've switched the certificates archive file format from `.rar` to `.zip`. This change was necessary because `unrar` cannot be used on `alpine` images, and using `7-zip` to extract our `.rar` files caused numerous errors.
magley
requested changes
Jun 19, 2025
Owner
There was a problem hiding this comment.
Clever use of internal_router and external_router. I also appreciate that you delete manifests from Distribution first, before touching the database.
Note: I didn't run this code on my computer.
The suggestions I left are mostly comments, stylistic improvements. I see no logic errors, but part of that is due to me still not exactly knowing how tags, images, manifests etc. correlate and how they are stored by Distribution i.e. a lack of fundamentals.
Also removed some unused components.
magley
approved these changes
Jun 19, 2025
Vasilijez
added a commit
that referenced
this pull request
Jun 30, 2025
I have tested on the mirrored repository and got an error, even though I committed only a comment and made a pull request. The problem was caused by GitHub runners. The last test related to the previous PR #43 was on June 19th. The runner image used by GitHub Actions was ubuntu-24.04 (20250602.3.0) with Docker Compose v2.36.0. Today, GitHub Actions used the runner image ubuntu-24.04 (20250622.1.0) with Docker Compose v2.36.2. If I understand correctly, the following PR introduced the issue, as services listed in `depends_on` are no longer automatically built before running the backend service. Because of that, I first build all services, as it is the best way to ensure that we don’t encounter a problem when the backend is up with any of its `depends_on` services. docker/compose#12863 Locally, I didn’t have any issues with Docker Compose and running the exact same command, as the Docker Compose version on my PC is older and `build` services before `up`. Used Ubuntu images: https://github.com/actions/runner-images/blob/releases/ubuntu24/20250602/images/ubuntu/Ubuntu2404-Readme.md https://github.com/actions/runner-images/blob/releases/ubuntu24/20250622/images/ubuntu/Ubuntu2404-Readme.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #40.
I’ve implemented
RegistryClientas a single shared instance across the application’s lifecycle to improve efficiency and performance.I adjusted the certificates to support seamless communication between the backend and distribution containers.
I have chosen to run Distribution GC before the container starts, as it can be dangerous if Distribution is not in read-only mode. I’ve also provided a variable to enable or disable this behavior, in case running it each time is too frequent.
I decided to remove the
base imagelabel, as it is very difficult to map it to the underlying Distribution concepts and mechanisms — even though it initially seemed like a good idea. I'll try to explain why this is the case.A
base imagedoesn't truly exist. What we have is a manifest (an image) that links to all the used blobs (layers). Layers are shared among manifests (images), and mounting layers is a common and frequent action. Some layers may become unused when an image is deleted, while others remain in use because they are still mounted elsewhere. Therefore, it's very hard to define a single "base image" for any given image. I can provide more examples if needed.