Ethan Quach wrote: > > > Frank Ludolph wrote: >> Ethan Quach wrote: >>> >>> >>> Frank Ludolph wrote: >>>> Ethan Quach wrote: >>>>> >>>>>> >>>>>> - The above two imply that criteria could be administered >>>>>> subsequent to a manifest being published. Given that criteria are >>>>>> an external entity from the actual install manifest (the >>>>>> ai_manifest), >>>>>> I think this makes sense. Perhaps we add a couple new >>>>>> subcommands to enable this? (This assumes that the list of >>>>>> 'criteria' >>>>>> is an interface we export, and is not customizable. I didn't see >>>>>> anything in the design doc to say otherwise.) >>>>>> >>>>>> installadm set-criteria -n <svcname> -m <manifest_name> >>>>>> name=value >>>>>> >>>>>> installadm get-criteria -n <svcname> -m <manifest_name> [name] >>> >>> I just realized that we should probably also implement a >>> 'del-criteria' (or 'unset-criteria') subcommand so that users can >>> delete criterias on the command line as well. It would take the >>> same arguments as 'set-criteria' > > >> maybe clear-criteria? > > That's fine by me. > >>> >>>>>> >>>>>> >>>>>> - Be able to view the contents of a published manifest. Perhaps: >>>>>> >>>>>> installadm export-manifest -n <svcname> -m <manifest_name> [-o >>>>>> file] >>>>>> >>>>>> >>>>>> - Being able to edit the attributes of a published manifest would >>>>>> seem like a natural follow-on to the above, but I don't have any >>>>>> evidence yet that this is required. Given the ability to export the >>>>>> contents of a manifest, it should suffice for now to just export a >>>>>> manifest to file, make edits, and re-publish. >>>>> >>>>> If a manifest gets republished (i.e. we readd a manifest to a service >>>>> that already has that manifest name), we'll check if the criterion >>>>> for the already published manifest matches that of the one being >>>>> added. If they differ, ask user which one they want. >>>> It would probably be a good idea to do this only if the current >>>> manifest has been altered using the set-criteria command so that >>>> the user is queried only when appropriate. Perhaps a flag could be >>>> set in the current manifest by the set-criteria command. >>> >>> I don't really see that having edited a criteria by the 'set-criteria' >>> command as being special. My goal of creating subcommands to >>> externally administer the criteria was so that a user could start >>> out with a null criteria, add/remove criteria on the fly, and it >>> resulting in the same thing. > > >> We're trying to warn the user who is unaware of a prior >> 'set-criteria' that is about to be overwritten. > > 'set-criteria' was intended to replace a person having to do -- > export-manifest, edit the file, add-manifest -- so my view of it > is identical to having edited a file per se. > >> If the two sets of criteria are different only because the the >> combined manifest was edited, then the criteria change is explicitly >> intended and we don't have to be concerned about accidentally >> overriding a set-criteria. Warning the user that they edited a file >> is irritating. > > Are you worried about collisions here? i.e. someone does an > export-manifest, edits the file, then tries the re-add the manifest, > but someone else has already changed the criteria via 'set-criteria'? We have different scenarios in mind. You're assuming that a user will export a manifest, edit, and re-import. I'm assuming that the user keeps their original manifests, edits as needed, and re-imports. Given your scenario there is no reason to flag that a set-criteria was done or to warn the user when the criteria portions don't match because they are always working with the most recently imported version of the manifest. I can support your scenario, but we should explicitly suggest that mode of operation in the documentation. > >>> >>>>> ... >>>>> >>>>> >>>>> For all of the installadm subcommands, the main operand shouldn't >>>>> need an option identifier. For example, >>>>> >>>>> installadm list -n <svcname> >>>>> >>>>> should be >>>>> >>>>> installadm list <svcname> >>>> Since all installadm subcommands, except delete-client, help, and >>>> version, require svcname I suggest that it be made an operand for >>>> all commands and all other arguments be options. >>> >>> The new [set|get]-criteria subcommands we're proposing >>> don't follow this convention. The svcname is specified as an >>> option in those cases. >> I was suggesting that those be changed too: >> installadm set-criteria -m <manifest_name> -p name=value svcname >> >> installadm get-criteria -m <manifest_name> -p [name] svcname >> >> where -p can be repeated for multiple properties > > This is fine by me, but we run into the -m inconsistency with > add-manifest again (I should have brought this up earlier). > With add-manifest, -m specifies the file name to publish as > the manifest, whereas these other commands including remove-manifest, > use -m to specify the manifest name which is different from > the file, see bug 4322. > > We can change add-manifest to be -f instead of -m since its a file. The add-manifest would need both -f filename and -m manifest name, right? > > ... > >>>> "Installadm help subcommand" would not be affected. "Delete-client >>>> mac_addr" would be changed to "delete-client -e mac_addr" to be >>>> consistent with "create-client -e mac_addr ..." and the use of >>>> svcname as the only command operand. >>> >>> Shouldn't the concept of the "command operand" depend on the >>> subcommand? For the case of create-client, the service is more >>> like a supporting option, and the client is the main operand, so I >>> was thinking create-client to become >>> >>> create-client ... [-t <imagepath>] -n <svcname> <client_ID> > > >> You could do that, but the syntax is easier to remember when the >> operand is the same across subcommands, and virtually all commands, >> including create-client require <svcname>. > > Not all *require* <svcname>. list and create-service use it as > an optional argument. (Notice the square brackets in the usage). OK, but I'm still concerned that the commands be easy to remember. A user shouldn't have to remember whether <svcname> requires -n in one command, but not in another.
One alternative is that there are no operands, only options. But "installadm start -n <svcname>" seems a little overkill. Still, it would be acceptable. Another alternative would be: if a subcommand has only one required option and no optional options, e.g. start <svcname>, that option should be an operand. Subcommands having multiple options, whether required or not, have only options and no operands. A slightly more complex rule to learn, but not too likely to err. I feel either is acceptable. > > >> Also, I don't quite understand why -t is required since each service >> has only one <imagepath>, right? > > Its not required. Square brackets means its an optional argument. Is it changing? $ installadm create-client shows -t as required. > > > > thanks, > -ethan
