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

Added support to modify service startup #130

Merged
merged 11 commits into from Aug 5, 2015
Merged

Added support to modify service startup #130

merged 11 commits into from Aug 5, 2015

Conversation

Cicco0
Copy link
Contributor

@Cicco0 Cicco0 commented Jun 19, 2015

I needed the possibility to modify the startup service, so I put this in. Now you are able to start bind in IPv4/IPv6 mode or passing some other variables.

In addition I had to fix some issues with concat_basedir to run rpsec tests locally.

I needed support for accessing the startup hocks, so I put this in. Now
you are able to define to resolve the configuration or to start bind in
ipv4 / ivp6 mode.
I made sure that the module will not overwrite any other file.
Fix rspec files with missing comcat_basedir definition.
@@ -0,0 +1,90 @@
# == Define: dns::server::startup
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we are ready to need this. I don't really like the name "startup" though, it makes me think it handles init script.

How about class: dns::server::defaults_file ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How about class dns::server::defaults? The definition of defaults_file will be used in the code to set the filename.

Copy link
Collaborator

Choose a reason for hiding this comment

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

That sounds fine

@Cicco0
Copy link
Contributor Author

Cicco0 commented Jun 26, 2015

Now it is renamed and corrected. Do you fix the merge conflict?

@solarkennedy
Copy link
Collaborator

It is usually up to the pull requested to fix merge conflicts and bring the PR into a mergable form.

Conflicts:
	manifests/server/params.pp
	spec/classes/dns__server__config_spec.rb
@Cicco0
Copy link
Contributor Author

Cicco0 commented Jun 26, 2015

The merge conflicts are solved :-)

@@ -6,4 +6,5 @@
# include dns::install
# include dns::config
# include dns::service
include dns::server::default
Copy link
Collaborator

Choose a reason for hiding this comment

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

Shouldn't this be in dns::server::install? Seems out of place here

@Cicco0
Copy link
Contributor Author

Cicco0 commented Jun 29, 2015

Ok is fixed and was moved to "manifest/server/install.pp".


) inherits dns::server::params {

if ! defined(Class['::dns::server']) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is incorrect now, this class should be safe to include no matter what

@solarkennedy
Copy link
Collaborator

Good.

We have an acceptance test suite using beaker that is now failing. We don't run these on Travis as they are too intensive.

You can run these tests with bundle exec rake beaker per the contributing doc:
https://github.com/ajjahn/puppet-dns/blob/master/CONTRIBUTING.md#integration-tests

I ran them here and here are the failures:
http://pastebin.com/JJWqzBqA

@Cicco0
Copy link
Contributor Author

Cicco0 commented Jul 10, 2015

I'll try it. Apparently I do not get it to work "out of the box".
github-puppet-dns/vendor/bundle/ruby/2.1.0/gems/beaker-2.16.0/lib/beaker/hypervisor/vagrant.rb:190:in block (2 levels) in vagrant_cmd': Failed to exec 'vagrant destroy --force'. Error was Bundler is using a binstub that was created for a different gem. (RuntimeError)`

@Cicco0
Copy link
Contributor Author

Cicco0 commented Aug 4, 2015

Now, after several more days, the problem could be solved. All tests were carried out and I get no error.

@solarkennedy
Copy link
Collaborator

Good. Can you pastebin the output?

@Cicco0
Copy link
Contributor Author

Cicco0 commented Aug 5, 2015

Sure:
bundle exec rake test

bundle exec rake spec

bundle exec rake beaker

@solarkennedy
Copy link
Collaborator

Thank you!

solarkennedy added a commit that referenced this pull request Aug 5, 2015
Added support to modify service startup
@solarkennedy solarkennedy merged commit 1157144 into ajjahn:master Aug 5, 2015
@Cicco0 Cicco0 deleted the feature/service-startup branch November 10, 2015 11:02
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.

None yet

2 participants