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 lvm >= instead of locking it. #62

Closed
rshade opened this issue Jan 11, 2017 · 10 comments
Closed

use lvm >= instead of locking it. #62

rshade opened this issue Jan 11, 2017 · 10 comments

Comments

@rshade
Copy link
Contributor

rshade commented Jan 11, 2017

from @Stromweld
That is good to hear, I disagree on the >= being too open ended part. Most community cookbooks especially ones straight from chef that is how they do their dependancies. This cookbook is the one that has always given me the most trouble after our monthly patches if the month happens to have an lvm update, I've ran into issues where the lvm cookbook has been updated with new lvm gem versions to support the new lvm version, but this cookbook was restricting lvm to an older version and not allowing the new gems to be installed in the latest lvm cookbook.

Cookbooks are built with a purpose, even with breaking changes the end result of the cookbook should still be the same. They may have changed how they get to the end result. If the changes do happen to break this cookbooks functionality then it makes sense to pin it to the last working version till this cookbook can be updated to fix the issue. You especially shouldn't have to do releases to support the latest lvm cookbook when they simply increment the minor version number due to a new feature release that doesn't affect any existing functionality.

Here's examples of dependancies from several cookbooks owned by chef themselves where they are only using >=:
https://supermarket.chef.io/cookbooks/chef-client#dependencies
https://supermarket.chef.io/cookbooks/yum-chef#dependencies
https://supermarket.chef.io/cookbooks/chef_nginx#dependencies
https://supermarket.chef.io/cookbooks/chef_slack#dependencies
https://supermarket.chef.io/cookbooks/chef-server#dependencies

I know this is kind of a long post and I'm not totally sure how the sound of it comes off. Please do not take it as me being combative or aggressive. I'm just laying out my thoughts and what I've seen that seems to be best practice from most community cookbooks in hopes to change your mind on the >= being too loose. I've had this debate on a couple of other popular cookbooks and after they have made the change they haven't had any issues with their dependancies updates. This is a great cookbook and with most of our infrastructure in AWS we rely heavily on this cookbook in one of our base cookbooks so when we do run into issues with this cookbook and the restrictive lvm dependency setting quite a few of our linux chef-clients start failing to converge until it is fixed and pushed out to the community supermarket.

@rshade
Copy link
Contributor Author

rshade commented Jan 11, 2017

@Stromweld I figure the best way is to open an issue on this. Once we get done with our chef12 migration. I will look at release the 3.0 with >= 4.0 if I can get a general consensus.

@Stromweld
Copy link
Contributor

New issue makes sense. Thanks for keeping an open mind. Look forward to see what others think too.

@rshade
Copy link
Contributor Author

rshade commented Jan 11, 2017

per @tas50 from chef:

Restrictive constraints in community cookbooks cause dep hell later

[11:35]
It's been a big problem in the past

[11:35]
Setting a floor has worked well

[11:35]
Users can set the ceiling in their env

rshade added a commit that referenced this issue Jan 11, 2017
@rshade rshade self-assigned this Jan 11, 2017
@rshade
Copy link
Contributor Author

rshade commented Jan 25, 2017

@Stromweld can you test: #61, and let me know how that works?

@Stromweld
Copy link
Contributor

Sure

@Stromweld
Copy link
Contributor

Everything worked as expected. knifed a new m3.xlarge server and it created a volume with the ephemeral drives raided together and mounted it.

@Stromweld
Copy link
Contributor

Stromweld commented Jan 25, 2017

This was using ephemeral_cookbook 3.0.0 here and lvm cookbook 4.0.5 which uses the new chef-ruby-lvm-attrib gem.

@Stromweld
Copy link
Contributor

Here's the output block from the chef-client run while knifing a new server on AWS.

172.21.16.119 Recipe: lvm::default
172.21.16.119 * yum_package[lvm2] action nothing (skipped due to action :nothing)
172.21.16.119 Recipe: ephemeral_lvm::default
172.21.16.119 * log[Ephemeral disks found for cloud 'ec2': ["/dev/xvdb", "/dev/xvdc"]] action write
172.21.16.119
172.21.16.119 * ruby_block[vgs command] action run
172.21.16.119 - execute the ruby block vgs command
172.21.16.119 * lvm_logical_volume[ephemeral0] action nothing (skipped due to action :nothing)
172.21.16.119 * lvm_volume_group[vg-data] action create
172.21.16.119 * chef_gem[vg-data_di-ruby-lvm-attrib_removal] action remove (up to date)
172.21.16.119 * chef_gem[vg-data_di-ruby-lvm_removal] action remove (up to date)
172.21.16.119 * chef_gem[chef-ruby-lvm-attrib] action install
172.21.16.119 - install version 0.0.28 of package chef-ruby-lvm-attrib
172.21.16.119 * chef_gem[chef-ruby-lvm] action install
172.21.16.119 - install version 0.2.2 of package chef-ruby-lvm
172.21.16.119 * mount[/mnt] action umount
172.21.16.119 - unmount /dev/xvdb
172.21.16.119 * mount[/mnt] action disable
172.21.16.119 - disable /dev/xvdb
172.21.16.119 * lvm_logical_volume[ephemeral0] action create
172.21.16.119 * chef_gem[ephemeral0_di-ruby-lvm-attrib_removal] action remove (up to date)
172.21.16.119 * chef_gem[ephemeral0_di-ruby-lvm_removal] action remove (up to date)
172.21.16.119 * directory[/mnt/ephemeral] action create
172.21.16.119 - create new directory /mnt/ephemeral
172.21.16.119 - change mode from '' to '0755'
172.21.16.119 - change owner from '' to 'root'
172.21.16.119 - change group from '' to 'root'
172.21.16.119 - restore selinux security context
172.21.16.119 * mount[/mnt/ephemeral] action mount
172.21.16.119 - mount /dev/mapper/vg--data-ephemeral0 to /mnt/ephemeral
172.21.16.119 * mount[/mnt/ephemeral] action enable
172.21.16.119 - enable /dev/mapper/vg--data-ephemeral0
172.21.16.119
172.21.16.119
172.21.16.119 * chef_gem[vg-data_di-ruby-lvm-attrib_removal] action remove (up to date)
172.21.16.119 * chef_gem[vg-data_di-ruby-lvm_removal] action remove (up to date)
172.21.16.119 * chef_gem[chef-ruby-lvm-attrib] action install (up to date)
172.21.16.119 * chef_gem[chef-ruby-lvm] action install (up to date)
172.21.16.119 * mount[/mnt] action nothing (skipped due to action :nothing)
172.21.16.119 * chef_gem[ephemeral0_di-ruby-lvm-attrib_removal] action remove (up to date)
172.21.16.119 * chef_gem[ephemeral0_di-ruby-lvm_removal] action remove (up to date)
172.21.16.119 * directory[/mnt/ephemeral] action nothing (skipped due to action :nothing)
172.21.16.119 * mount[/mnt/ephemeral] action nothing (skipped due to action :nothing)

@rshade rshade closed this as completed in 27cc02d Jan 26, 2017
@rshade
Copy link
Contributor Author

rshade commented Jan 26, 2017

@Stromweld 3.0.0 should be up on supermarket

@Stromweld
Copy link
Contributor

cool thanks

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

No branches or pull requests

2 participants