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

Fix problems with volume_extend recipe #59

Merged
merged 1 commit into from
Jun 10, 2016
Merged

Fix problems with volume_extend recipe #59

merged 1 commit into from
Jun 10, 2016

Conversation

dpattmann
Copy link
Contributor

Hi @shortdudey123,

the behaviour of the LVM cookbook has changed in version > 2.0.

To use a newer version of the LVM cookbook someone has to rewrite the volume_extend.rb recipe. 👼

Best,
Dennis

@@ -7,7 +7,7 @@
version '5.0.1'
depends 'apt', '>= 2.0'
depends 'yum', '>= 3.0'
depends 'lvm', '>= 1.5.1'
depends 'lvm', '< 2.0.0'
Copy link
Owner

Choose a reason for hiding this comment

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

Can you lock this to 1.6.1 instead?
There is changes in that version are required to make things work correctly.

@shortdudey123
Copy link
Owner

Also, can you rebase on master so tests pass :)

@shortdudey123
Copy link
Owner

Can you also file an issue here with the info about what lvm 2+ breaks and we will take a look at it

@dpattmann
Copy link
Contributor Author

From the LVM Changelog:

  • The gems are now installed when the provider is first used instead of in the default recipe.

The ruby gem is now missing...

Bug from Chef run

       ================================================================================
       Recipe Compile Error in /tmp/kitchen/cache/cookbooks/gluster/recipes/server.rb
       ================================================================================

       LoadError
       ---------
       cannot load such file -- lvm

       Cookbook Trace:
       ---------------
         /tmp/kitchen/cache/cookbooks/gluster/recipes/volume_extend.rb:25:in `block in from_file'
         /tmp/kitchen/cache/cookbooks/gluster/recipes/volume_extend.rb:20:in `each'
         /tmp/kitchen/cache/cookbooks/gluster/recipes/volume_extend.rb:20:in `from_file'
         /tmp/kitchen/cache/cookbooks/gluster/recipes/server.rb:25:in `from_file'

       Relevant File Content:
       ----------------------
       /tmp/kitchen/cache/cookbooks/gluster/recipes/volume_extend.rb:

        18:  # limitations under the License.
        19:  #
        20:  node['gluster']['server']['volumes'].each do |volume_name, volume_values|
        21:    # 1. Get the current size of the logical volume
        22:    # 2. Compare to the size set for the gluster volume
        23:    # 3. If different, run a resize action against that volume
        24:    # ToDO: change hardcoded VG name gluster into an attribute
        25>>   require 'lvm'
        26:  
        27:    LVM::LVM.new do |lvm|
        28:      lvm.logical_volumes.each do |lv|
        29:        # I'm ignoring these as I think this layout helps readability
        30:        # rubocop:disable Style/Next, Style/CaseIndentation, Lint/EndAlignment
        31:        if lv.full_name.to_s == "gluster/#{volume_name}"
        32:          lv_size_cur = lv.size.to_i
        33:          # Borrowed from the lvm cookbook
        34:          volume_lv_size_req = case volume_values['size']

       Platform:
       ---------
       x86_64-linux

@shortdudey123
Copy link
Owner

hmm ok, since volume_extend should never expand a volume on the first chef run (since it was just created anyway) maybe a guard needs to be put on the include_recipe instead of chasing the LVM cookbook around again

@dpattmann
Copy link
Contributor Author

Couldn't we just extend the server_install.rb recipe with the code from the lvm::default recipe in version 1.6.1 and remove the lvm cookbook completely from this cookbook?

@shortdudey123
Copy link
Owner

You would need to copy the LWRP's in as well and i would rather not do that

@dpattmann
Copy link
Contributor Author

I found another problem with the LVM cookbook... 😒

If you have defined a volume like gv0 during the first chef-run gluster::volume_extend is executed before the package 'lvm2' resource from the lvm::default cookbook.

Synchronizing Cookbooks:
  - gluster (5.0.1)
  - apt (3.0.0)
  - compat_resource (12.9.1)
  - yum (3.8.2)
  - lvm (1.6.1)
Compiling Cookbooks...
Recipe: lvm::default
  * chef_gem[di-ruby-lvm-attrib] action install (up to date)
  * chef_gem[di-ruby-lvm] action install (up to date)
[2016-05-27T14:36:53+00:00] WARN: This server is not configured for this volume
[2016-05-27T14:36:53+00:00] WARN: I can't invite myself into the pool!

  ================================================================================
  Recipe Compile Error in /var/chef/cache/cookbooks/gluster/recipes/server.rb
  ================================================================================

  Errno::ENOENT
  -------------
  No such file or directory - /sbin/lvm

  Cookbook Trace:
  ---------------
    /var/chef/cache/cookbooks/gluster/recipes/volume_extend.rb:27:in `new'
    /var/chef/cache/cookbooks/gluster/recipes/volume_extend.rb:27:in `block in from_file'
    /var/chef/cache/cookbooks/gluster/recipes/volume_extend.rb:20:in `each'
    /var/chef/cache/cookbooks/gluster/recipes/volume_extend.rb:20:in `from_file'
    /var/chef/cache/cookbooks/gluster/recipes/server.rb:25:in `from_file'

  Relevant File Content:
  ----------------------
  /var/chef/cache/cookbooks/gluster/recipes/volume_extend.rb:

   20:  node['gluster']['server']['volumes'].each do |volume_name, volume_values|
   21:    # 1. Get the current size of the logical volume
   22:    # 2. Compare to the size set for the gluster volume
   23:    # 3. If different, run a resize action against that volume
   24:    # ToDO: change hardcoded VG name gluster into an attribute
   25:    require 'lvm'
   26:  
   27>>   LVM::LVM.new do |lvm|
   28:      lvm.logical_volumes.each do |lv|
   29:        # I'm ignoring these as I think this layout helps readability
   30:        # rubocop:disable Style/Next, Style/CaseIndentation, Lint/EndAlignment
   31:        if lv.full_name.to_s == "gluster/#{volume_name}"
   32:          lv_size_cur = lv.size.to_i
   33:          # Borrowed from the lvm cookbook
   34:          volume_lv_size_req = case volume_values['size']
   35:            when /^(\d+)(k|K)$/
   36:              (Regexp.last_match[1].to_i * 1024)


  Running handlers:
[2016-05-27T14:36:53+00:00] ERROR: Running exception handlers
  Running handlers complete
[2016-05-27T14:36:53+00:00] ERROR: Exception handlers complete
  Chef Client failed. 0 resources updated in 01 seconds
[2016-05-27T14:36:53+00:00] FATAL: Stacktrace dumped to /var/chef/cache/chef-stacktrace.out
[2016-05-27T14:36:53+00:00] FATAL: Please provide the contents of the stacktrace.out file if you file a bug report
[2016-05-27T14:36:53+00:00] ERROR: No such file or directory - /sbin/lvm
[2016-05-27T14:36:53+00:00] FATAL: Chef::Exceptions::ChildConvergeError: Chef run process exited unsuccessfully (exit code 1)

So I think we should install the lvm package and ruby-lvm gems inside the gluster::server_install.rb recipe and include a LVM cookbook >= 1.5.1 to have the LWPR's accessible.

@shortdudey123
Copy link
Owner

shortdudey123 commented May 27, 2016

lvm::default is run before gluster::volume_extend per gluster::server which installs the gems but not the package yet is what you mean?

@dpattmann
Copy link
Contributor Author

dpattmann commented May 30, 2016

Yes. 😉

The lvm::default recipe is included before gluster::volume_extend but that doesn’t mean that chef is going to execute all resources at this time. Chef sometimes doesn’t care about the order of included recipes when you include them from different cookbook. 😞

...
...
...

I made some more tests and I think we must install the lvm2 package during the compile phase like the ruby lvm gems. 😒

@dpattmann
Copy link
Contributor Author

I updated my pull request to fix this issue. :-)

@shortdudey123
Copy link
Owner

Adding lvm2 to the package list and installing it compile phase seems reasonable

maybe a guard needs to be put on the include_recipe instead of chasing the LVM cookbook around again

Thoughts on ^^?

@dpattmann
Copy link
Contributor Author

That's a good idea, maybe we should check if di-ruby-lvm is installed.

include_recipe 'gluster::volume_extend' if Gem::Specification.find_by_name('di-ruby-lvm')

How about that?

@shortdudey123
Copy link
Owner

That would work since server_setup.rb would call the LVM LWRP and have the gem installed

Add that guard and validate that it works correctly

@dpattmann
Copy link
Contributor Author

I updated my PR :-)

include_recipe 'gluster::volume_extend' if begin
Gem::Specification.find_by_name('di-ruby-lvm')
rescue
Gem::LoadError
Copy link
Owner

Choose a reason for hiding this comment

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

It looks like this will still get passed up and cause the run to bomb out. I think you mean to have Gem::LoadError on the same like as the rescue statement

[2] pry(main)> puts 'pass' if begin
[2] pry(main)*   Gem::Specification.find_by_name('di-ruby-lvm')              
[2] pry(main)* rescue              
[2] pry(main)*   Gem::LoadError              
[2] pry(main)* end              
Gem::LoadError: Could not find 'di-ruby-lvm' (>= 0) among 179 total gem(s)
Checked in 'GEM_PATH=/Users/username/.gem/ruby/2.2.0:/usr/local/var/rbenv/versions/2.2.2/lib/ruby/gems/2.2.0', execute `gem env` for more information
from /usr/local/var/rbenv/versions/2.2.2/lib/ruby/2.2.0/rubygems/dependency.rb:315:in `to_specs'
[3] pry(main)>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are right @shortdudey123, I fixed this error. :-)

@shortdudey123
Copy link
Owner

Can you also add a line to CHANGELOG.md with your PR link and title near the top in the Unreleased section?

@dpattmann dpattmann changed the title Use LVM cookbook in a version lower then 2.0 Fix problems with volume_extend recipe Jun 10, 2016
@dpattmann dpattmann closed this Jun 10, 2016
…end recipe unless di-ruby-lvm gem is installed
@dpattmann dpattmann reopened this Jun 10, 2016
@shortdudey123
Copy link
Owner

@dpattmann thanks!!

@shortdudey123 shortdudey123 merged commit 128da38 into shortdudey123:master Jun 10, 2016
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.

3 participants