If a user wants to know "what manifest will a machine get?", they shouldn't have to review a list of manifest-and-criterias - it is very tedious and error-prone. Rather there should be a query command with, manually providing the criteria values of interest (multiple manifests might match), or alternatively a "no-install" boot.
As for the output of installadm -m -n <svcname> it would be good to group the criteria by their selectiveness, unique criteria (mac, IP) first followed by generic criteria (arch, memory, platform, ...) Frank On 12/9/2009 7:39 PM, Clay Baenziger wrote: > Hi John, > I'm really happy you pointed me to the AI design doc update as I > hadn't noticed the installadm list difference! On big concern I have > is that if an admin is trying to understand why a machine is getting a > particular manifest there's no means for comparison in the proposed > output: > > # installadm list -m -n<svcname> > > Manifest Criteria > > -------- -------- > > Manifest1 arch = SPARC > > mac = C0:FF:EE:C0:FF:EE > > Manifest2 mac = C0:FF:EE:C0:FF:E0 - C0:FF:EE:C0:FF:ED > > Manifest3 arch = AMD64 > > ipv4 = 172.20.48.190 - 172.20.48.255 > > Manifest4 platform = SUNW,Sun-Fire-880 > > ipv4 = 172.20.24.0 - 172.20.48.0 > > mem = 1024 > > > So my question (which I think is the most likely question for this > view) is what manifest will a machine get? For example, with only 4 > manifests can one ascertain what manifest a Sun Fire V880 with 2GB of > RAM and MAC address C0:FF:EE:C0:FF:EE and IP address of 172.20.48.251 > get? (How about if the criteria have a priority; for example what if a > manifest has a machine's exact MAC address and another manifest has > its exact IP address?) > > Versus the old output (which always concerned me as it could grow > unwieldly wide but would provide criterion priority, manifest > instances and a means to compare by each criterion): > > root at jumprope:/tmp# installadm list -cn install_test_ai_x86 > manifest instance arch mac ipv4 > platform mem manufacturer model > -------------- -------- ----- ----------------- --------------- > ---------------- ------- ------------ ------ > manifest1.xml: > 0 sparc C0:FF:EE:C0:FF:EE > - - - - - > C0:FF:EE:C0:FF:EE - - > manifest2.xml: > 0 - C0:FF:EE:C0:FF:E0 > - - - - - > C0:FF:EE:C0:FF:ED - - > manifest3.xml: > 0 amd64 - > 172.020.048.190 - - - - > - 172.020.048.255 - > manifest4.xml: > 0 - - 172.020.024.000 > sunwsun-fire-880 1024 MB - - > - 172.020.048.000 1024 MB > 1 - - > 172.020.024.000 - 1025 MB apple ibook > - 172.020.048.000 2048 MB > > > Also, I think we'd need to think how we want to handle multiple > criteria sets per manifest (referred to as instances by the current > installadm list, delete-service and the AI database)? (See my example > with an Apple iBook as a machine in the criteria for manifest4 which > has instances 0 and 1.) > > Thank you, > Clay > > On Wed, 9 Dec 2009, John Fischer wrote: > >> Clay, >> >> Thanks for the help. I appreciate you taking the time to type this >> up. I'll update the webrev tomorrow morning. >> >> Thanks, >> >> John >> >> Clay Baenziger wrote: >>> Hi John, >>> As we talked about in IRC and some high level thoughts: >>> *Please update your code to be in sync with slim_source (based off >>> the Python 2.6 Python files) >> >> Will do. >> >>> *Please move findTFTProot() from delete_service to >>> installadm_common so that it's more obvious there are multiple >>> consumers. Further it seems, which_arch would be a general >>> function well provided through installadm_common. >> >> Already moved. >> >>> *I'm rather concerned that you're rewriting list-manifest opposed >>> to simply using what's already written. The only bug I see >>> affecting list-manifest is pretty trivial: >>> 4298 installadm list -n should give different output if there is >>> no custom manifest >>> Further, I think this bug: >>> 4646 list: showing added manifest for a non running service. >>> Would probably better be: >>> 4646 list: Should provide warning manifests listed are for a >>> non-running service >>> (as we want to allow publication/deletion/listing of manifests >>> when the service is offline since there's no requirement for the >>> service to be online and this is how one can setup a service to >>> "go live" without affecting other things) >> >> Don't recall why I decided not to mess with list-manifest. I'll >> see if I can find my notes. >> >>> *I think you should take bug 4320 - installadm list -n should use >>> the -c option as default. And you should actually take ownership >>> (and mark as FIXINPROGRESS 4298 - installadm list -n should give >>> different output if there is no custom manifest. >> >> Defect 4320 does not match the design specification for list. >> I'll talk to Ethan about it. The design specification is located >> at: >> >> >> http://hub.opensolaris.org/bin/download/Project+caiman/auto_install/DesigndocdeltaforAISpring2009.2.pdf >> >> >> >> Defect 4298 should report an error if installadm list -m -n service_name >> does not have a custom manifest. This is currently what the list >> subcommand does. >> >>> *Do you have any example output as I can't visualize what this >>> looks like? (Also, have you run the output past Frank as he'd >>> given feedback on list-manifest?) >> >> OK. See the design specification for more detailed output: >> >> Manifest Criteria >> -------- -------- >> devpublisher.xml arch = i86pc >> mac = 01:C2:52:E6:4B:E0 >> ipv4 = 010.000.002.015 >> devpublisher3.xml arch = i86pc >> mac = 01:C2:52:E6:4B:E6 - 01:C2:52:E6:4B:E9 >> devpublisher4.xml arch = i86pc >> ipv4 = 192.168.168.151 >> devpublisher5.xml arch = i86pc >> ipv4 = 192.168.168.251 >> mac = 01:C4:52:E6:4B:E6 - 01:C4:52:E6:4B:E9 >> devpublisher6.xml arch = i86pc >> ipv4 = 192.168.168.012 >> mac = 01:C4:51:E6:4B:E6 - 01:C4:51:E6:4B:E9 >> mem = 2048 MB >> somethingelse.xml arch = i86pc >> mac = 01:C2:E2:E6:4B:E9 >> >> >>> Code bits: >>> list.py: >>> line 189 find_sparc_clients >>> line 337 find_x86_clients >>> I like the layout of these functions, however, couldn't >>> this data be rolled up into a x86_client and sparc_client >>> class to allow caching and easier reuse across all AI >>> components? (This would be very useful for the delete*.py >>> bits) >> >> Hmm... OK. This will take me a little while longer. May not make >> tomorrow's new webrev. >> >>> line 364 get_menu_info >>> Please use the GrubMenu class in installadm_common, unless >>> I'm missing what's special here? >> >> Already saw that one when you asked about moving findTFTPboot() into >> installadm_common.py and have a working list using it. >> >>> line 311 (and elsewhere) >>> Instead of concatenating two paths with a '/' please use >>> os.path.join(). >> >> Will do. >> >>> line 569 >>> "no local service\n" or "no local services\n"? Also (in >>> other places too), please follow PEP8's recommendation to >>> leave operators on the upper line when a line wrap is >>> encountered. And lastly I think you should be lined up >>> with the underscore from above. I think the lines would >>> be: >>> estr = _('%s: error: no local services\n') %\ >>> os.path.basename(sys.argv[0]) >> >> Ah, OK. >> >>> line 628 >>> You're doing an assignment here so rdict and second are >>> the same object: >>> >>> def c(foo): >>> ... b=foo >>> ... del(b[1]) >>> ... >>> >>> a >>> {1: 1, 2: 2, 3: 3} >>> >>> c(a) >>> >>> a >>> {2: 2, 3: 3} >>> You might want to see: >>> http://docs.python.org/library/copy.html >> >> Ah. I see. OK will do. >> >>> line 633: this line assumes that rdict has list objects as its >>> values. Otherwise, one will get an AttributeError likely. If >>> you're assuming that each dictionary only contains lists it could be >>> written for quicker execution, I believe, as: >>> >>> a={1: ['a'], 2: [2], 3: [3]} >>> >>> b={1: ['b'], 'c': [4]} >>> >>> def merge(foo, bar): >>> ... c={} >>> ... c.update(foo) >>> ... c.update(bar) >>> ... for key in set(foo.keys()) & set(bar.keys()): >>> ... c[key]=foo[key]+bar[key] >>> ... return c >>> >>> merge(a,b) >>> >>> {1: ['a', 'b'], 2: [2], 3: [3], 'c': [4]} >> >> OK >> >>> line 809: get_criteria_info: >>> Please use more descriptive variable names, I can't parse >>> what's going on here >> >> Changed criteria to mancriteria, crit to key, tk to tkey, tv dropped >> using svalue. Also modified the function as MAX/MIN code was >> essentially the same. See below: >> >> def get_criteria_info(mancriteria): >> """ >> Iterates over the criteria which consists of a dictionary with >> possibly arch, min memory, max memory, min ipv4, max ipv4, min >> mac, >> max mac, cpu, platform, min network and max network converting it >> into a dictionary with on arch, mem, ipv4, mac, cpu, platform, >> network. Any min/max attributes are stored as a range within the >> new dictionary. >> >> Args >> criteria = dictionary of the criteria >> >> Returns >> dictionary of combinded min/max and other criteria >> >> Raises >> None >> """ >> >> tdict = {'arch':'', 'mem':'', 'ipv4':'', 'mac':'', >> 'platform':'', 'network':'', 'cpu':''} >> >> for key in mancriteria.keys(): >> if not mancriteria[key] or mancriteria[key] == '': >> continue # no criteria for instance key >> >> svalue = AIdb.formatValue(key, mancriteria[key]) >> >> if key.find('MAX') == 0 or key.find('MIN') == 0: >> tkey = key[3:] # strip off the MAX or MIN for a new >> keyname >> if tdict[tkey] != '': # dealing with range >> if tdict[tkey] != svalue: >> if max(svalue, tdict[tkey]) == svalue: >> tdict[tkey] = tdict[tkey]+' - '+svalue >> else: >> tdict[tkey] = svalue+' - '+tdict[tkey] >> else: # first value, not range yet >> tdict[tkey] = svalue >> else: # not a range manifest criteria >> tdict[key] = svalue >> >> return tdict >> >> Hopefully, these changes help. >> >>> line 891: ensure >> >> Several corrected. >> >>> >>> Thank you, >>> Clay >>> >>> >>> On Tue, 1 Dec 2009, John Fischer wrote: >>> >>>> All, >>>> >>>> Updated webrev for this set of changes at: >>>> >>>> http://cr.opensolaris.org/~johnfisc/list >>>> >>>> These changes include feedback during a phone conversation from Ethan >>>> as well as the installadm man page change to document the list >>>> subcommand. >>>> >>>> Thanks, >>>> >>>> John >>>> >>>> John Fischer wrote: >>>>> Ethan, Clay and Sundar, >>>>> >>>>> When you get a chance can you review my webrev at: >>>>> >>>>> http://cr.opensolaris.org/~johnfisc/list >>>>> >>>>> These changes are for the installadm list subcommand fix and >>>>> will address: >>>>> >>>>> 4330 - installadm list should show which server provide the >>>>> install >>>>> services. >>>>> 4113 - nice to have an option to show what service the client is >>>>> using >>>>> 4298 - installadm list -n should give different output if >>>>> there is >>>>> no custom manifest >>>>> 4597 - installadm list: expect usage statement when giving >>>>> service >>>>> name without "-n" flag. >>>>> 4646 - list: showing added manifest for a non running service. >>>>> 5300 - list: does not show informational message for non running >>>>> service. >>>>> 8496 - list: no verbiage indicating that a service does not exist >>>>> when a non-existent service is given. >>>>> 8529 - 'installadm list' command lists same service three times >>>>> 6811 - list: should have similar output between list and list -n >>>>> 8015 - list-manifests output is hard to read >>>>> 9094 - installadm list: prints colons for empty MAC fields, and >>>>> doesn't account for colons or periods in MAC field's width >>>>> 4175 - install list error slips out >>>>> >>>>> The changed and added files are: >>>>> >>>>> usr/src/cmd/installadm/Makefile >>>>> usr/src/pkgdefs/SUNWinstalladm-tools/prototype_com >>>>> usr/src/cmd/installadm/installadm.c >>>>> usr/src/cmd/installadm/installadm.h >>>>> usr/src/cmd/installadm/list.py >>>>> >>>>> These changes also include a change to the Makefile and prototype_com >>>>> file for the delete_service and delete_client to be consistent with >>>>> the other python scripts within the /usr/lib/installadm directory. >>>>> >>>>> The remote service listing is being postponed due to a timing issue >>>>> with multiple remote services that grows with the number of services >>>>> within the domain. >>>>> >>>>> Thanks, >>>>> >>>>> John >>>>> _______________________________________________ >>>>> caiman-discuss mailing list >>>>> caiman-discuss at opensolaris.org >>>>> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss >>>> _______________________________________________ >>>> caiman-discuss mailing list >>>> caiman-discuss at opensolaris.org >>>> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss >>>> >>
