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 mixlib-install while installing / upgrading packages from omnitruck. #68

Merged
merged 3 commits into from Dec 1, 2015

Conversation

Projects
None yet
4 participants
@sersut
Contributor

sersut commented Nov 24, 2015

This PR removes the custom omnitruck logic and uses mixlib-install in order to do installation from omnitruck.

I have restored Berksfile and the chef_zero provisioner in order to be able to run kitchen tests.

/cc: @schisamo, @jtimberman, @patrick-wright

This goes with: chef/mixlib-install#23

@wrightp

This comment has been minimized.

Show comment
Hide comment
@wrightp

wrightp Nov 24, 2015

Contributor

👍

Contributor

wrightp commented Nov 24, 2015

👍

Show outdated Hide outdated Berksfile
Show outdated Hide outdated libraries/helpers.rb
@schisamo

This comment has been minimized.

Show comment
Hide comment
@schisamo

schisamo Dec 1, 2015

Contributor

👍

Contributor

schisamo commented Dec 1, 2015

👍

@sersut

This comment has been minimized.

Show comment
Hide comment
@sersut

sersut Dec 1, 2015

Contributor

@chef-cookbooks/engineering-services this PR is now ready for final review.

Contributor

sersut commented Dec 1, 2015

@chef-cookbooks/engineering-services this PR is now ready for final review.

@wrightp

This comment has been minimized.

Show comment
Hide comment
@wrightp

wrightp Dec 1, 2015

Contributor

lgtm

Contributor

wrightp commented Dec 1, 2015

lgtm

Show outdated Hide outdated .kitchen.yml
Chef::Log.debug("Installing #{gem_name} v#{gem_version} from Rubygems.org")
chefgem = Chef::Resource::ChefGem.new(gem_name, run_context)
chefgem.version(gem_version)
chefgem.run_action(:install)

This comment has been minimized.

@jtimberman

jtimberman Dec 1, 2015

Member

why can't we use the chef_gem resource here?

@jtimberman

jtimberman Dec 1, 2015

Member

why can't we use the chef_gem resource here?

This comment has been minimized.

@sersut

sersut Dec 1, 2015

Contributor

We would like to install the gem at compile time so it is just a syntax issue here. Didn't want to switch form Ruby to recipe syntax (even though same 😄)

@sersut

sersut Dec 1, 2015

Contributor

We would like to install the gem at compile time so it is just a syntax issue here. Didn't want to switch form Ruby to recipe syntax (even though same 😄)

This comment has been minimized.

@jtimberman

jtimberman Dec 1, 2015

Member

mm, I think we get the compile_time by doing the compile_time true property like we did before. I just don't super dig mixing the syntax with the resource DSL we use elsewhere in these libraries.

@jtimberman

jtimberman Dec 1, 2015

Member

mm, I think we get the compile_time by doing the compile_time true property like we did before. I just don't super dig mixing the syntax with the resource DSL we use elsewhere in these libraries.

require 'mixlib/versioning'
def ensure_mixlib_install_gem_installed!
node.run_state[:mixlib_install_gem_installed] ||= begin # ~FC001
install_gem_from_rubygems('mixlib-install', '0.8.0.alpha.0')

This comment has been minimized.

@jtimberman

jtimberman Dec 1, 2015

Member

I presume the version is specified because we're using a prerelease gem? Do we want to support options in the install_gem_from_rubygems and pass in --pre? Not sure what the best way to handle this is - I don't know if we want to hardcode an alpha version here. When would we update it? Why can't we release 0.8.0 and use the latest?

@jtimberman

jtimberman Dec 1, 2015

Member

I presume the version is specified because we're using a prerelease gem? Do we want to support options in the install_gem_from_rubygems and pass in --pre? Not sure what the best way to handle this is - I don't know if we want to hardcode an alpha version here. When would we update it? Why can't we release 0.8.0 and use the latest?

This comment has been minimized.

@sersut

sersut Dec 1, 2015

Contributor

mixlib-install is getting a main write over so we don't want to make a full release before the most of it is complete.

We can pass --pre now but I am OK to go with this until we make the mixlib-install release for good and remove the specified version altogether.

@sersut

sersut Dec 1, 2015

Contributor

mixlib-install is getting a main write over so we don't want to make a full release before the most of it is complete.

We can pass --pre now but I am OK to go with this until we make the mixlib-install release for good and remove the specified version altogether.

This comment has been minimized.

@jtimberman

jtimberman Dec 1, 2015

Member

If we can remove it altogether then 👍 😄 🍰

@jtimberman

jtimberman Dec 1, 2015

Member

If we can remove it altogether then 👍 😄 🍰

compile_time true
def ensure_mixlib_versioning_gem_installed!
node.run_state[:mixlib_versioning_gem_installed] ||= begin # ~FC001
install_gem_from_rubygems('mixlib-versioning', '1.1.0')

This comment has been minimized.

@schisamo

schisamo Dec 1, 2015

Contributor

I wonder if we should expose some attributes for the gem version (and optional git ref) like we do in artifactory-pro:
https://github.com/chef-cookbooks/artifactory-pro/blob/master/attributes/client.rb#L20-L51

This would allow you to rev the version of the gems in cookbooks that consume this library cookbook.

@schisamo

schisamo Dec 1, 2015

Contributor

I wonder if we should expose some attributes for the gem version (and optional git ref) like we do in artifactory-pro:
https://github.com/chef-cookbooks/artifactory-pro/blob/master/attributes/client.rb#L20-L51

This would allow you to rev the version of the gems in cookbooks that consume this library cookbook.

sersut pushed a commit that referenced this pull request Dec 1, 2015

Serdar Sutay
Merge pull request #68 from chef-cookbooks/sersut/GS-21
Use mixlib-install while installing / upgrading packages from omnitruck.

@sersut sersut merged commit 168412f into master Dec 1, 2015

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@sersut sersut deleted the sersut/GS-21 branch Dec 1, 2015

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