Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Use qpid::tools class #97

Merged
merged 1 commit into from
Sep 8, 2015
Merged

Use qpid::tools class #97

merged 1 commit into from
Sep 8, 2015

Conversation

ehelms
Copy link
Member

@ehelms ehelms commented Sep 4, 2015

Switches to using qpid::tools class instead of ensuring package
directly to prevent duplicate resource errors

@@ -15,7 +15,7 @@
Service[$broker_service] -> Exec['migrate_pulp_db']
} else {
if $pulp::messaging_transport == 'qpid' {
package { 'qpid-tools': ensure => installed } -> Class['pulp::service']
Class['qpid::tools'] -> Class['pulp::service']
Copy link
Member

Choose a reason for hiding this comment

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

You gave the example though of qpid being external to the pulp - does it need qpid-tools still on the pulp in that case?

If so, you can include qpid::tools here too

Copy link
Member Author

Choose a reason for hiding this comment

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

I think this needs to be present even on a pulp server before migrations are run? I can do further testing to try to pinpoint this better.

Copy link
Member

Choose a reason for hiding this comment

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

Right but Class['qpid::tools'] isn't including the class, it's just forcing the dependency if it's present.

I think you need to do include qpid:tools before this line to account for the case you put qpid on a different server, which doesn't apply to us but could for others.

Switches to using qpid::tools class instead of ensuring package
directly to prevent duplicate resource errors
@ehelms
Copy link
Member Author

ehelms commented Sep 8, 2015

Good call -- updated

@stbenjam
Copy link
Member

stbenjam commented Sep 8, 2015

APT

ehelms added a commit that referenced this pull request Sep 8, 2015
@ehelms ehelms merged commit c077b54 into theforeman:master Sep 8, 2015
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants