IZ580: install master host and install managed host commands fail when system name specified as enviromental variable but not as -s global argument

[marcingoldyn]

[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