Fixes #13191, #13192, #13193, #13195, #13196 - Remove pulp nodes #5750

johnpmitsch · 2016-02-01T18:56:34Z

This removes pulp nodes as they are not needed anymore in pulp 2.8

The pulp server on the capsule is now contacted directly

johnpmitsch · 2016-02-01T19:09:24Z

app/models/katello/content_view_puppet_environment.rb

+    end
+
+    def checksum_type
+      nil
    end


@jlsherrill I'm not too sure what I should do about these unprotected and checksum type methods, any suggestions?

we really need to turn unprotected into an attribute on the content view and look it up, but thats a feature we should add at some point.

as for Checksum_type, you may add a check to anything that calls checksum_type to only call it for yum repos

To clarify, leave unprotected as it is, and see if you can modify the code so it doesn't call checksum_type

jlsherrill · 2016-02-02T02:26:16Z

app/lib/actions/katello/capsule_content/create_or_update.rb

+
+        def create_repo_in_pulp(capsule_content, repository)
+          ueber_cert = ::Katello::Cert::Ca.ueber_cert(repository)
+          relative_path = repository.puppet? ? nil : repository.relative_path


actually relative_path is not nil if its a content_view_puppet_environment (in which case its the install_path) if i remember correctly:

https://github.com/Katello/katello/blob/master/app/lib/actions/katello/content_view_puppet_environment/create.rb#L18

johnpmitsch · 2016-02-03T13:46:03Z

app/services/katello/cert/ca.rb

+
+      def self.ssl_client_key
+        OpenSSL::PKey::RSA.new(File.open(SETTINGS[:katello][:ssl_client_key], 'r').read)
+      end


@jlsherrill I am trying to figure out how to organize this directory. I just stuck ssl_client_cert and ssl_client_key in here for now, but I think they should go in a separate module?

Maybe we should refactor the existing services/client/cert.rb to handle all of these things?

👍 There is a mixture of things in here that don't align to what I think of with 'Ca' as the module name. A bit of bikeshedding but worth it.

jlsherrill · 2016-02-03T18:21:33Z

app/lib/katello/capsule_content.rb

+      end
+
+      katello_repos = Katello::Repository.where(:id => katello_repo_ids)
+      puppet_repos = Katello::ContentViewPuppetEnvironment.where(:id => puppet_repo_ids)


I think you can simplify this a bit by querying by 'pulp_id' on repository and content view puppet enviornment rather than querying once based on id and then a 2nd time based on id.

bbuckingham · 2016-02-05T13:11:00Z

@johnpmitsch, Should this PR remove abstract_node_distributor_task.rb, node_metadata_generate.rb and references to NodesHttpDistributor from katello/glue/pulp/consumer.rb?

bbuckingham · 2016-02-05T13:25:21Z

app/lib/actions/katello/capsule_content/create_or_update.rb

+    module CapsuleContent
+      class CreateOrUpdate < ::Actions::EntryAction
+        # @param capsule_content [::Katello::CapsuleContent]
+        # @param pulp_repo [::Katello::Glue::Pulp::Repo]


Do these @param comments represent the inputs to the 'plan' method? If so, should they be updated to remove repo and add environment and content_view?

jlsherrill · 2016-02-09T21:26:14Z

app/models/setting/katello.rb

-        self.set('pulp_export_destination', N_("On-disk location for exported repositories"), "/tmp/katello-repo-exports")
+        self.set('pulp_export_destination', N_("On-disk location for exported repositories"), "/tmp/katello-repo-exports"),
+        self.set('pulp_client_key', N_("Path for ssl key used for pulp server auth"), "/etc/pki/katello/key.key"),
+        self.set('pulp_client_cert', N_("Path for ssl cert used for pulp server auth"), "/etc/pki/katello/server.crt")


These aren't the default values that are coming down from the installer, they are currently:

/etc/pki/katello/certs/abed.usersys.redhat.com-pulp-client.crt
&
/etc/pki/katello/private/abed.usersys.redhat.com-pulp-client.key

if that makes this difficult we may want to look at modifying the installer.

after chatting with eric, lets just set these to default to "" and update the setting in the installer

I totally forgot about this 😳 If there is no reason we need a hostname in the filename, I think it would be easier and lead to less problems to modify the installer to create a fixed name for the cert and key. I can make this change

you can use something like this: https://github.com/Katello/puppet-certs/blob/master/manifests/foreman.pp#L51-L54

to set the settings as well

PR open theforeman/puppet-certs#73

Also changed the settings to "" for default

johnpmitsch · 2016-02-10T14:11:58Z

app/models/setting/katello.rb

-        self.set('pulp_export_destination', N_("On-disk location for exported repositories"), File.join(Rails.root, 'repo-exports'))
+        self.set('pulp_export_destination', N_("On-disk location for exported repositories"), File.join(Rails.root, 'repo-exports')),
+        self.set('pulp_client_key', N_("Path for ssl key used for pulp server auth"), "/etc/pki/katello/private/pulp-client.key"),
+        self.set('pulp_client_cert', N_("Path for ssl cert used for pulp server auth"), "/etc/pki/katello/certs/pulp-client.crt")


Default values can't be blank so I added the file path that the installer will create here in the default

jlsherrill · 2016-02-12T15:05:59Z

app/lib/actions/katello/capsule_content/create_or_update.rb

+                      checksum_type: checksum_type,
+                      path: relative_path,
+                      with_importer: true,
+                      capsule_id: capsule_content.capsule.id)


i think you need to pass in docker_upstream_name in here for docker rpeos

theforeman-bot · 2016-02-12T17:47:08Z

@johnpmitsch, this pull request is currently not mergeable. Please rebase against the master branch and push again.

If you have a remote called 'upstream' that points to this repository, you can do this by running:

    $ git pull --rebase upstream master

This message was auto-generated by Foreman's prprocessor

theforeman-bot added Waiting on contributor Needs testing Not yet reviewed labels Feb 1, 2016

johnpmitsch force-pushed the remove_pulp_nodes branch from ddb150a to 4c2dfd6 Compare February 1, 2016 19:00

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

johnpmitsch force-pushed the remove_pulp_nodes branch 2 times, most recently from b22e376 to 5f3ee5a Compare February 1, 2016 19:05

johnpmitsch reviewed Feb 1, 2016
View reviewed changes

johnpmitsch force-pushed the remove_pulp_nodes branch from 5f3ee5a to 33fbb91 Compare February 1, 2016 22:31

jlsherrill reviewed Feb 2, 2016
View reviewed changes

johnpmitsch force-pushed the remove_pulp_nodes branch 2 times, most recently from 57012d3 to a0bff17 Compare February 2, 2016 15:46

johnpmitsch reviewed Feb 3, 2016
View reviewed changes

jlsherrill mentioned this pull request Feb 3, 2016

Fixes #13470: Remove the use of Thread sessions for configuration #5741

Merged

jlsherrill reviewed Feb 3, 2016
View reviewed changes

daviddavis mentioned this pull request Feb 4, 2016

Fixes #13568 - Don't create node distrbutors for yum repos #5754

Merged

johnpmitsch changed the title ~~[WIP] Fixes #13191, #13192, #13193, #13195 - Remove pulp nodes~~ Fixes #13191, #13192, #13193, #13195 - Remove pulp nodes Feb 4, 2016

johnpmitsch added the Waiting on contributor label Feb 4, 2016

bbuckingham reviewed Feb 5, 2016
View reviewed changes

theforeman-bot added Waiting on contributor and removed Not yet reviewed labels Feb 9, 2016

johnpmitsch force-pushed the remove_pulp_nodes branch from dd07659 to b1fb230 Compare February 9, 2016 17:10

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

johnpmitsch force-pushed the remove_pulp_nodes branch 2 times, most recently from be77596 to ee821e5 Compare February 9, 2016 20:30

jlsherrill reviewed Feb 9, 2016
View reviewed changes

johnpmitsch mentioned this pull request Feb 9, 2016

Fixes #13635 - set pulp client cert settings theforeman/puppet-certs#73

Merged

johnpmitsch force-pushed the remove_pulp_nodes branch 3 times, most recently from 7f14730 to 30b9dff Compare February 10, 2016 14:05

johnpmitsch reviewed Feb 10, 2016
View reviewed changes

johnpmitsch force-pushed the remove_pulp_nodes branch 4 times, most recently from d545ac4 to df4cce9 Compare February 11, 2016 18:19

johnpmitsch changed the title ~~Fixes #13191, #13192, #13193, #13195 - Remove pulp nodes~~ Fixes #13191, #13192, #13193, #13195, #13196 - Remove pulp nodes Feb 11, 2016

johnpmitsch force-pushed the remove_pulp_nodes branch from df4cce9 to c7d1930 Compare February 11, 2016 22:05

jlsherrill reviewed Feb 12, 2016
View reviewed changes

johnpmitsch force-pushed the remove_pulp_nodes branch from c7d1930 to 5cd360a Compare February 12, 2016 15:57

johnpmitsch closed this Feb 12, 2016

johnpmitsch force-pushed the remove_pulp_nodes branch from 5cd360a to 73f1028 Compare February 12, 2016 17:47

theforeman-bot added Waiting on contributor and removed Needs testing Needs re-review Not yet reviewed labels Feb 12, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixes #13191, #13192, #13193, #13195, #13196 - Remove pulp nodes #5750

Fixes #13191, #13192, #13193, #13195, #13196 - Remove pulp nodes #5750

johnpmitsch commented Feb 1, 2016

johnpmitsch Feb 1, 2016

jlsherrill Feb 1, 2016

jlsherrill Feb 2, 2016

jlsherrill Feb 2, 2016

johnpmitsch Feb 3, 2016

jlsherrill Feb 3, 2016

ehelms Feb 5, 2016

jlsherrill Feb 3, 2016

bbuckingham commented Feb 5, 2016

bbuckingham Feb 5, 2016

jlsherrill Feb 9, 2016

jlsherrill Feb 9, 2016

johnpmitsch Feb 9, 2016

jlsherrill Feb 9, 2016

johnpmitsch Feb 9, 2016

johnpmitsch Feb 9, 2016

johnpmitsch Feb 10, 2016

jlsherrill Feb 12, 2016

theforeman-bot commented Feb 12, 2016

Fixes #13191, #13192, #13193, #13195, #13196 - Remove pulp nodes #5750

Fixes #13191, #13192, #13193, #13195, #13196 - Remove pulp nodes #5750

Conversation

johnpmitsch commented Feb 1, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bbuckingham commented Feb 5, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

theforeman-bot commented Feb 12, 2016