Refs #28924: Drop qpid client configuration used by older katello-qpid #336
Conversation
|
Cleared tests |
|
Devel module updates: |
manifests/pulp.pp
Outdated
| @@ -84,6 +84,7 @@ | |||
| ) { | |||
| include katello::params | |||
| include certs | |||
| include certs::qpid | |||
|
|
|||
| class { 'certs::qpid_client': | |||
There was a problem hiding this comment.
You deleted this class but still include it here.
There was a problem hiding this comment.
I dropped katello::qpid_client. This one handles client certificates for Pulp effectively to talk to Qpid.
There was a problem hiding this comment.
I shouldn't do reviews when I'm not fully awake ... I assumed the test failure was due to this
|
Finally got around to updating the tests. |
manifests/pulp.pp
Outdated
| @@ -100,6 +100,7 @@ | |||
| ) { | |||
| include katello::params | |||
| include certs | |||
| include certs::qpid | |||
There was a problem hiding this comment.
This feels odd to me. Why does Pulp need qpid server certificates? However, the same is done here. I don't think it's worth to clean that up given Pulp 2 will be going away.
@timogoebel @laugmanuel do you think this will be an issue in your split deployments?
There was a problem hiding this comment.
Suppose Pulp itself does not need them, but they are only relevant when Pulp is involved so we need to ensure they get setup if Pulp is present for it's tasking system.
There was a problem hiding this comment.
I always thought certs::qpid were there to run a qpid server while certs::qpid_client were there to connect to a qpid server. Pulp only connects to a pulp server, so shouldn't this only use certs::qpid_client?
There was a problem hiding this comment.
That's true. So the confusing part is then why they were where they were in the first place. I'll update and drop this from here.
In theory we can drop this, giving it a test first.