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

Auto refresh service resource on change in bp.conf #10

Merged
merged 2 commits into from
Jun 26, 2017

Conversation

pythiankowalczyk
Copy link
Contributor

@pythiankowalczyk pythiankowalczyk commented Jun 24, 2017

…f service after changes in config file.

Closes #9.

@stivesso
Copy link
Collaborator

Thanks Piotr,

But I was more thinking about changing the require below ( On the 'netbackup-client' service ) to subscribe (subscribe — Applies a resource after the target resource. The subscribing resource refreshes if the target resource changes.) The main difference with the approach you proposed is that I am still keeping only one metaparameter for this relationship (which will be subscribe) instead of two (notify and require). Let me know what you think about that (see what it will look like below).

  file { 'bp.conf':
    ensure  => file,
    path    => '/usr/openv/netbackup/bp.conf',
    owner   => 'root',
    group   => 'root',
    mode    => '0644',
    content => template('netbackup/bp.conf.erb'),
  }

  # Only define netbackup init service if netbackup_version fact is set
  if $::netbackup_version != undef {
    service { 'netbackup-client':
      ensure     => $service_enabled,
      name       => 'netbackup',
      hasrestart => false,
      hasstatus  => false,
      pattern    => 'bpcd',
      provider   => $service_provider,
      require    => File['bp.conf']
    }
  }

With subscribe instead of require:

  file { 'bp.conf':
    ensure  => file,
    path    => '/usr/openv/netbackup/bp.conf',
    owner   => 'root',
    group   => 'root',
    mode    => '0644',
    content => template('netbackup/bp.conf.erb'),
  }

  # Only define netbackup init service if netbackup_version fact is set
  if $::netbackup_version != undef {
    service { 'netbackup-client':
      ensure      => $service_enabled,
      name        => 'netbackup',
      hasrestart => false,
      hasstatus  => false,
      pattern     => 'bpcd',
      provider   => $service_provider,
      subscribe => File['bp.conf']
    }
  }

@SohamChakraborty
Copy link

Steve, good catch. I like your approach. I will let Piotr comment what he thinks, but I am also in favor of having one metaparameter instead of two and specially so when one metaparameter can do the job.

@pythiankowalczyk
Copy link
Contributor Author

pythiankowalczyk commented Jun 26, 2017 via email

@andskli
Copy link
Owner

andskli commented Jun 26, 2017

@pythiankowalczyk will you update this PR with the suggested changes from @stivesso?

@pythiankowalczyk
Copy link
Contributor Author

@andskli here you go.

@andskli andskli changed the title https://github.com/andskli/puppet-netbackup/issues/9 - auto refresh o… Auto refresh service resource on change in bp.conf Jun 26, 2017
@andskli
Copy link
Owner

andskli commented Jun 26, 2017

Thank you. I took myself the liberty to actually update the description of this PR etc.

@stivesso you happy with this? If so LGTM once tests have passed.

@pythiankowalczyk
Copy link
Contributor Author

pythiankowalczyk commented Jun 26, 2017 via email

@andskli
Copy link
Owner

andskli commented Jun 26, 2017

I'm reopening this PR so that it can be merged.

@andskli andskli reopened this Jun 26, 2017
@SohamChakraborty
Copy link

SohamChakraborty commented Jun 26, 2017 via email

@andskli
Copy link
Owner

andskli commented Jun 26, 2017

@SohamChakraborty see #11. I'll look into it.

@stivesso
Copy link
Collaborator

Hi Andreas,

LGTM! Thanks Guyz...

@andskli andskli merged commit 7350405 into andskli:master Jun 26, 2017
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.

4 participants