Fixes #37455 - Create Katello push repositories as needed at container push time, handle non-auth validation. #10995
Conversation
|
The errors thrown are a little messy. They are all generic errors and they tend to get truncated on the podman side. I'm happy to improve them if you think it's a good idea. |
|
Containers with tags such as |
There was a problem hiding this comment.
I saw this error on my first push:
container_repository_name: ["for repository 'fedora-bootc' is not unique and cannot be created in 'Library'. Its Container Repository Name (default_organization-buttermilk_biscuits-fedora-bootc) conflicts with an existing repository. Consider changing the Lifecycle Environment's Registry Name Pattern to something more specific."]
I do have another container repo, but its label is centos-bootc_fedora-bootc
The error didn't throw until later. Was repo creation attempted twice maybe?
I pushed:
podman push 75eca4dcedfb `hostname`/default_organization/buttermilk_biscuits/fedora-bootc
Well shoot. Podman calls the endpoint that calls Another option is that the |
|
I deleted all of my repos and tried again, ended up hitting: |
app/controllers/katello/api/registry/registry_proxies_controller.rb
Outdated
Show resolved
Hide resolved
app/controllers/katello/api/registry/registry_proxies_controller.rb
Outdated
Show resolved
Hide resolved
app/controllers/katello/api/registry/registry_proxies_controller.rb
Outdated
Show resolved
Hide resolved
app/controllers/katello/api/registry/registry_proxies_controller.rb
Outdated
Show resolved
Hide resolved
app/controllers/katello/api/registry/registry_proxies_controller.rb
Outdated
Show resolved
Hide resolved
|
I realize that we will definitely need some way to tell push repositories apart from normal ones. Relying on the version_href to say "container-push" in it could be a pretty good way to do so. When making content views, we'll need it to determine how contents should be copied. We'll also need it for deleting container push repositories, since it's the distribution that needs to be deleted rather than the repo itself. |
I missed this comment before. The tag is separate from the repository name -- it is tagging the manifest that the user is pushing up. Under the hood, if not tag is noted, podman will auto-assign latest to the manifest being pushed. I'm guessing in this case then that all pushed content is getting the latest tag. It's important that tags are taken in exactly as they're pushed. |
app/controllers/katello/api/registry/registry_proxies_controller.rb
Outdated
Show resolved
Hide resolved
|
I tested passing in custom tags and it works :) |
ianballou and I talked about this outside of the PR and we came to the conclusion that we are handling tags correctly at the moment. Tag info shouldn't need to be stored anywhere in the repo; handling everything with Pulp should be completely fine. |
app/controllers/katello/api/registry/registry_proxies_controller.rb
Outdated
Show resolved
Hide resolved
|
As we discussed in-person before, the main thing missing now is being able to index content. To do so, I'd recommend a strategy like:
def container_push_repositories_api
PulpContainerClient::RepositoriesContainerPushApi.new(api_client)
end
# ...
repo.update(version_href: ::Katello::Pulp3::Repository.api(SmartProxy.pulp_primary, ::Katello::Repository::DOCKER_TYPE).
container_push_repositories_api.list(name: container_repo_name_here).results.first.latest_version_href)
# ...(Determined via https://www.rubydoc.info/gems/pulp_container_client/2.20.0/PulpContainerClient/RepositoriesContainerPushApi)
Create the ::Katello::Pulp3::RepositoryReference that would normally happen during the Pulp 3 repo creation. This is skipped because Pulp is making the repo for us. # pulp_repo_href looks like /pulp/api/v3/repositories/container/container-push/018f9759-7d55-7b1e-82b0-f2f062ebd606/
# Note that there is no version -- this is just the repository reference, not a reference to any repository version
RepositoryReference.where(
root_repository_id: repo.root_id,
content_view_id: repo.content_view.id,
repository_href: pulp_api_response.pulp_href).create!
Bonus points: Using a similar method as above, also find the distribution and save it as a -> API: |
app/controllers/katello/api/registry/registry_proxies_controller.rb
Outdated
Show resolved
Hide resolved
|
@ianballou I really appreciate all the help with content indexing above (too big to quote). The newest update to the PR should fix the duplicate Pulp entry, set the root repo's |
|
Already can see one improvement: |
|
I wonder if this is starting to get more complicated than we need, especially since this capitalization scenario we're guarding against should be very rare. I'm trying to think if we could completely throw out the "by ID" style to ensure that Katello and Pulp are always aligned. |
|
Throwing out the ID method entirely will force the rare ambiguous org / product owner to recreate the org / product. I think the safer middle ground would be to check if a user is trying to push content to a repository with a different name / ID scheme and to tell them to destroy it first. It makes for a decent experience for the very rare ambiguous org/product user, and shouldn't over-complicate the code for the 99% use-case. |
There was a problem hiding this comment.
The error messages are looking better! One issue I noticed:
- Push an image by name/label
- Create another org with the same name
- Create a product in the new org with the same name as the product in the old org
- Push the same image in step (1) to the product in step (3) by ID
- The ID repo gets created in Pulp, but no repository is created in the new org/product.
It looks like the Root repository was created under the new org. They do have the proper product association as well. However, neither of them have any Repository records.
The erroneous Root repositories also don't have any RepositoryReferences.
The first repository pushed by name is all correct, it seems.
Edit: I was able to push a new repo via ID to the new org, but it doesn't seem to have the version_href or distribution references set up.
I think in my Pulp the arianna repository already existed from a prior push.
-> It looks like this didn't matter.
app/controllers/katello/api/registry/registry_proxies_controller.rb
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Things have changed a bit -- now I'm seeing:
Error: writing manifest: uploading manifest latest to centos9-katello-devel.manicotto.example.com/id/3/2/curl: blob upload unknown: Could not locate local uploaded repository for content indexing.
when trying to push the same container repository (curl) to a second org with the same name by ID.
However, if I push a brand new container repository, it seems to work just fine now.
|
I think the commit and title can be changed to a "Fixes" instead of "Refs" |
|
It looks like pulling the pushed content broke somewhere along the way: It seems Katello still thinks we're distributing the contents using the old container name format. However, I'd be okay with pushing this to another PR, since in our plan we did have distribution as a separate task. Up to you. I know this PR has been lingering a while! |
|
Pushing the same repo to multiple products seems to be failing, looks like the container repository name is being saved just as the name of the repository and not the full path with org and product: Looks like it's because, even though the relative_path is getting set, the That could be used perhaps instead of |
|
I think I may have fixed all the issues with duplicate org / product, try this patch: |
|
The container repository name was getting duplicated and throwing errors. |
qcjames53
changed the title
qcjames53
force-pushed
the
37455
branch
2 times, most recently
from
d4e8151
to
626a443
Compare
|
Looks like just a single test failure: |
|
I just updated the PR description and testing steps. I'll get right on the failing test, thanks. |
…r push time, handle non-auth validation. Co-authored-by: Ian Ballou <ianballou67@gmail.com>
|
The failing test should be fixed. I ran into a few issues with content indexing after making the changes in the most recent push so you may need to reset the db. |
There was a problem hiding this comment.
This is working well now! I've finished testing a bunch of scenarios, pushing and pulling content with different levels of ambiguity.
I noticed one error that could be improved:
vagrant@centos9-katello-devel ~/katello $ podman push 66640e9653de `hostname`/id/1/339/arianna-id:nachos
Getting image source signatures
Copying blob b7d21803af9e [--------------------------------------] 8.0b / 3.0MiB | 1.7 MiB/s
Error: writing blob: initiating layer upload to /v2/id/1/339/arianna-id/blobs/uploads/ in centos9-katello-devel.manicotto.example.com: StatusCode: 404, "{\"displayMessage\":\"Couldn't find Organization with...
which happens when pushing by-ID to an org that doesn't exist.
If it's obvious how to fix it, great. If not, we can leave this for later. I'm guessing the JSON returned for the error is just malformed.
| :bad_request | ||
| ) | ||
| end | ||
| @organization = Organization.find(org_id.to_i) |
There was a problem hiding this comment.
Here is where the error I mentioned before is originating. It seems that, when pushing by-ID, the organization will fail here rather than being caught elsewhere. It just means a more ugly error is returned to podman, but it's still a 404.
What are the changes introduced in this pull request?
labeloridConsiderations taken when implementing this change?
What are the testing steps for this pull request?