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

Add new test for "Uploading the same Content Unit twice" #81

Closed
4 tasks done
omaciel opened this issue Feb 3, 2016 · 18 comments
Closed
4 tasks done

Add new test for "Uploading the same Content Unit twice" #81

omaciel opened this issue Feb 3, 2016 · 18 comments

Comments

@omaciel
Copy link
Contributor

omaciel commented Feb 3, 2016

As per this issue, there is a new regression in Pulp 2.8 Beta when one tries to upload the same content (ie. rpms, puppet modules, etc) to an existing repository. We need a new automated test that does the following:

  • Create a new feed-less repository
  • Manually import a valid content type (i.e. RPM, etc)
    • Assert that the content type was successfully uploaded into the repository
  • Manually try to import the same exact content type to the same repository
    • Assert that a PulpCodedException is raised

We should make sure that all valid content types (rpms, puppet modules, docker images, etc) are used for this test to make sure that we get the same, expected behavior across the board.

  • docker v1
  • puppet
  • python
  • rpm

Not testable: docker v2 and ostree. See: here.

@omaciel
Copy link
Contributor Author

omaciel commented Feb 3, 2016

@peterlacko please consider this issue your top priority :)

@peterlacko
Copy link

Sure, will do!

@Ichimonji10
Copy link
Contributor

I'm self-assigning this.

@Ichimonji10
Copy link
Contributor

Branch master...Ichimonji10:duplicate contains a first draft of tests for this issue. They all succeed at the moment, and they're structured so that common code can be re-used between the different plugins.

@bmbouter, this set of tests targets Pulp #1406. In that issue, @mhrivnak suggests that a PulpCodedException should be raised. Is this exception visible to an end user? Or is that an internal implementation detail?

As things stand, the test case asserts that it is possible to upload the same content to a single repository twice in a row, and that no errors are reported. From my understanding of the issue, this is how Pulp 2.7 and 2.8 should both act. Am I missing something here?

@bmbouter
Copy link
Member

@Ichimonji10 Any PulpCodedException is designed to be user-facing. When we raise them, the Pulp httpd middleware should convert them to JSON and hand them back to the user in a structured way. If we're generically raising a 500 without any coded exception, I consider that a bug.

Regarding Pulp #1406, in 2.7 I expect no exception to be raised since 1406 was introduced in 2.8.0. I expect 2.8.0+ to not have an exception raised when issue 1406 is at MODIFIED.

If there are more questions let me know please post back. Thanks @Ichimonji10

@Ichimonji10
Copy link
Contributor

Let me make sure I have this right. In a nutshell, when duplicate content units are uploaded:

  • No exception error should be raised in returned by Pulp 2.x.
  • An exception error should be raised in returned by Pulp 3.x.

@bmbouter
Copy link
Member

@Ichimonji10 We haven't defined a behavior for Pulp 3.x

Let me put it this way. In all Pulp 2.x versions no exception should be raised when uploading a unit that is already known to Pulp. In 2.7 and (likely) all versions prior this has not been a problem. On 2.8.0 there is a regressions which is what Pulp 1406 is tracking specifically. Once 1406 is resolved 2.8.0 will not raise an error either which will make it consistent with earlier versions. Does that make sense? Post back questions!

@Ichimonji10
Copy link
Contributor

Yes, that makes sense. Thank you.

@omaciel
Copy link
Contributor Author

omaciel commented Feb 23, 2016

Hi @PulpQE/quality-engineers where do we stand on this issue?

@preethit
Copy link

@omaciel The issue is still being worked on as a blocker for 2.8. Its in an assigned state.
https://pulp.plan.io/issues/1406

@Ichimonji10
Copy link
Contributor

As of #121, Pulp Smash tests duplicate uploads for the docker plugin. The other plugins are untested.

@asmacdo
Copy link
Contributor

asmacdo commented Mar 7, 2016

Completion of this issue should include either a port of the relevant pulp-automation tests or the creation of a separate issue:
https://github.com/RedHatQE/pulp-automation/blob/master/tests/general_tests/test_21_upload_cud.py

@Ichimonji10
Copy link
Contributor

Ichimonji10 commented Apr 25, 2016

One can upload Docker v2 content to a Pulp repository, and as soon as we find a place to host a docker image tarball, we can implement that test. See: http://pulp-docker.readthedocs.org/en/latest/user-guide/recipes.html#upload-v1-images-to-pulp

One cannot upload Docker v2 content directly into a Pulp repository. See: http://pulp-docker.readthedocs.org/en/latest/user-guide/concepts.html#upload

There doesn't appear to be any mechanism for uploading OSTree content to a Pulp repository. See:

I'm updating the comment at the top of this thread appropriately.

@elyezer
Copy link
Contributor

elyezer commented Apr 27, 2016

Would be fedora people a good place to host the image tarball?

@Ichimonji10
Copy link
Contributor

Ichimonji10 commented Apr 27, 2016

I'd love it if we could do that. We're already hosting static RPMs there. Why not static tarballs too?

By the way, this seems highly related to #221. The process for generating the Docker tarball is:

docker pull busybox:latest
docker save busybox:latest > busybox:latest.tar

It's simple, but having those steps saved would be great from a reproducibility standpoint.

Maybe we should push these tools into a pulp-smash-fixtures repository? Both #221 and a docker script would bring in additional dependencies beyond what's listed in Pulp Smash → About → Scope and Limitations → Portability, and placing the scripts in a separate repository makes it clear that these scripts have their own set of dependencies. Also, it'd let the binary blobs brought in by the RPM generation script stay separate.

Ichimonji10 added a commit to pulp/pulp-fixtures that referenced this issue Apr 28, 2016
Create a new project, "Pulp Fixtures." This new project is a collection
of scripts for creating fixture data for a Pulp server. At present,
Docker and RPM fixture data can be generated with the following
commands, respectively:

    make docker-fixtures
    make rpm-fixtures

Each of these make targets requires that certain utilities be available
and usable. For example, `docker-fixtures` requires that the Docker
tools are installed, that the Docker daemon is running and that the
current user belongs to the `docker` group.

The advantage of collecting these scripts into a single repository is
that the scripts can be given a fairly common interface and set of
behaviours. In turn, this allows users of tools like Pulp Smash to more
easily reproduce a test environment.

The Docker scripts are written in response to
pulp/pulp-smash#81

The RPM scripts are adapted from
pulp/pulp-smash#221 (Thanks @bmbouter!)
@Ichimonji10
Copy link
Contributor

I've created a new repository, Pulp Fixtures. That repository has a make target for creating Docker fixtures, make docker. I make use of the fixture in my Docker duplicate upload tests, which can be found in my master...Ichimonji10:dup-up branch.

As soon as we can host the output of make docker somewhere, I'll be able to get that test updated accordingly and merged.

@elyezer
Copy link
Contributor

elyezer commented Apr 28, 2016

I will try to create a job that uploads the docker and RPM bits to fedorapeople.

@Ichimonji10
Copy link
Contributor

👍 Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants