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 #14188 - change pulp_parent to qpid_client class #78

Merged
merged 1 commit into from
Mar 16, 2016

Conversation

johnpmitsch
Copy link
Contributor

No description provided.

$deploy = $::certs::deploy,

$nodes_cert_dir = $certs::params::nodes_cert_dir,
$nodes_cert_name = $certs::params::nodes_cert_name,
Copy link
Member

Choose a reason for hiding this comment

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

Can drop these nodes

@stbenjam
Copy link
Member

We need to pin rake to make tests pass, foreman did this here: theforeman/foreman-installer-modulesync#28

I opened a PR for us to do it: Katello/foreman-installer-modulesync#4

@johnpmitsch johnpmitsch force-pushed the pulp_parent_removal branch from eff2d85 to 4c0f8e9 Compare March 11, 2016 19:11
@stbenjam
Copy link
Member

[test]

$messaging_ca_cert = $certs::ca_cert,
$messaging_client_cert = $certs::params::messaging_client_cert

) inherits pulp::params { # lint:ignore:inherits_across_namespaces
Copy link
Member

Choose a reason for hiding this comment

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

We can drop this inherits

@ehelms
Copy link
Member

ehelms commented Mar 14, 2016

I was thinking maybe we keep pulp_parent and pulp_child for now to make refactoring to use qpid_client less impactful and require less coordination of merging code.

@johnpmitsch johnpmitsch force-pushed the pulp_parent_removal branch from 2fa04ed to cce5ca3 Compare March 14, 2016 15:04
@johnpmitsch johnpmitsch changed the title change pulp_parent to qpid_client class Fixes #14188 - change pulp_parent to qpid_client class Mar 14, 2016
@johnpmitsch johnpmitsch force-pushed the pulp_parent_removal branch 2 times, most recently from de9d7f1 to 2b0e5fd Compare March 14, 2016 15:15
$regenerate = $::certs::regenerate,
$deploy = $::certs::deploy,

$messaging_ca_cert = $certs::ca_cert,
Copy link
Member

Choose a reason for hiding this comment

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

Looks like this parameter isn't used and could be removed

@ehelms
Copy link
Member

ehelms commented Mar 14, 2016

@johnpmitsch johnpmitsch force-pushed the pulp_parent_removal branch from 2b0e5fd to 3f87524 Compare March 14, 2016 15:41
@@ -26,7 +26,6 @@
class { '::certs::apache': hostname => $capsule_fqdn }
class { '::certs::qpid': hostname => $capsule_fqdn }
class { '::certs::qpid_router': hostname => $capsule_fqdn }
class { '::certs::pulp_child': hostname => $capsule_fqdn }
Copy link
Member

Choose a reason for hiding this comment

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

Need to replace this with qpid_client, and down below

@johnpmitsch johnpmitsch force-pushed the pulp_parent_removal branch from 3f87524 to d78e216 Compare March 14, 2016 18:34
@johnpmitsch
Copy link
Contributor Author

@ehelms updated

@ehelms
Copy link
Member

ehelms commented Mar 14, 2016

Can you fix the test failure? Otherwise looks good

@johnpmitsch johnpmitsch force-pushed the pulp_parent_removal branch from d78e216 to e55f49b Compare March 14, 2016 18:46
@johnpmitsch
Copy link
Contributor Author

@ehelms fixed

@johnpmitsch
Copy link
Contributor Author

normal functionality and capsule-certs-generate is working with these changes

@ehelms
Copy link
Member

ehelms commented Mar 16, 2016

ACK - thanks @johnpmitsch !

ehelms added a commit that referenced this pull request Mar 16, 2016
Fixes #14188 - change pulp_parent to qpid_client class
@ehelms ehelms merged commit 65bb6c9 into theforeman:master Mar 16, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants