Opened 51 years ago
Last modified 10 years ago
#892 new defect
IZ580: install master host and install managed host commands fail when system name specified as enviromental variable but not as -s global argument
Reported by: | marcingoldyn | Owned by: | |
---|---|---|---|
Priority: | normal | Milestone: | |
Component: | hedeby | Version: | 1.0u1 |
Severity: | Keywords: | Sun cli | |
Cc: |
Description
[Imported from gridengine issuezilla http://gridengine.sunsource.net/issues/show_bug.cgi?id=580]
Issue #: 580 Platform: Sun Reporter: marcingoldyn (marcingoldyn) Component: hedeby OS: All Subcomponent: cli Version: 1.0u1 CC: None defined Status: NEW Priority: P3 Resolution: Issue type: DEFECT Target milestone: 1.0u5next Assigned to: aja (aja) QA Contact: adoerr URL: * Summary: install master host and install managed host commands fail when system name specified as enviromental variable but not as -s global argument Status whiteboard: Attachments: Issue 580 blocks: Votes for issue 580: Vote for this issue Opened: Mon Oct 6 05:45:00 -0700 2008 ------------------------ Description: When user set enviromental variable SDM_SYTEM before installing master or managed host, and then tries to perform above operation without setting -s <system_name> global argument, the commands will fail for him. It can be reproduce by this set of commands. 1. First we will set the enviromental variable SDM_SYSTEM export SDM_SYSTEM=somesystemname 2. We will check that the system variable is set echo $SDM_SYSTEM 3. Now we will try to install master or managed host. sdmadm -p user imsth -au someuser -ca_admin_mail mail_adress -ca_country co -ca_location location -ca_org organization -ca_org_unit unit -ca_state state -cs_port 5463 -sge_root /sge_root this operation will fail with following error: Install master host command is using default local spool dir: /var/spool/sdm/null Error: No system name specified Evaluation: The problem applies just to those 2 installation commands. General question is if we want to allow such action that user before installation can set the SDM_SYSTEM name and perform installation of master or managed host. In documentaion it is not stated that it is not working for installation commands however. The severity of this bug is maximum P3. Suggested Fix/Work Around: There exists work around for this 2 commands. User has to specify -s <system_name> global argument while executing one of above commands. solutions for this solving this issue are: 1. Change the documentation in a way, that we will explicitly write in there that SDM_SYSTEM variable can be set and it will be working after installation of master or manged host, but for installation commands user has to provide knowingly the system name as -s <system_name> global argument. 2. Change the implementation of Installation Commands. Analysis: In current code we have method in AbstractCli class - evaluateExecutionEnv(). In this method evaluation of system name is done either from -s global option or from enviromental variable SDM_SYSTEM. Its just validation of this name. It will not return the system name from enviromental variable to the installation command unless it is set as -s global argument. How to test: If we will decide to change implementation in a way, that installation commands will take system name from SDM_SYSTEM variable, we can consider improving our current installation tests for SDM. ATC 1 PD ETC 2 PD ------- Additional comments from torsten Mon Oct 6 06:12:53 -0700 2008 ------- I think the more interesting cases are the uninstall commands! How do they behave? The worst thing that can happen with an install per environment variable is that you get an error message saying that the system already exists. With an uninstall however, you could delete a system by accident, if you forgot to specify the system name via -s and have SDM_SYSTEM set in the environment. That is NOT GOOD! Assuming that the uninstall requires a -s parameter (if it doesn't, this would be an issue imo), I'd say that the install command should behave consistently with this, so also require a -s parameter. This would require no change in the code, the documentation would need to be updated, though, to document install (and uninstall) special case. ------- Additional comments from marcingoldyn Mon Oct 6 06:50:20 -0700 2008 ------- so for uninstallation commands it behaves like this, that it will uninstall the system that is specified under SDM_SYSTEM variable if set. But from the other hand, if someone specifies SDM_SYSTEM he should know that in case of not-specifing the -s argument explicitly, the default system will be affected. And I think this is documented. ------- Additional comments from adoerr Mon Oct 6 06:56:00 -0700 2008 ------- The point made by Torsten is valid. Allowing the SDM_SYSTEM environment variable for the install and uninstall commands could cause havoc. I personally think that a consistent, predictable behavior is more important than protecting a user from himself, however. In other words, IMHO, the SDM_SYSTEM environment variable should work for *all* commands. In addition the 'createExecEnv(boolean onlyEvaluate)' method should be refactored into two separate methods. There should be a 'createExecEnv()' method that just creates an execution environment. In addition a second method, 'evaluateExecEnv()', should just evaluate the execution environment *without* any side effects. That means 'evaluateExecEnv()' will *not* create an execution environment if it does not exist. ------- Additional comments from marcingoldyn Mon Oct 6 07:40:31 -0700 2008 ------- ok so its decided, when user will set up a SDM_SYSTEM variable there will be possible to perfom installation of managed host and master host without specifing the -s <system_name> global option. method evaluateExecEnv() right now is not making any harm and site effects to the code. It is just implemented in a strange way, that its actually calling the createExecEnv() method but with the flag onlyEvaluate set to true. It means that there is just check done if the system name given by the user is valid and after that *no* executionenv is created, null value is returned. Anyway it is good idea to as a part of this issue make there such clean up to divide this two action. To solve the issue we have to create some helper method that will read this enviromental variable sdm_system in this cli commands when -s global argument is not specified. It could be done in AbstractCLi class. ------- Additional comments from adoerr Mon Oct 6 08:03:45 -0700 2008 ------- The helper method you are talking about is already there, sort of. Please have a look at 'AbstractCli.evaluateSystemName()'. The problem is that currently there are two ways to get the system name. One way is to get it from the execution environment, using 'ExecutionEnv.getSystemName()'. The second way is to use 'AbstractCli.getSystemName()', which always does return the value for the '-s' option. This second way is used by the 'InstallMasterCliCommand' and the reason why this command does not respect the SDM_SYSTEM variable. IMHO, however, the system name should *always* be taken from the execution environment. My suggestion is to get rid of 'AbstractCli.getSystemName()' and use 'ExecutionEnv.getSystemName()' exclusively. In case the environment variable SDM_SYSTEM is set *and* the '-s' option is specified, the value of the '-s' option must have precedence over the SDM_SYSTEM environment variable. ------- Additional comments from marcingoldyn Tue Oct 7 01:22:11 -0700 2008 ------- Yes I agree. The problem is that at this time there is no execution env created. We can create it but using not validated data. Maybe for this commands it would be easier if we use this AbstractCli.getSystemName()? We will add there just one line, that in case that systemName will be null try to get it from SDM_SYSTEM. Validation of this name is always done when execution env is created so we dont have to worry about check if its correct. What do you think? ------- Additional comments from aja Wed Oct 14 02:17:44 -0700 2009 ------- accepting the issue ------- Additional comments from rhierlmeier Wed Nov 25 07:21:10 -0700 2009 ------- Milestone changed ------- Additional comments from rhierlmeier Wed Nov 25 07:25:01 -0700 2009 ------- reassigned to aja
Note: See
TracTickets for help on using
tickets.