voxpupuli / puppet-pkgng

A Puppet package provider for FreeBSD's PkgNG package manager.
https://forge.puppet/com/puppet/pkgng
Apache License 2.0
39 stars 33 forks source link

version pinning not working #34

Closed sethlyons closed 9 years ago

sethlyons commented 10 years ago

when i set 'ensure => [version number]' on a package, a package of a different version gets installed. e.g.

package { 'openssl': ensure => '1.0.1_9'; }

however, 1.0.1_11 gets installed. on the next puppet run, puppet returns: Notice: /Stage[main]/Site::Freebsd/Package[openssl]/ensure: ensure changed '1.0.1_11' to '1.0.1_9'

but does not actually downgrade the package.

zachfi commented 10 years ago

Thank you for the report. Does the output happen every run?

I believe the cause is here: https://github.com/xaque208/puppet-pkgng/blob/master/lib/puppet/provider/package/pkgng.rb#L95

We never actually make use of resource[:version] here to set the args of pkg() correctly.

sethlyons commented 10 years ago

Hi -- yes, that output happens on every run.

Thanks.

On Sun, May 4, 2014 at 12:14 AM, Zach Leslie notifications@github.comwrote:

Thank you for the report. Does the output happen every run?

I believe the cause is here: https://github.com/xaque208/puppet-pkgng/blob/master/lib/puppet/provider/package/pkgng.rb#L95

We never actually make use of resource[:version] here to set the args of pkg() correctly.

— Reply to this email directly or view it on GitHubhttps://github.com/xaque208/puppet-pkgng/issues/34#issuecomment-42123717 .

zachfi commented 10 years ago

I'll try to take a look at this soon. Anyone with spare cycles before then should be able to reference the link above to figure out where this is handled.

zachfi commented 10 years ago

I took a brief look at this the other evening. I think I have a fix, but I couldn't get the tests to work like I expected them to, so I can't be sure. I'll might stab it again over the weekend to see where I can get.

zachfi commented 10 years ago

I just remembered about this, so I've still not gotten to the tests. Here is the current state: https://github.com/xaque208/puppet-pkgng/compare/versionfix

sethlyons commented 10 years ago

Thanks for the update. From my testing so far it seems to be working. It looks to be using a version that is older than your master branch. Do you mind merging in those changes and then I can test for you some more?

Thanks.

On Mon, Jun 23, 2014 at 12:29 PM, Zach Leslie notifications@github.com wrote:

I just remembered about this, so I've still not gotten to the tests. Here is the current state: https://github.com/xaque208/puppet-pkgng/compare/versionfix

— Reply to this email directly or view it on GitHub https://github.com/xaque208/puppet-pkgng/issues/34#issuecomment-46868272 .

zachfi commented 10 years ago

I need to get the spec tests working before I'm willing to merge to master. If you are an r10k user, you can just pin to that branch for now and I'll update here when I've merged to master. I'll try to give some attention here this week.

sethlyons commented 10 years ago

That sounds great. Could you merge the master branch into versionfix so that versionfix has everything up-to-date plus the version fixes? I pinned r10k to versionfix, but I'd love to have ae5b7d75ac3e091a02fe073d2b21323207a33bff back as well.

Thanks!

On Mon, Jun 30, 2014 at 10:16 AM, Zach Leslie notifications@github.com wrote:

I need to get the spec tests working before I'm willing to merge to master. If you are an r10k user, you can just pin to that branch for now and I'll update here when I've merged to master. I'll try to give some attention here this week.

— Reply to this email directly or view it on GitHub https://github.com/xaque208/puppet-pkgng/issues/34#issuecomment-47536581 .

zachfi commented 10 years ago

I've just rebased to bring in all of the master commits. That should get you going.

sethlyons commented 10 years ago

Awesome, thanks!

On Mon, Jun 30, 2014 at 1:04 PM, Zach Leslie notifications@github.com wrote:

I've just rebased to bring in all of the master commits. That should get you going.

— Reply to this email directly or view it on GitHub https://github.com/xaque208/puppet-pkgng/issues/34#issuecomment-47557558 .

sethlyons commented 10 years ago

my testing is going well. i found one issue, though. we use tac_plus (net/tac_plus4), and their version numbers have letters. the current version is tac_plus-F4.0.27.a. that doesn't match the regex in https://github.com/xaque208/puppet-pkgng/blob/versionfix/lib/puppet/provider/package/pkgng.rb#L46, so puppet thinks it needs to install the package on every run.

zachfi commented 10 years ago

If you can provide fixture data, we can update the test to support it.

sethlyons commented 10 years ago

from my testing so far, if you update the regex from: \w+-\d.\s\w\/\w.*

to: \w+-\w?\d.\s\w\/\w.*

it will match tac_plus-F4.0.4.27a as well as the rest of the packages i have installed.

zachfi commented 10 years ago

Could you send a PR with that update? Also, update the fixture data in https://github.com/xaque208/puppet-pkgng/blob/master/spec/fixtures please. I think https://github.com/xaque208/puppet-pkgng/blob/master/spec/fixtures/pkg.info is the only one required, so just add the line we will be matching with pkg info.

zachfi commented 10 years ago

Oh, and if you do, send the PR to the branch I've got open please, not master.

sethlyons commented 10 years ago

i think instead of attempting to write a regex to parse every possible combination of pkg info -ao, it's better to use pkg query -a "%n %v %o". you can then just split on a space and have all three fields. i made the updates in my local repo, but i'm still working on the tests. if you want to see what i did, it's here: https://github.com/jewjitsu/puppet-pkgng/blob/versionfix/lib/puppet/provider/package/pkgng.rb

once i have the tests working i'll send the PR.

sethlyons commented 10 years ago

even though all the tests passed, it looks like this isn't quite there yet. i just tried to use it on a test host and it didn't know any of my packages were installed. i'll try to figure out where the issue is. sample debug output is below. i'll also try to think of a test for this case.

Debug: Prefetching pkgng resources for package Debug: Executing '/usr/local/sbin/pkg query -a "%n %v %o"' Error: Could not prefetch package provider 'pkgng': undefined method `find' for nil:NilClass Debug: Packagesyslog-ng: {} Notice: /Stage[main]/Site::Freebsd/Package[syslog-ng]/ensure: current_value absent, should be 3.5.4.1 (noop) Debug: /Package[syslog-ng]: The container Class[Site::Freebsd] will propagate my refresh event

sethlyons commented 10 years ago

i updated the way we're sending arguments to pkg query. in the last week or so of testing, this is working now.

zachfi commented 10 years ago

51 Looks to be passing the tests. Can you confirm that the branch is wokring well for your use case?

sethlyons commented 10 years ago

So far so good.

On Tue, Jul 29, 2014 at 11:11 AM, Zach Leslie notifications@github.com wrote:

51 https://github.com/xaque208/puppet-pkgng/pull/51 Looks to be

passing the tests. Can you confirm that the branch is wokring well for your use case?

— Reply to this email directly or view it on GitHub https://github.com/xaque208/puppet-pkgng/issues/34#issuecomment-50489735 .

zachfi commented 9 years ago

Code above merged.