saltstack / salt

Software to automate the management and configuration of any infrastructure or application at scale. Get access to the Salt software package repository here:
https://repo.saltproject.io/
Apache License 2.0
14.15k stars 5.47k forks source link

Question about Solaris pkg module #2108

Closed rrtj3 closed 12 years ago

rrtj3 commented 12 years ago

So, I've started working on a Solaris 'pkg' module and I have some questions about how I should handle some issues....

Basically, the package management sucks big time on solaris. It does not support any dependency handling and does not support the concept of a network repository. Up until solaris 11 anyway. Solaris 11 finally supports the concept of a network repository and dependency handling. (But, alas using a completely different pkg format, IPS. They do still support pkg format packages too though.)

So, with no concept of a "network repo" I need to be able to pass in either a http url to a specific package or a local path to a package. I think it would be cool to be able to have a salt:// path with a package so the package would automatically gets transferred to the client but I'm not sure how that would work out.... thoughts?

Also, to automate the installation or remove of a file, an "admin" file needs to be fed to the pkgadd/pkgrm programs. We could either just require the solaris admin to have one of those in place ahead of time or the mores slick way would be for us to automatically take care of that ahead of time and write that file out to some temp directory and then point pkgadd/pkgrm at it.... Thoughts?

Lastly, I don't believe that pkgadd understands the concept of a "newer" package. This combined with the fact that there is no network repo I'm not sure how we can make the "latest" pkg state feature work.

If it doesn't make sense for a pkg module function to exist in the solarispkg, for example "compare_version" and "available_versions", should I just remove the functions or just have a "pass" in them. I guess what I'm trying to get at is that I want it to work with the salt states feature but I don't think we can do all of the features.

Sorry if this is a bit much, I'll work on the module some more and submit it for review/comments as I get closer, but any suggestions on the above would be helpful.

Thanks

UtahDave commented 12 years ago

As far as the "latest" pkg state feature, could you compare the installed version to what's available on your http:// or salt:// version?

thatch45 commented 12 years ago

I think we can make all of this happen, well, maybe not pkg.latest, but I have a solution for that.

The pkg.installed and latest functions accept _kwargs,and the _kwargs that are passed to the state function get passed to the module function. This means that all arguments that are declared in the sls file get passed in.

Next the file.cache function can handle salt:// http:// and ftp:// connections, so the thing to do is this:

The sls would look like this:

bill_joy_vi:
  pkg.installed:
    - source: salt://pkgs/bill_joy_vi.pkg

so in the install function:

def install(name, **kwargs):
    pkg_file = __salt__['file.cache'](kwargs['source'])
    install_pkg(pkg_file)

The next trick, since latest just won't apply, we just redirect it to installed:

def latest(name, **kwargs):
    return install(name, **kwargs)

As for the admin file, we have one of two choices. Since I don' t know what the admin file needs to look like...

  1. Genrate the admin file, require more kwargs to be filled out in the sls file to make it happen when we can't just divine what needs to be there
  2. Require another source argument, admin_source, that points to the admin file.

Did I miss anything? :)

rrtj3 commented 12 years ago

Thanks Thomas. This is exactly what I was looking for. This is a big help.

@UtahDave I'll look at this a bit more but I'm not sure if this is all that feasible with sun packaging. Few reasons, I'm not sure if you can see what version a uninstalled package is. I'll look into that more though. The other reason is that the versioning scheme is horrendous. For example, here is what versions of solaris packages look like:

{'ppt05.pvt.hawaii.edu': {'CSWbash': '4.2.37,REV=2012.08.24', 'CSWbdb47': '4.7.25,REV=2009.10.18_rev=p4', 'CSWbdb48': '4.8.30,REV=2010.12.06_rev=p0', 'CSWcacertificates': '20120511,REV=2012.05.11', 'CSWcas-cpsampleconf': '1.42,REV=2010.11.26', 'CSWcas-cptemplates': '1.45,REV=2011.08.28', 'CSWcas-crontab': '1.48,REV=2012.09.02', 'CSWcas-etcservices': '1.42,REV=2011.02.16', 'CSWcas-etcshells': '1.45,REV=2011.07.12', 'CSWcas-inetd': '1.42,REV=2010.11.26', 'CSWcas-initsmf': '1.44,REV=2011.04.21',

I briefly looked at how puppet's sun package provider handles this and they also don't look like they handle versions. On "update" it seems they just blindly uninstall and reinstall.... I've got to look at this a bit more though. If you have any ideas I'm open though :)

Thanks

thatch45 commented 12 years ago

Yes, those versions are hairy! And as far as update goes, since we can't check uninstalled versions then it would probably just verify that the package is installed

rrtj3 commented 12 years ago

So, I'm working on the install function and I have something like this:

def install(name, **kwargs):
     pkgfile = __salt__['file.cache'](kwargs['source'])

     default_admin_file = "mail=
                          instance=overwrite
                          partial=nocheck
                          runlevel=nocheck
                          idepend=nocheck
                          rdepend=nocheck
                          space=nocheck
                          setuid=nocheck
                          conflict=nocheck
                          action=nocheck
                          basedir=default"

    if kwargs['admin_source']:
        adminfile = __salt__['file.cache'](kwargs['admin_source'])
    else:
        adminfile = default_admin_file

    cmd = '/usr/sbin/pkgadd -n -a {0} -d {1} \'all\''.format(adminfile, pkgfile)
    __salt__['cmd.retcode'](cmd)

I'd like to allow people to provide an alternate admin file should they need it but provide a default one that should work for most cases. My question is what is the best way I can "write" this default_admin_file variable out to a file on the machines so they can reference it on the pkgadd command? Note: I realize the default_admin_file variable would not work in it's current form, but I wanted to get the idea across.

Thanks for any help.

thatch45 commented 12 years ago

This is looking spot on. the thing to do is use the tempfile module in python to save the default_admin_file string. You could also accept arguments for the variables in the admin file via kwargs:

    instance=kwargs.get('instance', 'overwrite')

So the admin file args can be passed in OR they can supply an admin file

All in all though, this looks great, just look at tempfile and I think you will be dead on!

rrtj3 commented 12 years ago

Cool. Thanks yet again. I'll try to incorporate the ability for them to pass arguments to the adminfile as well as just provide a whole admin file as well.

thatch45 commented 12 years ago

Awesome, I will need to buy you lunch sometime for your help!

rrtj3 commented 12 years ago

Sure :) But I'd have to buy you a car or something to make the exchange even though... :) (Thanks for salt.)

Really, I'm looking forward to getting solaris support at a good place so I can start shifting off of puppet and onto salt. Work and life are busy though so trying to fit in salt work where I can.

thatch45 commented 12 years ago

well, I appreciate it a great deal! Hopefully I will have some Solaris servers soon to work with, the closest I have are smartos servers on Joyent cloud.

rrtj3 commented 12 years ago

After the pkg module is done I'll start working on either modifying the current user and group modules to work with solaris or creating new modules if the differences are large. (I notice that freebsd created new user/group modules.) Then the cron module will need a bit of work. I think that will be a pretty good base and I'll be able to start looking to use it on our solaris boxes.

In regards to SmartOS (and other solaris derivatives) a lot of the work that we're doing here (e.g. smf, user, group, cron) should "just work" on those platforms as well. SmartOS will probably need another pkg module since I think they use pkgsrc and/or it's binary equiv. pkgin.

thatch45 commented 12 years ago

Yes, I had to write another FreeBSD module for users/groups to use the pw command, I imagine there will be similar needs on other unicies. My understanding is that smartos uses pkgsrc, but I have not gotten that far quite yet :)

rrtj3 commented 12 years ago

So, still plugging away at the solaris pkg module but I'm trying to figure out which function in the 'cp' module I should be using to transfer the pkg file to the minions. I thought I should use the cp.cache_file function but it's not seeming to behave how I thought it would. I'm guessing my thinking is off.

When I test this from the master:

salt <hostname> cp.cache_file salt://tmp/ZABagent2.pkg

I can't find the file anywhere on the minion. Now if I add a third slash, which I did by accident, like so:

salt <hostname> cp.cache_file salt:///tmp/ZABagent2.pkg

the file gets transferred to the minion's /tmp dir.

So I guess I have two questions :)

Thanks for any help in explaining this a bit.

Romeo

rrtj3 commented 12 years ago

Playing with the get_url function I realize that I need the three /// slashes. Makes sense. So I guess the cache_file function is supposed to cache the file on the minion in the same directory that it exists on the master.

That leads me to think I should be using the get_url or get_file function to transfer the solaris pkg files to the minions. In regards to not having the package files retransferred on every run, would you mind letting me know how I should attack that? If you can just point me in the right direction that'd be great. Thanks again.

thatch45 commented 12 years ago

The function cp.cache_file function is probably the way to go, it will return a string with the location of the file. The file will be downloaded into the file cache. If the function returns an empty string then the file failed to download. With the location you should be able to use the downloaded file without issue

rrtj3 commented 12 years ago

Hmm, somethings odd then. If I run this on the master:

salt <hostname> cp.cache_file salt:///var/ZABagent2.pkg

It creates an empty file in /var on the minion. The second time (with the empty file still there) the file gets "populated" with it's data. This is reproducible everytime. I think I'll upgrade my master to 0.10.3 and see if that helps.

Also, is cp.cache_file supposed to put the file in the same place on the minion that is exists on the master?

thatch45 commented 12 years ago

no, only use two slashes, I am concerned though that adding a slash seems to create such a strange issue.

rrtj3 commented 12 years ago

Hmm, with the two slashes I get the empty return value. I'll upgrade my master to 0.10.3 and try again. If it still happens I'll try running with debug on and in the foreground.

thatch45 commented 12 years ago

Where on the master are the files you are trying to download? They should be in paths under the file_roots options. So if the file_roots option looks like this:

file_roots:
  base:
    - /srv/salt

and the file you wish to download is "/srv/salt/cheese" then you would pass in: salt://cheese

Are you telling me that with a third slash it downloads the file from the master outside of the file_roots option?

rrtj3 commented 12 years ago

Yes, that is what I'm saying. I didn't realize that the files had to be within the file_roots. I was just pointing at that where ever they were on the filesystem. So it does look like the third slash is allowing downloads outside of the file_roots.

rrtj3 commented 12 years ago

I'm happy that you probably just solved my problem but I think I may have introduced a larger one.

rrtj3 commented 12 years ago

I've verified that a salt-call from the client is able to do a cp.get_file for any file on the master if three ///'s are used.

thatch45 commented 12 years ago

ok, I actually think this should be an easy fix, I am on it!

thatch45 commented 12 years ago

16b41facd5829433435adcb0acdea863fd2ab59f Should fix this issue

rrtj3 commented 12 years ago

That's great! You guys are amazing. Also having the files in the file_root works as expected. I should have figured, but my mental model is still developing from a puppet-esque architecture. Thanks much.

thatch45 commented 12 years ago

I am glad it was such an easy fix!

rrtj3 commented 12 years ago

Before I go digging around a bunch I just want verify that I'm not missing something obvious. I have the solaris pkg module almost done and am testing it out. I can run a:

salt <hostname> pkg.install ZABagent2.pkg source='salt://files/pkgs/solaris/ZABagent2.pkg'

and that works just fine.

When I try this in a sls file:

zabbix:
  pkg.installed:
    - source: salt://files/pkgs/solaris/ZABagent2.pkg

I'm getting this error:

local:
----------
    State: - pkg
    Name:      zabbix
    Function:  installed
        Result:    False
        Comment:   An exception occured in this state: Traceback (most recent call last):
  File "/opt/csw/lib/python/site-packages/salt/state.py", line 860, in call
    *cdata['args'], **cdata['kwargs'])
  File "/opt/csw/lib/python/site-packages/salt/states/pkg.py", line 87, in installed
    **kwargs)
TypeError: install() takes exactly 1 non-keyword argument (2 given)

        Changes:   

Am I supposed to pass a kwarg in a sls file differently? Or is this an issue with the state that I should track down?

Thanks.

thatch45 commented 12 years ago

Yes, it looks like your pkg.install module function does not accept **kwargs, if you don't follow me give me a link to your module and I will take a look

rrtj3 commented 12 years ago

Hi, thanks. Here the link:

https://github.com/romeotheriault/salt/blob/add-solaris-pkg-module/salt/modules/solarispkg.py

thatch45 commented 12 years ago

At first glance this looks right, lemme look more

thatch45 commented 12 years ago

On your install function, add "refresh=False" as the second argument, line 102

rrtj3 commented 12 years ago

Bingo! It's beautiful when it work :) Thanks. I'm getting pretty close to submitting this in a pull request but there are basically two things left that I wanted to talk to you about:

def latest(name, **kwargs):
    return install(name, **kwargs)

It looks like that would require some change to the pkg state. I'm wondering if instead it might make sense to have the available_version function in the pkg module just always return the version that's installed? That way we don't have to have some special case for solaris packages in the pkg state. Thoughts?

Thanks again for your help.

thatch45 commented 12 years ago

Yes, you are spot on, the available_version function should be defined in your module and it can just call the version function. Just like how the purge function works in your module :)

rrtj3 commented 12 years ago

Ok great, that's what I'll do.

rrtj3 commented 12 years ago

Since this module is now submitted, I'll close this for now. Thanks.

thatch45 commented 12 years ago

Heh, yes! Thanks