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

Fixes #29421 - add pulp3 debian support #8632

Merged
merged 1 commit into from Nov 2, 2020

Conversation

m-bucher
Copy link
Contributor

@m-bucher m-bucher commented Mar 26, 2020

First goal is to implement
create, update, sync and delete debian repositories on pulp3

Probably not yet ready for final review, but definitely open for comments and critics 😉

TODOs

  • verify if we want need repository verification in the first implementation
  • verify if we want need repository signing in the first implementation
    For now we will assign hardcoded SigningService with name katello_deb_sign, if it exists.
  • fix https://pulp.plan.io/issues/6897 in pulp_deb to get the tests running
    fix still needs to be cherry-picked into pulp_deb 2.4

@theforeman-bot
Copy link

Issues: #29421

katello.gemspec Outdated Show resolved Hide resolved
@jlsherrill
Copy link
Member

@m-bucher curious how close this is for testing, a quick glace the code looks good! Do you need any help? You look like you've figured most of it out, but if you need any help i'd be happy to do a video chat to hammer out any questions.

@m-bucher
Copy link
Contributor Author

@m-bucher curious how close this is for testing, a quick glace the code looks good! Do you need any help? You look like you've figured most of it out, but if you need any help i'd be happy to do a video chat to hammer out any questions.

Currently I am looking into writing more tests, but due to my setup I am not really able to record correct VCRs for it.
Code wise I think it is pretty done for the first implementation.
A point I am still not sure about is the how-this-gets-enabled by foreman-installer part.

Having a video chat would be great, maybe @quba42 would like to join for this as he is more familiar with the pulp_deb implementation and how far we are from a release there.

@m-bucher
Copy link
Contributor Author

m-bucher commented Oct 9, 2020

@jlsherrill I finally managed to find time to get the tests to run through 😄
After some minor catch-up I had to do with the latest master and some basic testing I believe it is best to get this merged.

What I have not tested, yet was subscribing a host and checking whether it can access the repositories. I might have to add a redirect in Apache similar to that one: https://github.com/theforeman/puppet-foreman_proxy_content/blob/master/templates/pulpcore-isos-apache.conf.erb

@jlsherrill
Copy link
Member

getting an error when trying to upload a deb to a repo:

ArgumentError

wrong number of arguments (given 2, expected 0..1)

---
- "/home/vagrant/foreman/.vendor/ruby/2.5.0/gems/pulp_deb_client-2.6.1/lib/pulp_deb_client/api/content_packages_api.rb:30:in
  `create'"
- "/home/vagrant/git/katello/app/lib/actions/pulp3/repository/save_artifact.rb:14:in
  `invoke_external_task'"
- "/home/vagrant/foreman/.vendor/ruby/2.5.0/gems/dynflow-1.4.7/lib/dynflow/action/polling.rb:84:in
  `initiate_external_action'"
- "/home/vagrant/foreman/.vendor/ruby/2.5.0/gems/dynflow-1.4.7/lib/dynflow/action/polling.rb:19:in
  `run'"
- "/home/vagrant/foreman/.vendor/ruby/2.5.0/gems/dynflow-1.4.7/lib/dynflow/action/cancellable.rb:14:in
  `run'"
- "/home/vagrant/git/katello/app/lib/actions/pulp3/abstract_async_task.rb:10:in `run'"

@jlsherrill
Copy link
Member

everything else looks fine and works fine. I'll start building the client gem rpms

@jlsherrill
Copy link
Member

opened theforeman/foreman-packaging#5880 to bump the version of that client

@m-bucher
Copy link
Contributor Author

m-bucher commented Oct 27, 2020

getting an error when trying to upload a deb to a repo:

Looks like you found another difference in the pulp_deb_client-gem. There the ContentPackagesApi create only takes an opts-hash, but rpm and file client-bindings gem takes the realtive_path as a separate parameter.

So we might have to fix this in pulp_deb: https://pulp.plan.io/issues/7756

@jlsherrill
Copy link
Member

@m-bucher we could also make this logic more customizable per content type, by creating a 'create' method in the content_backend_service layer (i.e. services/katello/pulp3/deb.rb/rpm.rb/etc...) that could handle the differences. We could put a generic one in pulp3/pulp_content_unit.rb

@m-bucher
Copy link
Contributor Author

@jlsherrill see fixup-commit for implementation of a translation-method for package-upload.
It worked, however, I got a pulp-error during pulpcore.app.tasks.upload.commit:

  error:
    traceback: |2
        File "/usr/lib/python3.6/site-packages/rq/worker.py", line 883, in perform_job
          rv = job.perform()
        File "/usr/lib/python3.6/site-packages/rq/job.py", line 657, in perform
          self._result = self._execute()
        File "/usr/lib/python3.6/site-packages/rq/job.py", line 663, in _execute
          return self.func(*self.args, **self.kwargs)
        File "/usr/local/lib/python3.6/site-packages/pulpcore/app/tasks/upload.py", line 28, in commit
          serializer.is_valid(raise_exception=True)
        File "/usr/lib/python3.6/site-packages/rest_framework/serializers.py", line 243, in is_valid
          raise ValidationError(self.errors)
    description: "{'non_field_errors': [ErrorDetail(string='sha512 checksum must be
      unique.', code='unique')]}"

@m-bucher
Copy link
Contributor Author

The error seems to be that I already uploaded the file once successfully into pulpcore, but it was not added to the repository due to the previous failure in the call of the create-method.

Is there a process to clean this up without having to remove stuff manually from pulpcore?

Should the first upload-task be resumable, rather than ending in failed with a dangling upload in pulpcore?

@jlsherrill
Copy link
Member

@m-bucher to be clear, the problem happens if something happens during the upload process and the artifact gets uploaded but not 'saved'? and then pulp doesn't allow it to be uploaded again? That does seem like a legit bug, hopefully we can look up the artifact by its href and just 'resume' the process. Would you mind filing an issue about that?

I'll re-test!

@m-bucher
Copy link
Contributor Author

@jlsherrill thanks, created https://projects.theforeman.org/issues/31202

Any idea why the test, failed? seems like part of them was not even started 🤔

@jlsherrill
Copy link
Member

thanks @m-bucher !

we've been seeing this error in jenkins over the past couple days. I've still got no idea what the cause is :/ It seems like once a pr hits it, it continues to hit it. But probably not enough data to confirm that for sure, lets get some more:

[test katello]

@m-bucher
Copy link
Contributor Author

[test katello]

create, update, sync and delete debian repositories on pulp3

Add tests

Add signing-service

Add verification of deb-repos in pulp3
@m-bucher
Copy link
Contributor Author

So I got the test to run and fixed another issue along the way 😁
I do not think The katello:reimport errors are related.

However, this one might be:

Failure:
Katello::Service::Applicability::ApplicableContentHelperTest#test_applicable_differences_adds_and_removes_no_rpm_ids [/home/jenkins/workspace/katello-pr-test/test/services/katello/applicability/applicable_content_helper_test.rb:105]
Minitest::Assertion: Expected: [[], []]
  Actual: [[1046165992], []]

Any idea on how to fix this @jlsherrill ?

@jlsherrill
Copy link
Member

I think that failure might be a transient one, it should go away on next test. The reimport ones are happening on other prs, so its possible something changed in foreman or tasks. We'll investigate

@jlsherrill
Copy link
Member

[test katello]

Copy link
Member

@jlsherrill jlsherrill left a comment

Choose a reason for hiding this comment

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

ACK! But don't forget about incremental update still needed some work to work under deb :)

@jlsherrill jlsherrill merged commit cbd77ad into Katello:master Nov 2, 2020
@m-bucher m-bucher deleted the add_pulp3_deb_support branch June 14, 2021 13:10
m-bucher added a commit to ATIX-AG/katello that referenced this pull request Jun 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants