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

WIP - Fixes #11612 - Enables use of Ostree Units #5838

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
16 changes: 16 additions & 0 deletions app/controllers/katello/api/v2/ostree_branches_controller.rb
@@ -0,0 +1,16 @@
module Katello
class Api::V2::OstreeBranchesController < Api::V2::ApiController
apipie_concern_subst(:a_resource => N_("an ostree branch"), :resource => "ostree_branch")
include Katello::Concerns::Api::V2::RepositoryContentController

private

def resource_class
OstreeBranch
end

def filter_by_content_view_filter(filter)
Copy link
Member

Choose a reason for hiding this comment

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

Can I make content view filters with ostree content?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

This is a nonsensical addition though is it not? You can't filter by content view filters for ostree branches and someone looking at this code would think you could.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bbuckingham @ehelms @jlsherrill @daviddavis What would be your suggested approach to fix this

  1. Add a "can_do_filters?" to every controller including this concern OR
  2. Add something in the RepositoryType plugin to say "can_do_filters?" and make the controller handle it.

Also this seems like a change that is arguably outside the scope of this PR which already seems to be doing a lot. I say this because I would have to change controllers that include this RepositoryContentController concern for either option.

Would you prefer a separate story or want me to change here ?

Copy link
Contributor

Choose a reason for hiding this comment

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

What about just raising an error here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Anyway created http://projects.theforeman.org/issues/14046 .. I 'll update the message in this PR if I fix it here

Copy link
Member

Choose a reason for hiding this comment

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

I'd probably re-word it to capture that the API is specifying content view filters as an API param on types that it does not apply to (

param :content_view_filter_id, :identifier, :desc => N_("content view filter identifier")
) more so than its a functional code issue. That being said, I think its a valid bug and I would just drop this function from your change since puppet modules suffer from the same fate.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So does DockerManifest ...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ehelms thing is even if filter id is not provided this method is called.

Copy link
Member

Choose a reason for hiding this comment

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

Does it?

resource_class.where(:uuid => filter.send("#{singular_resource_name}_rules").pluck(:uuid))
end
end
end
8 changes: 8 additions & 0 deletions app/models/katello/content_view_version.rb
Expand Up @@ -216,6 +216,10 @@ def package_count
Katello::Rpm.in_repositories(self.repositories.archived).count
end

def ostree_branch_count
ostree_branches.count
end

def docker_manifest_count
manifest_counts = repositories.archived.docker_type.map do |repo|
repo.docker_manifests.count
Expand All @@ -240,6 +244,10 @@ def errata(errata_type = nil)
errata
end

def ostree_branches
OstreeBranch.in_repositories(archived_repos).uniq
end

def docker_manifests
DockerManifest.in_repositories(archived_repos).uniq
end
Expand Down
21 changes: 21 additions & 0 deletions app/models/katello/glue/pulp/repo.rb
Expand Up @@ -477,6 +477,26 @@ def pulp_docker_manifest_ids
Katello.pulp_server.extensions.repository.docker_manifest_ids(self.pulp_id)
end

def pulp_ostree_branch_ids
Katello.pulp_server.extensions.repository.pulp_ostree_branch_ids(self.pulp_id)
end

def index_db_ostree_branches
ostree_branches_json.each do |ostree_branch_json|
branch = OstreeBranch.where(:uuid => ostree_branch_json[:_id]).first_or_create
branch.update_from_json(ostree_branch_json)
end
OstreeBranch.sync_repository_associations(self, pulp_ostree_branch_ids)
end

def ostree_branches_json
ostree_branches = []
pulp_ostree_branch_ids.each_slice(SETTINGS[:katello][:pulp][:bulk_load_size]) do |sub_list|
ostree_branches.concat(Katello.pulp_server.extensions.ostree_branches.find_all_by_unit_ids(sub_list))
end
ostree_branches
end

def sync_schedule(date_and_time)
if date_and_time
Katello.pulp_server.extensions.repository.create_or_update_schedule(self.pulp_id, importer_type, date_and_time)
Expand Down Expand Up @@ -786,6 +806,7 @@ def index_content
self.index_db_docker_manifests
self.index_db_puppet_modules
self.index_db_package_groups
self.index_db_ostree_branches if Katello::RepositoryTypeManager.find(Repository::OSTREE_TYPE).present?
self.import_distribution_data
true
end
Expand Down
30 changes: 21 additions & 9 deletions app/models/katello/ostree_branch.rb
@@ -1,14 +1,26 @@
module Katello
class OstreeBranch < Katello::Model
belongs_to :repository, :class_name => "Katello::Repository", :inverse_of => :ostree_branches
validates :name, presence: true, uniqueness: {scope: :repository_id}
validates :repository, :presence => true
validate :ensure_ostree_repository

def ensure_ostree_repository
unless self.repository.ostree?
errors.add(:base, _("Branch cannot be created since it does not belong to an RPM OSTree repository."))
end
include Concerns::PulpDatabaseUnit

has_many :repository_ostree_branches, :dependent => :destroy
has_many :repositories, :through => :repository_ostree_branches, :inverse_of => :ostree_branches

scoped_search :on => :name, :complete_value => true
scoped_search :on => :version, :complete_value => true
scoped_search :on => :commit, :complete_value => true
scoped_search :on => :uuid, :complete_value => true

CONTENT_TYPE = Pulp::DockerManifest::CONTENT_TYPE

def self.repository_association_class
RepositoryOstreeBranch
end

def update_from_json(json)
update_attributes(:name => json[:branch],
:version => json[:metadata][:version],
:commit => json[:commit]
)
end
end
end
7 changes: 6 additions & 1 deletion app/models/katello/repository.rb
Expand Up @@ -50,7 +50,8 @@ class Repository < Katello::Model

has_many :docker_tags, :dependent => :destroy, :class_name => "Katello::DockerTag"

has_many :ostree_branches, :class_name => "Katello::OstreeBranch", :dependent => :destroy
has_many :repository_ostree_branches, :class_name => "Katello::RepositoryOstreeBranch", :dependent => :destroy
has_many :ostree_branches, :through => :repository_ostree_branches

has_many :system_repositories, :class_name => "Katello::SystemRepository", :dependent => :destroy
has_many :systems, :through => :system_repositories
Expand Down Expand Up @@ -527,6 +528,8 @@ def remove_content(units)
self.rpms -= units
elsif puppet?
self.puppet_modules -= units
elsif ostree?
self.ostree_branches -= units
elsif docker?
remove_docker_content(units)
end
Expand Down Expand Up @@ -558,6 +561,8 @@ def removable_unit_association
self.docker_manifests
elsif puppet?
self.puppet_modules
elsif ostree?
self.ostree_branches
else
fail "Content type not supported for removal"
end
Expand Down
9 changes: 9 additions & 0 deletions app/models/katello/repository_ostree_branch.rb
@@ -0,0 +1,9 @@
module Katello
class RepositoryOstreeBranch < Katello::Model
self.include_root_in_json = false

# Do not use active record callbacks in this join model. Direct INSERTs and DELETEs are done
belongs_to :repository, :inverse_of => :repository_ostree_branches, :class_name => 'Katello::Repository'
belongs_to :ostree_branch, :inverse_of => :repository_ostree_branches
end
end
7 changes: 7 additions & 0 deletions app/services/katello/pulp/ostree_branch.rb
@@ -0,0 +1,7 @@
module Katello
module Pulp
class OstreeBranch < PulpContentUnit
CONTENT_TYPE = "ostree"
end
end
end
Expand Up @@ -12,6 +12,7 @@ attributes :package_count
attributes :puppet_module_count
attributes :docker_manifest_count
attributes :docker_tag_count
attributes :ostree_branch_count

node :errata_counts do |version|
partial('katello/api/v2/errata/counts', :object => Katello::RelationPresenter.new(version.errata))
Expand Down
7 changes: 7 additions & 0 deletions app/views/katello/api/v2/ostree_branches/index.json.rabl
@@ -0,0 +1,7 @@
object false

extends "katello/api/v2/common/metadata"

child @collection[:results] => :results do
extends 'katello/api/v2/ostree_branches/show'
end
4 changes: 4 additions & 0 deletions app/views/katello/api/v2/ostree_branches/show.json.rabl
@@ -0,0 +1,4 @@
object @resource

attributes :uuid, :id
attributes :name, :version, :commit
1 change: 1 addition & 0 deletions app/views/katello/api/v2/repositories/base.json.rabl
Expand Up @@ -13,6 +13,7 @@ end

node :content_counts do |repo|
{
:ostree_branches => repo.ostree_branches.count,
:docker_manifest => repo.docker_manifests.count,
:docker_tag => repo.docker_tags.count,
:rpm => repo.rpms.count,
Expand Down
1 change: 1 addition & 0 deletions config/routes/api/v2.rb
Expand Up @@ -108,6 +108,7 @@ class ActionDispatch::Routing::Mapper
end
end

api_resources :ostree_branches, :only => [:index, :show]
api_resources :docker_manifests, :only => [:index, :show]
api_resources :docker_tags, :only => [:index, :show] do
collection do
Expand Down
41 changes: 41 additions & 0 deletions db/migrate/20160301070319_add_version_ostree_branches.rb
@@ -0,0 +1,41 @@
class AddVersionOstreeBranches < ActiveRecord::Migration
def up
drop_table :katello_ostree_branches if ActiveRecord::Base.connection.table_exists? "katello_ostree_branches"

create_table :katello_ostree_branches do |t|
Copy link
Member

Choose a reason for hiding this comment

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

It is better to drop the table entirely and recreate it instead of migrating the current table?

Copy link
Contributor

Choose a reason for hiding this comment

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

We have to recreate the content by reindexing anyway so dropping the table and creating it seems easier than dropping all rows and migrating the table. Either works for me though.

Copy link
Member

Choose a reason for hiding this comment

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

Fair enough - given the way the down looks I'd wonder if a change migration would be cleaner? I do not claim to be a migration expert which is why I ask these questions :)

t.string :version, :limit => 255
t.string :name, :limit => 255
t.string :uuid, :null => false, :limit => 255
t.string :commit, :limit => 255
t.timestamps
end

create_table :katello_repository_ostree_branches do |t|
t.references :ostree_branch, :null => false
t.references :repository, :null => true
t.timestamps
end

add_index :katello_repository_ostree_branches, [:ostree_branch_id, :repository_id],
:name => :katello_repo_ostree_branch_repo_id, :unique => true

add_foreign_key :katello_repository_ostree_branches, :katello_repositories,
:column => :repository_id
end

def down
drop_table :katello_repository_ostree_branches
drop_table :katello_ostree_branches
Copy link
Contributor

Choose a reason for hiding this comment

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

You need to create the index and table from this down migration or else it will fail:

https://git.io/v29SS

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@daviddavis updated

create_table :katello_ostree_branches do |t|
t.string :name, :null => false, :limits => 255
t.references :repository, :null => false
t.timestamps
end

add_index :katello_ostree_branches, [:repository_id],
:name => :index_branches_on_repository

add_index :katello_ostree_branches, [:repository_id, :name],
:name => :katello_ostree_branches_repo_branch, :unique => true
end
end
Expand Up @@ -62,6 +62,10 @@
<div translate ng-if="version.docker_tag_count && version.docker_tag_count > 0">
{{ version.docker_tag_count }} Docker Tags
</div>
<div translate ng-if="version.ostree_branch_count && version.ostree_branch_count > 0">
{{ version.ostree_branch_count }} OSTree Units
</div>

</td>
<td bst-table-cell>
<div class="ellipsis-column">
Expand Down
@@ -0,0 +1,23 @@
(function () {
'use strict';

/**
* @ngdoc factory
* @name Bastion.ostree-branches.factory:OstreeBranch
*
* @description
* Provides a BastionResource for interacting with Ostree Branches
*/
function OstreeBranch(BastionResource) {
return BastionResource('/katello/api/v2/ostree_branches/:id',
{'id': '@id'}
);
}

angular
.module('Bastion.ostree-branches')
.factory('OstreeBranch', OstreeBranch);

OstreeBranch.$inject = ['BastionResource'];

})();
@@ -0,0 +1,14 @@
(function () {
'use strict';

/**
* @ngdoc module
* @name Bastion.ostree-branches
*
* @description
* Module for Ostree Branch related functionality.
*/
angular
.module('Bastion.ostree-branches', ['Bastion']);

})();
Expand Up @@ -118,14 +118,14 @@
</td>
<td bst-table-cell>
<span ng-show="repository.content_type == 'puppet'">
<a translate ui-sref="products.details.repositories.manage-content.puppet-modules({productId: product.id, repositoryId: repository.id})">
<a translate ui-sref="products.details.repositories.manage-content.puppet-modules({productId: product.id, repositoryId: repository.id})">
{{ repository.content_counts.puppet_module || 0 }} Puppet Modules
</a>
</span>

<span ng-show="repository.content_type == 'yum'">
<div>
<a translate ui-sref="products.details.repositories.manage-content.packages({productId: product.id, repositoryId: repository.id})">
<a translate ui-sref="products.details.repositories.manage-content.packages({productId: product.id, repositoryId: repository.id})">
{{ repository.content_counts.rpm || 0 }} Packages
</a>
</div>
Expand All @@ -149,6 +149,15 @@
</span>
</div>
</span>

<span ng-show="repository.content_type == 'ostree'">
<div>
<span translate>
{{ repository.content_counts.ostree || 0 }} OSTree Units
</span>
</div>
</span>

</td>
</tr>
</tbody>
Expand Down
Expand Up @@ -185,6 +185,12 @@ angular.module('Bastion.products').config(['$stateProvider', function ($statePro
permission: 'view_products',
collapsed: true,
templateUrl: 'repositories/details/views/repository-manage-docker-manifests.html'
})
.state('products.details.repositories.manage-content.ostree-branches', {
url: '/repositories/:repositoryId/content/ostree_branches',
permission: 'view_products',
collapsed: true,
templateUrl: 'repositories/details/views/repository-manage-ostree-branches.html'
});

$stateProvider.state('products.details.tasks', {
Expand Down
@@ -0,0 +1,52 @@
<span page-title ng-model="repository">{{ 'Manage Ostree Branchesfor Repository:' | translate }} {{ repository.name }}</span>

<a ui-sref="products.details.repositories.info({productId: product.id, repositoryId: repository.id})">
<i class="fa fa-angle-double-left"></i>
{{ "Back to Repository Details" | translate }}
</a>

<div data-extend-template="layouts/details-nutupane.html">

<div data-block="pane-loading"></div>

<div data-block="messages">
<div bst-alerts success-messages="successMessages" error-messages="errorMessages"></div>

<div bst-alert="success" ng-hide="generationTaskId === undefined">
<button type="button" class="close" ng-click="clearTaskId()">&times;</button>
<p translate>
Ostree Branch metadata generation has been initiated in the background. Click
<a ng-href="{{ taskUrl() }}">Here</a> to monitor the progress.
</p>
</div>
</div>

<div data-block="header">
<h3 translate>Ostree Branches in {{ repository.name }}</h3>
</div>

<div data-block="table">
<table class="table table-striped table-bordered" >

<thead>
<tr bst-table-head>
<th bst-table-column><span translate>Branch Name</span></th>
<th bst-table-column><span translate>Version</span></th>
</tr>
</thead>

<tbody>
<tr bst-table-row ng-repeat="item in detailsTable.rows" row-select="item">
<td bst-table-cell>
{{ item.name }}
</td>
<td bst-table-cell>
{{ item.version }}
</td>
</tr>
</tbody>

</table>
</div>

</div>