replies below (many comments elided with "...") ed
On Fri, Apr 15, 2011 at 04:22:39PM +1200, Tim Foster wrote: > On Thu, 2011-04-14 at 19:40 -0700, Edward Pilatowicz wrote: > > On Wed, Mar 30, 2011 at 09:36:05PM -0700, Brock Pytlik wrote: > > > Here's the system repository work that Tim and I have been working > > > on for quite a while now. > > > http://cr.opensolaris.org/~bpytlik/ips-sysrepo-v1/ > > > > > > > more comments below. > > Thanks for taking a look Ed, > > > ---------- > > src/pkg/manifests/package%2Fsysrepo.p5m > > src/util/apache2/sysrepo/logs/access_log > > src/util/apache2/sysrepo/logs/error_log > > > > - hm. it seems weird to me to deliver empty log files that will get > > appended to. (that said, i don't know if this is something that we > > recommend to folks packaging up other services.) > > Right - Apache wants any configured log files to exist before it'll run. > > I could simply touch them as part of the start method script, instead of > delivering empty stubs, but felt that having some means for an > administrator to 'pkg search' for a location for sysrepo logs would be > neat. > sounds good to me. > > ---------- > > src/svc/svc-pkg-sysrepo > > src/svc/pkg-sysrepo.xml > > ... > > - why are we bothering to support prefork and worker type apache > > deployments. can't we just decide on one and use that? > > That's in there now, because I haven't yet investigated which is > preferable for the sysrepo service :-( > > I'm not sure how to answer questions like "What size should our cache > be?", "Disk proxy cache or memory cache?", "How many workers/threads > should we use?" > > Those are things that are really going to depend on the number of > clients using the system publisher per-site, and how much pkg traffic is > going through it. > > [ I'm delivering our mako http.conf template with a preserve attribute > for this reason ] > > Hopefully over time, we'll work out what the best default values are - I > tried to come up with sensible defaults, but if there are any apache > experts in the room, I'd welcome their input. > i guess my point is that i'd rather not make this a user configurable option now (which it seems to be since the files are marked as editable). if we discovere performance issues we can do some additional work and change the setting if we want. > > ---------- > > src/svc/svc-pkg-sysrepo > > ... > > - this seems kinda weird: > > /usr/bin/su pkg5srv > > > > why isn't this whole service running as the pkg5srv user? (also, if > > the sysrepo httpd process is writing to the cache dir doesn't it need > > to be running as the same user as the htcacheclean process? > > That's due to the way apache starts. We start as root because we're > looking to use a low-numbered port (1008) Apache then changes ownership > and runs the httpd processes as the user specified in the > sysrepo_httpd.conf file, with any proxy cache files being owned by that > user too. > ah. i didn't realize that apache changes it's user. a comment to this effect would be good. also, fyi, we don't need to be root to access low number ports. in an smf profile we can just specify the pkg5srv user and the priv required to access low numbered ports. (you don't have to deploy this way i just wanted to mention it to make sure you're aware of the option.) > I felt that starting htcacheclean as that user was preferable to it > being a process that runs as root. > agreed. > > - given that we're using apache as an embedded server i don't think we > > should be sourcing ${APACHE_ETC_ROOT}/envvars (since it in turn > > sources /etc/apache2/2.2/envvars which presumably exists to configure > > the the general apache server, not our embedded one.) > > We can't avoid that if we're using the stock copy of apachectl, which > sources /usr/apache2.2/bin/envvars which in turn > sources /etc/apache2/2.2/envvars, unless we ship our own (essentially > duplicate) version of the apachectl script. > i just looked at apachectl. i'd say we don't bother with it since we dont use the majority of stuff in it. also, it touches stuff in /var/run/apache2/2.2 which we don't want. given that we only ever use the "start|stop|graceful" commands, it seems that the only functionality we want from this script is: ULIMIT_MAX_FILES="ulimit -S -n `ulimit -H -n`" $HTTPD -k $ARGV so let's just invoke those two commands directly. > But you do bring up a good point: elsewhere in the sysrepo_httpd.conf, > we share configuration with /etc/apache2.2 by looking > at /etc/apache2/2.2/conf.d/modules-64.load, so that's something I need > to fix too. Driving a bigger wedge between /etc/apache2 and the system > repository is needed. > really, the only thing we should be using from apache2 is the httpd binary and the stock configuration. ie, we should avoid anything in /etc/apache2. > > ---------- > > src/util/apache2/sysrepo/envvars > > > > - this file doesn't deliver any content and we always update this file > > from the smf service start service. so why bother delivering this > > file? why not just always overwrite it from the smf start scrit? > > The intent was to allow system administrators more flexibility in > modifying the apache configuration. The start method script modifies > any existing file, rather than clobbering it entirely, mirroring the way > the current apache22 service works. > my thoughts here are that apache is really an implementation detail of the sysrepo service, and until we get more experience with it i don't see any reason to allow customers to tweak with the internals of the system repo. (and even when we do decide that we want to expose configuration variables for the system repo to say tune perf in different environments, i'd rather expost those tunables via sfm properties instead of telling users to edit apache config files.) > > ---------- > > src/util/apache2/sysrepo/sysrepo_publisher_response.mako > > ... > > - the XXX comment says we might not want to expose the source URIs, but > > if i access syspub/0 i can see them there anyway as the "proxied_uris" > > and "origins". so it seems we're already exposing this information > > via additional interfaces? if we have to expose this information then > > remove the XXX comment. > > That's not the same thing - in particular, this is for file:// > repositories being proxied by the system publisher. > > For example, in the syspub/0 response you would see a URI like: > > http:///test12/a7999d8ab1883a309202cf3fa8393025b292ae42 > > but the system publisher "publisher/0" response for that repository > exposes that file:/// URI from the global zone: > > ---- > { > "packages": [], > "publishers": [ > { > "alias": null, > "intermediate_certs": [], > "name": "test12", > "packages": [], > "repositories": [ > { > "collection_type": "core", > "description": "This is an automatic response. This publisher is > generated automatically by the IPS system repository, and serves content from > the file-based repository file:///tmp/ips.test.14490/repo_contents2", > "legal_uris": [], > "mirrors": [], > "name": "IPS System Repository: test12", > "origins": [], > "refresh_seconds": null, > "registration_uri": "", > "related_uris": [] > } > ], > "signing_ca_certs": [] > } > ], > "version": 1 > } > ---- > > The reference to the file:// repo needs to be removed if we're to avoid > exposing global-zone paths to non-global zones. > > It turns out, that this template is actually unnecessary for standard > system-publisher client access: the pkg(1) client, when when using the > system publisher, obtains all of its publisher configuration from the > "syspub/0" response now. > > This code is there because I was testing the sysrepo server-side before > I had Brock's client side changes, so was using the sysrepo as a > standard pkg(5) server and needed the publisher/0 response to work for > all proxied URLs, be they file:// or http*:// ones in order to install > images. > > [ I'd like to keep it around in the meantime if nobody objects, as it > makes testing the server code a little more straightforward, and has > some interesting possibilities for the future (like using the system > publisher as one-stop proxy for common groups of pkg publishers) ] > sure. in that case can we just not install it by default? perhaps have it installed by the pkg developer package? ... _______________________________________________ pkg-discuss mailing list [email protected] http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
