Skip to content

(PUP-3248) Fix PMT tests for directory environment#3099

Closed
johnduarte wants to merge 2 commits intopuppetlabs:masterfrom
johnduarte:pmt_directory_env_support
Closed

(PUP-3248) Fix PMT tests for directory environment#3099
johnduarte wants to merge 2 commits intopuppetlabs:masterfrom
johnduarte:pmt_directory_env_support

Conversation

@johnduarte
Copy link
Contributor

The beaker host['distmoduledir'] variable is not meaningful if the
host is using directory environments (e.g. 'production').

This commit updates the helper methods for validating that a module
is (or is not) installed on the host by iterating over the
modulepath if a specific modulepath is not provided.

The calls to these helper methods have been updated for the change
in parameter ordering in the helper methods.

@puppetcla
Copy link

CLA signed by all contributors.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is probably better specified as a 'puppet config print modulepath --environment '; default could be 'production'. Otherwise we're just obtaining the modulepath for whatever environment happens to be associated with the agent section of puppet.conf, (which is likely to be the default, 'production', but it's a little vague).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jpartlow the purpose of this helper is to obtain the modulepath segments as an array. It is not intended to obtain the the environment.

Copy link
Contributor

Choose a reason for hiding this comment

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

Right, but modulepath is a property of the environment. Really all I'm saying is that it's clearer to obtain settings from puppet by using the puppet config tool and specifying environment and section as needed. A puppet agent --configprint modulepath is the equivalent of saying puppet config print --section agent modulepath And moving forward, modulepath is deprecated in a puppet.conf section. Instead you really need to be asking puppet config print modulepath --environment <something>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jpartlow thank you for the additional context. I have pushed an update incorporating your suggested change to puppet config print modulepath --environment #{environment}.

@johnduarte johnduarte force-pushed the pmt_directory_env_support branch 2 times, most recently from 655eabc to b7f521c Compare September 18, 2014 17:59
The beaker host['distmoduledir'] variable is not meaningful if the
host is using directory environments (e.g. 'production').

This commit updates the helper methods for validating that a module
is (or is not) installed on the host by iterating over the
modulepath if a specific modulepath is not provided.

The calls to these helper methods have been updated for the change
in parameter ordering in the helper methods.
@johnduarte johnduarte force-pushed the pmt_directory_env_support branch from b7f521c to 0126627 Compare September 19, 2014 00:29
Copy link
Contributor

Choose a reason for hiding this comment

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

Why don't we want these running in the pe suite?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jpartlow Running these in PE does 'bad things' to the puppet master. I haven't had a chance to do the root cause analysis as to why yet.

Copy link
Contributor

Choose a reason for hiding this comment

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

I suspect it's because the generate_base_legacy_and_directory_environments is being given the real master['puppetpath']. We should probably change this to point to a tmpdir, and then I think this will be insulated.

@jpartlow
Copy link
Contributor

Also, let's rebase this to stable and we'll merge up from there; since there's still discussion of a Puppet 3.7.2 and the pe-puppet 3.4.x is targeted to the 3.7.x puppet stable atm.

@johnduarte
Copy link
Contributor Author

@jpartlow this has already been rebased on stable. Cheers.

@jpartlow
Copy link
Contributor

The PR's still targeting master though?

@johnduarte
Copy link
Contributor Author

@jpartlow this is the error I get if the environmentpath is not amended with the given path in the setup method.

  * Install a module into a non default legacy environment

centos6-64-ma 14:33:40$  puppet module install pmtacceptance-nginx --config=/tmp/environmentpath.Dygiq5/puppet2.conf --environment=legacyenv  
/opt/puppet/lib/ruby/site_ruby/1.9.1/puppet/application.rb:365:in `run': Could not find a directory environment named 'legacyenv' anywhere in the path: /etc/puppetlabs/puppet/environments. Does the directory exist? (Puppet::Environments::EnvironmentNotFound)
    from /opt/puppet/lib/ruby/site_ruby/1.9.1/puppet/util/command_line.rb:146:in `run'
    from /opt/puppet/lib/ruby/site_ruby/1.9.1/puppet/util/command_line.rb:92:in `execute'
    from /usr/local/bin/puppet:8:in `<main>'

@jpartlow
Copy link
Contributor

Ah, ok, I think what is going on. We are inheriting the original puppet.conf settings (sourced from $settings::config), and they now have environmentpath set, so that legacyenv is being ignored. So we do need to set environmentpath in the generate_base_legacy_and_directory_environments() method, but we want to set it to an empty string to begin with (clear it, essentially).

Copy link
Contributor

Choose a reason for hiding this comment

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

So I think this just needs to change to:

"environmentpath", ""

(and we can get ride of the envpath variable lines above that)

This commit changes calls to the
generate_base_legacy_and_directory_environments helper to use a
tmpdir rather than master['puppetpath'] to isolate the test
environments and prevent collisions.

The generate_base_legacy_and_directory_environments has also been
altered to remove the `environmentpath` setting in the generated
configuration file.
@johnduarte johnduarte force-pushed the pmt_directory_env_support branch from b9f8dfe to 5a49f76 Compare September 19, 2014 23:10
@johnduarte
Copy link
Contributor Author

This PR needs to be reopened against the puppet stable branch.

@johnduarte johnduarte closed this Sep 19, 2014
@johnduarte johnduarte deleted the pmt_directory_env_support branch February 11, 2015 19:38
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