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

Add options to mod info #717

Merged
merged 1 commit into from
Jul 29, 2014
Merged

Add options to mod info #717

merged 1 commit into from
Jul 29, 2014

Conversation

genebean
Copy link
Contributor

@genebean genebean commented May 3, 2014

While using this wonderful module at work today I ran into an issue where there was no way to modify the info.conf file created by include apache::mod::info The purpose of this PR is to add that functionality.

@genebean
Copy link
Contributor Author

genebean commented May 3, 2014

The test below are identical in syntax as far as I can see yet the FreeBSD one fails... not sure what's going on here and would love some help. The line general_info_specs() refers to the block of code containing all the tests.

  context 'On a Debian OS' do
    let :facts do
      {
        :osfamily               => 'Debian',
        :operatingsystemrelease => '6',
        :concat_basedir         => '/dne',
      }
    end

    # Load the more generic tests for this context
    general_info_specs()

    it { should contain_file('info.conf').with({
      :ensure => 'file',
      :path   => '/etc/apache2/mods-available/info.conf',
    } ) }
    it { should contain_file('info.conf symlink').with({
      :ensure => 'link',
      :path   => '/etc/apache2/mods-enabled/info.conf',
    } ) }
  end

  context 'on a RedHat OS' do
    let :facts do
      {
        :osfamily               => 'RedHat',
        :operatingsystemrelease => '6',
        :concat_basedir         => '/dne',
      }
    end

    # Load the more generic tests for this context
    general_info_specs()

    it { should contain_file('info.conf').with({
      :ensure => 'file',
      :path   => '/etc/httpd/conf.d/info.conf',
      } ) 
    }
  end

  context 'On a FreeBSD OS' do
    let :facts do
      {
        :osfamily               => 'FreeBSD',
        :operatingsystemrelease => '9',
        :concat_basedir         => '/dne',
      }
    end

    # Load the more generic tests for this context
    general_info_specs()

    it { should contain_file('info.conf').with({
      :ensure => 'file',
      :path   => '/usr/local/etc/apache22/Modules/info.conf',
    } ) }
  end

@igalic
Copy link
Contributor

igalic commented May 18, 2014

This line might be the reason why it fails on FreeBSD

@genebean
Copy link
Contributor Author

@igalic I'll look at that more this afternoon but was wondering if you would elaborate on why the inclusion of mod_info would be causing this?

@igalic
Copy link
Contributor

igalic commented May 18, 2014

This is the only place this "extra" include actually happens.

Generally, multiple includes don't matter, however, in a later test, you're passing doing a resource definition to test a specific set of parameters.

There are two ways to fix this: Either, on FreeBSD you modify the default_mods in your test, or in general.

For the latter, I would create a Jira issue first.

@genebean
Copy link
Contributor Author

Thanks! I'll investigate this today.

@igalic
Copy link
Contributor

igalic commented May 18, 2014

IIRC, the main reason why we have different set of default_mods for different platforms is so that we actually conform to their expected defaults.

@igalic
Copy link
Contributor

igalic commented Jun 4, 2014

@genebean any news on this? Can you rebase it as a starter?

@apenney
Copy link
Contributor

apenney commented Jun 26, 2014

^^ any news on the rebase?

@genebean
Copy link
Contributor Author

genebean commented Jul 6, 2014

Sorry, school and work got in the way of working on this. Going to attempt doing the rebase tonight and / or tomorrow. Actually pulling new code after posting this update.

@genebean
Copy link
Contributor Author

genebean commented Jul 7, 2014

Rebasing done, now on to trying to fix the duplicate declaration error.

@genebean
Copy link
Contributor Author

genebean commented Jul 7, 2014

Thanks for finding that line in the default mods list @igalic! I updated my testing to not include the default mods and all tests related to mod_info are now passing.

It seems some other modules are the cause of the Puppet 3.5 tests failing as all mod_info test pass there too.

@genebean
Copy link
Contributor Author

genebean commented Jul 7, 2014

@apenney - this was the first time I've had a need to do a rebase... if you happen to have any tips for how i could have improved on what I did here I welcome them.

@blkperl
Copy link
Contributor

blkperl commented Jul 7, 2014

Github has some good documentation on how to rebase.

https://help.github.com/articles/about-git-rebase

Generally you would want something like

git rebase --interactive HEAD~18

but since you have some merge commits in there you will want to get rid of them first.

# or whatever origin has the github remote
git fetch upstream                 

# this will put your commit on top and remove the merge commits
git rebase upstream/master   

# now rebase using the number of commits
# choose the fixup option to squash all the commits, delete all the commit msgs and write a new 
# commit message that describes what this pr does
git rebase --interactive HEAD~<numofcommits> 

@genebean
Copy link
Contributor Author

genebean commented Jul 8, 2014

so, in this case since the code is passing builds, do I need to clean up the history or is it ready to merge?

Updated README.md with new settings info.
Updated tests for apache::mod::info
@genebean
Copy link
Contributor Author

genebean commented Jul 9, 2014

Sorry @blkperl, reread your post and realized you were telling me to clean stuff up. I have squashed all commits into one. The new commit is genebean/puppetlabs-apache@28d2371

@genebean
Copy link
Contributor Author

Just wanted to follow up and see what's next here.

@blkperl
Copy link
Contributor

blkperl commented Jul 27, 2014

Any idea why the tests are failing?

@genebean
Copy link
Contributor Author

Stuff unrelated to mod_info on Puppet 3.5. I see failures in the following

  • apache::mod::negotiation
  • apache::mod::php
  • apache::mod::worker
  • apache::fastcgi::server

@genebean
Copy link
Contributor Author

FWIW, I see from the build that just ran (https://travis-ci.org/puppetlabs/puppetlabs-apache/builds/29471380) that the same 4 things are failing but only on Puppet 3.5.

@igalic
Copy link
Contributor

igalic commented Jul 29, 2014

This is just because of strict variables, @apenney is working super hard to fix that with puppet_facts

igalic added a commit that referenced this pull request Jul 29, 2014
@igalic igalic merged commit 86e1118 into puppetlabs:master Jul 29, 2014
@genebean
Copy link
Contributor Author

Thanks for the info @igalic, I appreciate it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants