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

Separate scripts to be installed on puppetmaster from those that are installed on foreman server #2

Merged
merged 1 commit into from
May 19, 2012

Conversation

dalen
Copy link
Contributor

@dalen dalen commented Apr 25, 2012

We shouldn't make the assumption that foreman runs on the same hosts as the
puppetmaster.
So this patch separates the scripts that are to be installed on the
puppetmaster (enc & report script) into the class foreman::puppetmaster
that can be added to the puppetmaster.

We shouldn't make the assumption that foreman runs on the same hosts as the
puppetmaster.
So this patch separates the scripts that are to be installed on the
puppetmaster (enc & report script) into the class foreman::puppetmaster
that can be added to the puppetmaster.

Change-Id: I89b0439d18c1e4032b64c12c337de51958e7f1ab
@@ -49,6 +49,5 @@
}

if $foreman::params::reports { include foreman::config::reports }
if $foreman::params::enc { include foreman::config::enc }
Copy link
Member

Choose a reason for hiding this comment

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

should this be part of the configuration params? that you wont be required users to add a requirement to add the puppetmaster class?

looks good!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

On Thu, Apr 26, 2012 at 10:04, Ohad Levy
reply@reply.github.com
wrote:

@@ -49,6 +49,5 @@
   }

   if $foreman::params::reports { include foreman::config::reports }

  •  if $foreman::params::enc     { include foreman::config::enc }

should this be part of the configuration params? that you wont be required
users to add a requirement to add the puppetmaster class?

Well, possibly you could use it only for reports and not as ENC. But
yes, maybe move it directly into the foreman::puppetmaster class
instead of having that one include foreman::config::enc if that's what
you mean?

Erik Dalén

Copy link
Member

Choose a reason for hiding this comment

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

where do you include the foreman::puppetmaster class? in the puppet server class instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

On Thu, Apr 26, 2012 at 19:55, Ohad Levy
reply@reply.github.com
wrote:

@@ -49,6 +49,5 @@
   }

   if $foreman::params::reports { include foreman::config::reports }

  •  if $foreman::params::enc     { include foreman::config::enc }

where do you include the foreman::puppetmaster class? in the puppet server class instead?

Yes, although I already had a class for my puppetmaster with some
other stuff in it as well so I didn't use the foreman-installer module
for that. But I can fix it there as well and send a patch for it,
definitely makes sense to fix that there.

Erik Dalén

Copy link
Member

Choose a reason for hiding this comment

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

great :) as in its current form it would break people who install it from here.

Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I sent a pull request for that as well now. Sorry for the delay.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks @dalen - I'll test both pull-req's together tonight, but it looks pretty good at a glance :)

GregSutcliffe added a commit that referenced this pull request May 19, 2012
Separate scripts to be installed on puppetmaster from those that are installed on foreman server
@GregSutcliffe GregSutcliffe merged commit c19f897 into theforeman:master May 19, 2012
@GregSutcliffe
Copy link
Member

Merged,. Sorry for the delay, and thanks!

mcanevet referenced this pull request in mcanevet/puppet-foreman Mar 13, 2013
Resync with theforeman's github
jsomara referenced this pull request in jsomara/puppet-foreman May 28, 2013
Use SCLed paths in Foreman vhost template
tmclaugh referenced this pull request in hubspotdevops/puppet-foreman Dec 19, 2013
Sync with upstream for Foreman 1.3.0 release. Though I have no intention of moving this branch further and instead try to get https://github.com/ekohl/puppet-foreman/tree/puppetlabs-apache-v2 in working and mergeable shape, but in the mean time I'm fine with merging this.
cegeka-jenkins pushed a commit to cegeka/puppet-foreman that referenced this pull request Oct 23, 2017
Separate scripts to be installed on puppetmaster from those that are installed on foreman server
jturel pushed a commit to jturel/puppet-foreman that referenced this pull request May 1, 2018
Master forked to "oldstable" for older puppet users
Merged to master
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