fixes #13479, 13483, 13504, 13508, 13509, 13524, 13505, 13550 - docker v2 - model/controller/routes/ui updates #5745

bbuckingham · 2016-01-31T13:05:20Z

NOTE: This PR requires Pulp 2.8

This PR contains several commits containing initial changes support docker v2. Please see commit list for details.

With docker v2, the docker 'image' will no longer be needed; however, removal of it will be handled separately.

beav · 2016-01-31T20:22:55Z

app/models/katello/content_view_version.rb

+      image_counts = repositories.archived.docker_type.map do |repo|
+        repo.docker_manifests.count
+      end
+      image_counts.sum


maybe replace image_counts with manifest_counts

bbuckingham · 2016-02-02T15:31:58Z

[test]

beav · 2016-02-02T21:53:21Z

app/lib/actions/katello/repository/clone_docker_content.rb

@@ -4,7 +4,7 @@ module Repository
      class CloneDockerContent < Actions::Base
        def plan(source_repo, target_repo)
          sequence do
-            plan_action(Pulp::Repository::CopyDockerImage,
+            plan_action(Pulp::Repository::CopyDockerManifest,


just confirming, copying manifests will copy blobs too?

@beav, good question! They are. I did check the output of the target repo after the copies completed to ensure that the counts for blob/manifest/tag match that of the source repo.

beav · 2016-02-02T22:00:14Z

@bbuckingham this looks good so far. I'm not clear on what index_db_docker_manifests does but it's just because I'm not familiar with that area of the code.

bbuckingham · 2016-02-03T12:33:33Z

@beav, Thanks for the review and comments. Regarding index_db_docker_manifests, it is a method that is intended to retrieve the units (manifests & tags) from the repository and then create corresponding records in the Katello database. This is used to support searching for content. The logic for this indexing is similar to the units on other repository types; however, for docker it was modified to address the changes that have come with pulp 2.8 and docker v2 (i.e. manifest as new content unit and the movement of tag from metadata to an actual content unit).

beav · 2016-02-04T19:54:27Z

app/lib/actions/pulp/repository/presenters/docker_presenter.rb

@@ -31,7 +31,7 @@ def humanized_details

            if content_started?(download_details)
              if items_total(download_details) > 0
-                ret << (_("New images: %{count}.") % {:count => count_summary})
+                ret << (_("New manifests: %{count}.") % {:count => count_summary})


The pulp code appears to key the download count off of blobs instead of manifests.

Thanks @beav! Updated to reference blobs.

beav · 2016-02-04T20:00:53Z

My main concern is making sure the rake task runs OK since I'm not sure of the ordering of the DB migration and the migration rake tasks.

APJ aside from that, the blob vs manifest thing on sync status can be addressed as its own issue if needed.

beav · 2016-02-05T14:11:25Z

Code changes look good, and tests are 💚!

ACK from me

komidore64 · 2016-02-05T14:42:40Z

db/migrate/20160129192548_add_docker_v2_schema.rb

+    add_foreign_key :katello_repository_docker_manifests, :katello_repositories,
+                    :column => :repository_id
+
+    add_foreign_key :katello_docker_tags, :katello_docker_manifests,


Should the down portion also account for these added indices and foreign keys? Or would those be removed since they're associated with the affected tables?

@komidore64, the deletion of the tables appears to also drop the foreign keys and indexes associated with them.

jlsherrill · 2016-02-05T15:14:54Z

As discussed on IRC, i'm not positive if this is handling the situation where pulp may either delete a tag or move it to a different manifest that I think may occur. example:

You sync a docker repo
Tag 'latest' is assigned to manifest A
Later you sync the docker repo again
Tag 'latest' is now assigned to manifest B

I believe pulp will either remove the old tag or 'move it' to the new manifest B. it looks like when we index we aren't handling this situation (assuming this actually is a situation).

I'm okay with it being handled as a followup PR (if it is indeed a problem). If so, please file an issue for it.

beav · 2016-02-05T15:47:35Z

@jlsherrill that seems like a valid situation, like 'latest' may point to Fedora 79 and then Fedora 80 in six months.

I'll create a redmine for it to set up the test scenario with a local registry and see what happens.

beav · 2016-02-05T16:02:52Z

@jlsherrill http://projects.theforeman.org/issues/13582

komidore64 · 2016-02-05T16:46:08Z

ACK from me.

looks good. 👍 🍻

jlsherrill · 2016-02-05T19:20:09Z

APJ for me

beav · 2016-02-05T19:23:35Z

[test]

…or tags This commit contains some initial changes to support the docker v2 model. This includes the introduction of a new model (manifests) and modifications to the existing docker tags model. With docker v2, the docker image model will no longer be needed; however, removal of it and any associations will be done separately from this commit.

…e for tags This commit contains some initial changes to support the docker v2 api controllers. This includes the introduction of a new controller (manifests) and modifications to the existing docker tags controller. With docker v2, the docker image APIs will no longer be needed; however, removal of them and any associations will be done separately from this commit.

…counts Small change to replace image with manifest counts in the Content -> Product UI.

…fest counts Small change to replace image with manifest counts in the Content -> Content Views UI.

This commit contains minor changes to support publishing and promoting content views using Pulp 2.8 with Docker v2.

…t v2 Manifests Update the UI to support manifests and remove images.

…ge Manifests This commit contains changes to convert the current 'Manage Images' functionality to support 'Manage Manifests' for docker v2. This includes the UI, model and action changes.

This commit contains several changes related to removing docker v1 image support. The changes are primarily to schema, model, controllers...etc; however, it includes any image code that remained after migrating katello to docker v2 (pulp2.8).

beav · 2016-02-05T22:19:43Z

🏈 🏈 🏈 🏈

fixes #13479, 13483, 13504, 13508, 13509, 13524, 13505, 13550 - docker v2 - model/controller/routes/ui updates

theforeman-bot added Needs testing Not yet reviewed labels Jan 31, 2016

bbuckingham changed the title ~~fixes #13479 - docker v2 - add model for manifests and update model for tags~~ fixes #13479, 13483 - docker v2 - model/controller/routes updates for manifests and tags Jan 31, 2016

bbuckingham force-pushed the issue-13479 branch from 56fb7bd to a71adc0 Compare January 31, 2016 19:39

beav reviewed Jan 31, 2016
View reviewed changes

bbuckingham force-pushed the issue-13479 branch 4 times, most recently from f6a89ac to 054517d Compare February 2, 2016 14:24

bbuckingham changed the title ~~fixes #13479, 13483 - docker v2 - model/controller/routes updates for manifests and tags~~ fixes #13479, 13483, 13504, 13508, 13509 - docker v2 - model/controller/routes/ui updates Feb 2, 2016

bbuckingham changed the title ~~fixes #13479, 13483, 13504, 13508, 13509 - docker v2 - model/controller/routes/ui updates~~ fixes #13479, 13483, 13504, 13508, 13509, 13524 - docker v2 - model/controller/routes/ui updates Feb 2, 2016

beav reviewed Feb 2, 2016
View reviewed changes

ehelms added the Waiting on contributor label Feb 3, 2016

bbuckingham force-pushed the issue-13479 branch from d910c80 to a917eb8 Compare February 3, 2016 12:23

theforeman-bot added Needs testing and removed Waiting on contributor labels Feb 3, 2016

bbuckingham force-pushed the issue-13479 branch from a917eb8 to 3c4b41a Compare February 3, 2016 12:26

bbuckingham changed the title ~~fixes #13479, 13483, 13504, 13508, 13509, 13524 - docker v2 - model/controller/routes/ui updates~~ fixes #13479, 13483, 13504, 13508, 13509, 13524, 13505 - docker v2 - model/controller/routes/ui updates Feb 3, 2016

bbuckingham changed the title ~~fixes #13479, 13483, 13504, 13508, 13509, 13524, 13505 - docker v2 - model/controller/routes/ui updates~~ fixes #13479, 13483, 13504, 13508, 13509, 13524, 13505, 13550 - docker v2 - model/controller/routes/ui updates Feb 4, 2016

beav reviewed Feb 4, 2016
View reviewed changes

ehelms added Waiting on contributor and removed Needs testing Not yet reviewed labels Feb 5, 2016

bbuckingham force-pushed the issue-13479 branch from 3ab9b8d to bba4e82 Compare February 5, 2016 02:23

theforeman-bot added Needs testing Needs re-review and removed Waiting on contributor labels Feb 5, 2016

bbuckingham force-pushed the issue-13479 branch from bba4e82 to e98d30a Compare February 5, 2016 12:32

komidore64 reviewed Feb 5, 2016
View reviewed changes

bbuckingham force-pushed the issue-13479 branch 2 times, most recently from 1bb6de6 to 895bec1 Compare February 5, 2016 16:43

bbuckingham added 8 commits February 5, 2016 16:27

fixes #13504 - docker v2: Content -> Product UI updates for manifest …

5d44152

…counts Small change to replace image with manifest counts in the Content -> Product UI.

fixes #13508 - docker v2: Content -> Content View UI updates for mani…

87ce091

…fest counts Small change to replace image with manifest counts in the Content -> Content Views UI.

fixes #13509 - docker v2: update content view publishing and promotion

e3acdfb

This commit contains minor changes to support publishing and promoting content views using Pulp 2.8 with Docker v2.

fixes #13524 - docker v2: Content -> Docker Tags UI updates to suppor…

ede4dd7

…t v2 Manifests Update the UI to support manifests and remove images.

fixes #13505 - docker v2: Content -> Products: update to support Mana…

3599e85

…ge Manifests This commit contains changes to convert the current 'Manage Images' functionality to support 'Manage Manifests' for docker v2. This includes the UI, model and action changes.

bbuckingham force-pushed the issue-13479 branch from 895bec1 to fec8d1f Compare February 5, 2016 21:28

bbuckingham added a commit that referenced this pull request Feb 5, 2016

Merge pull request #5745 from bbuckingham/issue-13479

f8509ae

fixes #13479, 13483, 13504, 13508, 13509, 13524, 13505, 13550 - docker v2 - model/controller/routes/ui updates

bbuckingham merged commit f8509ae into Katello:master Feb 5, 2016

bbuckingham deleted the issue-13479 branch February 5, 2016 22:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fixes #13479, 13483, 13504, 13508, 13509, 13524, 13505, 13550 - docker v2 - model/controller/routes/ui updates #5745

fixes #13479, 13483, 13504, 13508, 13509, 13524, 13505, 13550 - docker v2 - model/controller/routes/ui updates #5745

bbuckingham commented Jan 31, 2016

beav Jan 31, 2016

bbuckingham commented Feb 2, 2016

beav Feb 2, 2016

bbuckingham Feb 2, 2016

beav Feb 2, 2016

beav commented Feb 2, 2016

bbuckingham commented Feb 3, 2016

beav Feb 4, 2016

bbuckingham Feb 5, 2016

beav commented Feb 4, 2016

beav commented Feb 5, 2016

komidore64 Feb 5, 2016

bbuckingham Feb 5, 2016

jlsherrill commented Feb 5, 2016

beav commented Feb 5, 2016

beav commented Feb 5, 2016

komidore64 commented Feb 5, 2016

jlsherrill commented Feb 5, 2016

beav commented Feb 5, 2016

beav commented Feb 5, 2016

fixes #13479, 13483, 13504, 13508, 13509, 13524, 13505, 13550 - docker v2 - model/controller/routes/ui updates #5745

fixes #13479, 13483, 13504, 13508, 13509, 13524, 13505, 13550 - docker v2 - model/controller/routes/ui updates #5745

Conversation

bbuckingham commented Jan 31, 2016

Choose a reason for hiding this comment

bbuckingham commented Feb 2, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

beav commented Feb 2, 2016

bbuckingham commented Feb 3, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

beav commented Feb 4, 2016

beav commented Feb 5, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jlsherrill commented Feb 5, 2016

beav commented Feb 5, 2016

beav commented Feb 5, 2016

komidore64 commented Feb 5, 2016

jlsherrill commented Feb 5, 2016

beav commented Feb 5, 2016

beav commented Feb 5, 2016