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

(QENG-608) Check_for_package method should not use which command #229

Merged
merged 1 commit into from
Jun 19, 2014

Conversation

anodelman
Copy link
Contributor

  • properly differentiate between packages and commands
  • check_for_package updated to check for the given package name using
    the correct package manager per OS
  • added check_for_command, which uses 'which' to determine if a command
    is currently available

@anodelman
Copy link
Contributor Author

@branan can you look over the solaris/sles code - I don't have a test OS for either or those to do sanity checks on?

@puppetlabs-jenkins
Copy link
Contributor

💚 Test passed.
Refer to this link for build results: http://jenkins-beaker.delivery.puppetlabs.net/job/Beaker%20Specs/201/

result = exec(Beaker::Command.new("rpm -i #{name}"), :acceptable_exit_codes => (0...127))
when /ubuntu|debian/
result = exec(Beaker::Command.new("dpkg -s #{name}"), :acceptable_exit_codes => (0...127))
when /solaris/
Copy link
Contributor

Choose a reason for hiding this comment

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

this is only valid on solaris 11, solaris 10 has different packaging

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Tested on solaris11 and solaris10 - both use pkginfo.

This is a little confusing as to which commands lists which packages (pkginfo vs pkg info):
http://stackoverflow.com/questions/18397029/solaris-11-pkginfo-vs-pkg-info

Are we going to be installing legacy packages or IPS packages?

Copy link
Contributor

Choose a reason for hiding this comment

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

You already switched it, but yeah - I think the right thing to do is IPS on Solaris 11 and legacy on Solaris 10.

And yes, I had my IPS commands and legacy commands backwards in my brain when I made that comment.

@puppetlabs-jenkins
Copy link
Contributor

💚 Test passed.
Refer to this link for build results: http://jenkins-beaker.delivery.puppetlabs.net/job/Beaker%20Specs/204/

@puppetlabs-jenkins
Copy link
Contributor

💚 Test passed.
Refer to this link for build results: http://jenkins-beaker.delivery.puppetlabs.net/job/Beaker%20Specs/210/

result = exec(Beaker::Command.new("zypper se -i --match-exact #{name}"), :acceptable_exit_codes => (0...127))
when /el-4/
@logger.debug("Package query not supported on rhel4")
return False
Copy link
Contributor

Choose a reason for hiding this comment

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

False isn't a class in Ruby. I think you want the keyword false or the class FalseClass, and maybe a test :P

@puppetlabs-jenkins
Copy link
Contributor

💚 Test passed.
Refer to this link for build results: http://jenkins-beaker.delivery.puppetlabs.net/job/Beaker%20Specs/212/

@puppetlabs-jenkins
Copy link
Contributor

💚 Test passed.
Refer to this link for build results: http://jenkins-beaker.delivery.puppetlabs.net/job/Beaker%20Specs/215/

@anodelman
Copy link
Contributor Author

Added spec test coverage.

@puppetlabs-jenkins
Copy link
Contributor

💚 Test passed.
Refer to this link for build results: http://jenkins-beaker.delivery.puppetlabs.net/job/Beaker%20Specs/219/

@puppetlabs-jenkins
Copy link
Contributor

🔴 Test failed.
Refer to this link for build results: http://jenkins-beaker.delivery.puppetlabs.net/job/Beaker%20Combined%20Smoketest/3/

@anodelman
Copy link
Contributor Author

retest this please

@puppetlabs-jenkins
Copy link
Contributor

🔴 Test failed.
Refer to this link for build results: http://jenkins-beaker.delivery.puppetlabs.net/job/Beaker%20Combined%20Smoketest/9/

@justinstoller
Copy link
Contributor

retest this please

@puppetlabs-jenkins
Copy link
Contributor

💚 Test passed.
Refer to this link for build results: http://jenkins-beaker.delivery.puppetlabs.net/job/Beaker%20Combined%20Smoketest/17/

@justinstoller
Copy link
Contributor

So this looks good, but I feel like I shouldn't merge it until I've manually played with it on some exotic platforms (ie, there's changes here that can't really be verified without full level integration tests that I don't believe our smoke tests are sufficient for).

- properly differentiate between packages and commands
- check_for_package updated to check for the given package name using
  the correct package manager per OS
- added check_for_command, which uses 'which' to determine if a command
  is currently available
- added spec test coverage for check_for_package
@anodelman
Copy link
Contributor Author

@justinstoller do you have an ETA here?

@puppetlabs-jenkins
Copy link
Contributor

🔴 Test failed.
Refer to this link for build results: http://jenkins-beaker.delivery.puppetlabs.net/job/Beaker%20Combined%20Smoketest/66/

@anodelman
Copy link
Contributor Author

retest this please

@puppetlabs-jenkins
Copy link
Contributor

🔴 Test failed.
Refer to this link for build results: http://jenkins-beaker.delivery.puppetlabs.net/job/Beaker%20Combined%20Smoketest/68/

@branan
Copy link
Contributor

branan commented Jun 6, 2014

retest this please

@puppetlabs-jenkins
Copy link
Contributor

🔴 Test failed.
Refer to this link for build results: http://jenkins-beaker.delivery.puppetlabs.net/job/Beaker%20Combined%20Smoketest/90/

@anodelman
Copy link
Contributor Author

retest this please

@puppetlabs-jenkins
Copy link
Contributor

🔴 Test failed.
Refer to this link for build results: http://jenkins-beaker.delivery.puppetlabs.net/job/Beaker%20Combined%20Smoketest/120/

@colinPL
Copy link
Contributor

colinPL commented Jun 18, 2014

I have run check_for_package on centos6, sles11, debian6, solaris10, and solaris11 with success.

I checked that the returned boolean matched the output of the package command (if it said nginx was not installed, then the boolean should be false).

@justinstoller
Copy link
Contributor

retest this please

@puppetlabs-jenkins
Copy link
Contributor

🔴 Test failed.
Refer to this link for build results: http://jenkins-beaker.delivery.puppetlabs.net/job/Beaker%20Combined%20Smoketest/131/

justinstoller added a commit that referenced this pull request Jun 19, 2014
(QENG-608) Check_for_package method should not use which command
@justinstoller justinstoller merged commit 7b0efa1 into voxpupuli:master Jun 19, 2014
@justinstoller
Copy link
Contributor

Merged on @colinPL's recommendation :)

@anodelman anodelman deleted the pkg-vs-cmd branch April 14, 2015 20:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants