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

Use delete_undef_values() for mount options #39

Merged
merged 3 commits into from
May 11, 2016
Merged

Conversation

skpy
Copy link
Contributor

@skpy skpy commented Jan 26, 2016

In newer versions of Puppet, undefined variables are actually undef,
and not an empty string. This causes gluster::mount to return a mount
option list that looks like:

defaults,undef,undef,undef,undef,undef

By using stdlib 4.2.0 or newer, we can use the delete_undef_values()
function.

The other alternative would be to explicitly add an else block to
each possible mount option's if to set it to the empty string.

In newer versions of Puppet, undefined variables are actually `undef`,
and not an empty string.  This causes `gluster::mount` to return a mount
option list that looks like:
```
defaults,undef,undef,undef,undef,undef
```
By using stdlib 4.2.0 or newer, we can use the `delete_undef_values()`
function.

The other alternative would be to explicitly add an `else` block to
each possible mount option's `if` to set it to the empty string.
@jyaworski
Copy link
Member

Is stdlib 4.2.0 old enough that this wouldn't require a major version bump?

@skpy
Copy link
Contributor Author

skpy commented Jan 26, 2016

That's a good question. I think so. 4.2.0 was released in mid-2014.

The biggest question, which I didn't test, is whether older Puppet 3.x releases will properly treat undefined variables ($ll, $lf, etc in this manifest) as undef or as just an empty string. If the latter, then we'd have to do else { $lf = '' }.

@jyaworski
Copy link
Member

They're different in 3.x, per here. So it would likely be treated as an empty string.

@daenney
Copy link
Member

daenney commented Jan 26, 2016

That's a good question. I think so. 4.2.0 was released in mid-2014.

Off by one, mid-2015 😉. https://tickets.puppetlabs.com/browse/PUP-4718

@skpy
Copy link
Contributor Author

skpy commented Jan 26, 2016

@daenney the 4.2.0 to which I am referring is the version of stdlib that provides the delete_undef_values() function. But as @jyaworski points out, my proposed solution may not be correct for older versions of Puppet.

New commit coming shortly.

This looks a little janky, but preserves functionality between Puppet
3.x and Puppet 4.x parsers.
@jyaworski
Copy link
Member

Tests are failing. Can you look into it?

@skpy
Copy link
Contributor Author

skpy commented Jan 31, 2016

I've run the tests manually on OSX and CentOS 7.1 without problem. On OSX, I used the system Ruby (2.0.0), and on CentOS I used rvm with Ruby 2.2.4p230.

Form my CentOS VM:

[vagrant@c1-smerrill puppet-gluster-1]$ export PUPPET_VERSION="~> 4.0"
[vagrant@c1-smerrill puppet-gluster-1]$ export STRICT_VARIABLES="yes"
[vagrant@c1-smerrill puppet-gluster-1]$ ruby --version
ruby 2.2.4p230 (2015-12-16 revision 53155) [x86_64-linux]
[vagrant@c1-smerrill puppet-gluster-1]$ rvm --version
rvm 1.26.11 (latest) by Wayne E. Seguin <wayneeseguin@gmail.com>, Michal Papis <mpapis@gmail.com> [https://rvm.io/]
[vagrant@c1-smerrill puppet-gluster-1]$ bundle --version
Bundler version 1.11.2
[vagrant@c1-smerrill puppet-gluster-1]$ gem --version
2.4.8
[vagrant@c1-smerrill puppet-gluster-1]$ rm Gemfile.lock
[vagrant@c1-smerrill puppet-gluster-1]$ bundle install --without system_tests --path=vendor/bundle
Fetching https://github.com/rodjek/rspec-puppet.git
Fetching https://github.com/rodjek/puppet-lint.git
Fetching https://github.com/voxpupuli/puppet-blacksmith.git
Fetching https://github.com/voxpupuli/voxpupuli-release-gem.git
Fetching gem metadata from https://rubygems.org/.........
Fetching version metadata from https://rubygems.org/...
Fetching dependency metadata from https://rubygems.org/..
Resolving dependencies.......
Using rake 10.5.0
Using CFPropertyList 2.2.8
Using addressable 2.4.0
Using ast 2.2.0
Using backports 3.6.7
Using coderay 1.1.0
Using diff-lcs 1.2.5
Using unf_ext 0.0.7.1
Using ffi 1.9.10
Using facter 2.4.6
Using json 1.8.3
Using multipart-post 2.0.0
Using formatador 0.2.5
Using multi_json 1.11.2
Using net-http-persistent 2.9.4
Using net-http-pipeline 1.0.1
Using rb-fsevent 0.9.7
Using lumberjack 1.0.10
Using nenv 0.2.0
Using shellany 0.0.1
Using method_source 0.8.2
Using slop 3.6.0
Using thor 0.19.1
Using json_pure 1.8.3
Using highline 1.7.8
Using metaclass 0.0.4
Using mime-types 2.99
Using netrc 0.11.0
Using powerpack 0.1.1
Using puppet-lint 1.1.0 from https://github.com/rodjek/puppet-lint.git (at master@2546fed)
Using rspec-support 3.4.1
Using websocket 1.2.2
Using rainbow 2.1.0
Using rspec-puppet-utils 2.2.1
Using ruby-progressbar 1.7.5
Using bundler 1.11.2
Using puppet-syntax 2.1.0
Using launchy 2.4.3
Using parser 2.3.0.2
Using unf 0.1.4
Using ethon 0.8.1
Using rb-inotify 0.9.5
Using jgrep 1.4.0
Using spdx-licenses 1.0.0
Using travis-lint 2.0.0
Using faraday 0.9.2
Using notiffany 0.0.8
Using pry 0.10.3
Using hiera 3.0.6
Using mocha 1.1.0
Using puppet-lint-absolute_classname-check 0.1.3
Using puppet-lint-classes_and_types_beginning_with_digits-check 0.1.0
Using puppet-lint-leading_zero-check 0.1.0
Using puppet-lint-trailing_comma-check 0.3.1
Using puppet-lint-unquoted_string-check 0.2.5
Using puppet-lint-variable_contains_upcase 1.0.0
Using puppet-lint-version_comparison-check 0.1.2
Using rspec-core 3.4.2
Using rspec-expectations 3.4.0
Using rspec-mocks 3.4.1
Using pusher-client 0.6.2
Using astrolabe 1.3.1
Using domain_name 0.5.20160128
Using typhoeus 0.8.0
Using listen 3.0.5
Using facterdb 0.3.1
Using metadata-json-lint 0.0.11
Using faraday_middleware 0.10.0
Using gh 0.14.0
Using puppet 4.3.2
Using rspec 3.4.0
Using rubocop 0.35.0
Using http-cookie 1.0.2
Using guard 2.13.0
Using travis 1.8.2
Using rspec-puppet-facts 1.3.0
Using rspec-puppet 2.3.2 from https://github.com/rodjek/rspec-puppet.git (at master@6bbedd2)
Using rest-client 1.8.0
Using guard-rake 1.0.0
Using puppetlabs_spec_helper 1.0.1
Using puppet-blacksmith 3.3.1 from https://github.com/voxpupuli/puppet-blacksmith.git (at master@4029c26)
Using voxpupuli-release 0.3.0 from https://github.com/voxpupuli/voxpupuli-release-gem.git (at master@0624378)
Bundle complete! 26 Gemfile dependencies, 82 gems now installed.
Gems in the group system_tests were not installed.
Bundled gems are installed into ./vendor/bundle.
[vagrant@c1-smerrill puppet-gluster-1]$ bundle exec rake test
---> syntax:manifests
---> syntax:templates
---> syntax:hiera:yaml
Cloning into 'spec/fixtures/modules/stdlib'...
remote: Counting objects: 441, done.
remote: Compressing objects: 100% (318/318), done.
remote: Total 441 (delta 151), reused 255 (delta 107), pack-reused 0
Receiving objects: 100% (441/441), 192.62 KiB | 0 bytes/s, done.
Resolving deltas: 100% (151/151), done.
/home/vagrant/.rvm/rubies/ruby-2.2.4/bin/ruby -I/home/vagrant/puppet-gluster-1/vendor/bundle/ruby/2.2.0/gems/rspec-core-3.4.2/lib:/home/vagrant/puppet-gluster-1/vendor/bundle/ruby/2.2.0/gems/rspec-support-3.4.1/lib /home/vagrant/puppet-gluster-1/vendor/bundle/ruby/2.2.0/gems/rspec-core-3.4.2/exe/rspec --pattern spec/\{classes,defines,unit,functions,hosts,integration,types\}/\*\*/\*_spec.rb --color

gluster::client
  when installing on Red Hat Enterprise Linux
    when using all default values
      should include gluster::install
    when a version number is specified
      should include gluster::install with version 3.6.1
    when repo is false
      should include gluster::install with repo=>false

gluster
  installing on Red Hat Enterprise Linux
    using all defaults
      should create gluster::install
      should manage the Gluster service
    specific version and package names defined
      should create gluster::install
      should manage the Gluster service
    when volumes defined
      should create gluster::volume
    when volumes incorrectly defined
      should fail

gluster::install
  installing on an unsupported architecture
    should not install
  installing on Red Hat Enterprise Linux
    when repo is true
      should create gluster::repo
    when repo is false
      should not create gluster::repo
    when client is true
      should install glusterfs-fuse package
    when client is false
      should not install glusterfs-fuse package
    when server is true
      should install glusterfs-server
    when server is false
      should not install glusterfs-server

gluster::repo::yum
  version not specified
    should not install
  bogus version
    should not install
  unsupported architecture
    should not install
  Red Hat Enterprise Linux
    latest Gluster on RHEL 6 x86_64
      should install
    latest Gluster on RHEL 6 x86_64 with priority
      should install

gluster::service
  should start the service

gluster::mount
  no volume specified
    should fail
  bogus ensure value
    should fail

Finished in 2.34 seconds (files took 0.64684 seconds to load)
24 examples, 0 failures

@jyaworski
Copy link
Member

Can you rebase?

@jyaworski jyaworski added the enhancement New feature or request label Feb 29, 2016
@jyaworski
Copy link
Member

Ping @skpy

Use `delete_undef_values` rather than create empty-string variables.
@skpy
Copy link
Contributor Author

skpy commented May 11, 2016

@jyaworski merged master, and used delete_undef_values.

@jyaworski jyaworski merged commit d80e75e into voxpupuli:master May 11, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants