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

Allow for physical_volumes and volume_groups that change a system lives #30

Merged
merged 2 commits into from
Sep 16, 2013
Merged

Allow for physical_volumes and volume_groups that change a system lives #30

merged 2 commits into from
Sep 16, 2013

Conversation

csschwe
Copy link
Contributor

@csschwe csschwe commented Apr 27, 2013

The issue being addressed is the change of a systems after it is provisioned. Sometimes you have to add disks or modify a server that makes it non standard. Usually this occurs on systems that have a large amount of data.

By adding the unless_vg (physical_volume) and createonly (volume_group) options you can have standard deployments and still support the non standard growth while using puppet-lvm.

lines.split(/\n/).grep(/,#{@resource[:name]}$/).map { |s|
if @resource[:createonly].to_s == "false" || ! vgs(@resource[:name])
lines = pvs('-o', 'pv_name,vg_name', '--separator', ',')
lines.split(/\n/).grep(/,#{@resource[:name]}$/).map { |s|
Copy link
Contributor

Choose a reason for hiding this comment

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

For blocks that span more than one line, it's convention to use do/end. This could be refactored as follows:

lines.split(/\n/).grep(/,#{@resource[:name]}$/).map do |s|
  s.split(/,/)[0].strip
end

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, this is an existing behavior. Ah well, the point still stands, but don't feel compelled to change this. :)

@adrienthebo
Copy link
Contributor

It looks like this is failing in Travis-ci; could you take a look at that? Is https://travis-ci.org/puppetlabs/puppetlabs-lvm/jobs/6675203#L149 a valid failure?

@csschwe
Copy link
Contributor Author

csschwe commented May 21, 2013

I can't tell why https://travis-ci.org/puppetlabs/puppetlabs-lvm/jobs/6675203#L149 is failing. (most likely do to my lack of experience with travis-ci).

The change I made will bypass the pvs if the unlessvg option is set to a VG and the VG exists.

Ok, figured it out, building new spec tests now.

Thanks

@derekivey
Copy link

I'd love to see this merged into the LVM module. We also have this issue. I added another physical volume to one of our instances yesterday to extend a volume group and Puppet keeps complaining about it.

@resource.expects(:[]).with(:name).returns('/dev/sdb')
@provider.expects(:pvs).returns(true)
@provider.should be_exists
end
###
Copy link

Choose a reason for hiding this comment

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

Any chance I can get you to remove this spec test as it's not working?

@csschwe
Copy link
Contributor Author

csschwe commented Sep 15, 2013

Removed spec test that did not work from the code.

apenney pushed a commit that referenced this pull request Sep 16, 2013
Allow for physical_volumes and volume_groups that change a system lives
@apenney apenney merged commit f5e86ab into puppetlabs:master Sep 16, 2013
@csschwe csschwe deleted the createonly-for-vg-pv branch March 7, 2015 23:23
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.

5 participants