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

Ensure pulp-server package present before qpid client certs created #270

Closed
wants to merge 1 commit into from

Conversation

ehelms
Copy link
Member

@ehelms ehelms commented Jan 13, 2020

No description provided.

@ehelms
Copy link
Member Author

ehelms commented Jan 13, 2020

Devel installs suffer from this error:

[ERROR 2020-01-13T19:38:46 main]  Could not set 'directory' on ensure: Could not find group pulp (file: /usr/share/foreman-installer/modules/certs/manifests/qpid_client.pp, line: 45)

@ekohl
Copy link
Member

ekohl commented Jan 13, 2020

Should we fix it here? This class has no knowledge of the pulp-server package and thus will fail to compile with all dependencies.

Edit: of course just after I submit I remember theforeman/puppet-foreman#779 (comment)

Untested alternative:

Package <| name == 'pulp-server' |> -> File <| group == 'pulp' |>

@ehelms
Copy link
Member Author

ehelms commented Jan 13, 2020

Maybe? I am not always sure where something should live. This class for example won't work unless the pulp group exists. Is ensuring that exists the purview of this puppet class? or somebody using this class? I think that production likely gets lucky that this works right now.

@ehelms
Copy link
Member Author

ehelms commented Jan 13, 2020

Just the edits, as this current idea doesnt work:

    [ERROR 2020-01-13T20:11:50 verbose] (Cert[candlepin-migration.war.example.com-qpid-client-cert] => Class[Certs::Qpid_client] => Class[Pulp] => Class[Pulp::Install] => Package[pulp-server] => File[/etc/pki/pulp] => Class[Certs::Qpid_client])\nTry the '--graph' option and opening the resulting '.dot' file in OmniGraffle or GraphViz
    [ERROR 2020-01-13T20:11:50 verbose]  Failed to apply catalog: One or more resource dependency cycles detected in graph

@ekohl
Copy link
Member

ekohl commented Jan 13, 2020

Regarding design: most modules take care of placing their own files to be able to properly set the dependencies. In puppet-pulpcore we define the pulp user and group so Puppet would automatically find this. That means we can't define them again in puppet-pulp.

@ehelms
Copy link
Member Author

ehelms commented Jan 13, 2020

The virtual resource idea failed with:

    [ERROR 2020-01-13T20:29:55 verbose]  Could not find group pulp

This is likely due to nothing actually declaring this resource in the devel scenario yet?

@@ -42,27 +42,29 @@

if $deploy {

Package <| name == 'pulp-server' |> -> File <| group == 'pulp' |>
Copy link
Member

Choose a reason for hiding this comment

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

Alternative suggestion:

Suggested change
Package <| name == 'pulp-server' |> -> File <| group == 'pulp' |>
Package <| name == 'pulp-server' |> -> Class['qpid_client']

@ehelms
Copy link
Member Author

ehelms commented Jan 14, 2020

This appears to be the crux of the problem (https://github.com/theforeman/puppet-katello/blob/master/manifests/pulp.pp#L127). We need to generate the messaging certificates before setting up Pulp, but those certificates need pulp-server package in order to have the user and group.

Is there any puppet magic to help?

Copy link
Member

@ekohl ekohl left a comment

Choose a reason for hiding this comment

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

This appears to be the crux of the problem (https://github.com/theforeman/puppet-katello/blob/master/manifests/pulp.pp#L127). We need to generate the messaging certificates before setting up Pulp, but those certificates need pulp-server package in order to have the user and group.

Is there any puppet magic to help?

Usually the actual deployment of certs is done within a module and then applies the correct chaining. Since pulp contains all classes (pulp::install and pulp::service), you can't really get in between there as we've chained them now.

What we can do in the various modules is:

class { 'certs::qpid_client':
  require => Class['pulp::install'],
  notify  => Class['pulp::service'],
}

This needs to be applied in the 3 places where we call pulp: katello, foreman_proxy_content and katello_devel.

@@ -42,27 +42,29 @@

if $deploy {

Package <| name == 'pulp-server' |> -> Class['qpid_client']
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 I wasn't fully awake when I made the suggestion:

Suggested change
Package <| name == 'pulp-server' |> -> Class['qpid_client']
Package <| name == 'pulp-server' |> -> Class['certs::qpid_client']

@ehelms
Copy link
Member Author

ehelms commented Jan 21, 2020

I believe this has been fixed by other updates and PRs. Thanks @ekohl !

@ehelms ehelms closed this Jan 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants