glassfish
  1. glassfish
  2. GLASSFISH-13894

update-node-* lets you set a nodedir on in-use node if original node does not have one

    Details

    • Type: Bug Bug
    • Status: Reopened
    • Priority: Minor Minor
    • Resolution: Unresolved
    • Affects Version/s: 3.1
    • Fix Version/s: future release
    • Component/s: admin
    • Labels:
      None
    • Environment:

      Operating System: All
      Platform: All

      Description

      build: glassfish-3.1-b23-10_07_2010.zip

      Create a node on localhost specifying a node directory, e.g. mynodes and the
      same install directory as localhost node. Create standalone instance using new
      node. At this point directory structure is created in filesystem for node and
      instance. Go to Edit Node screen, for the newly created node and modify node
      directory by deleting the entry. Attempt to start instance and the following
      error is printed on screen:

      An error has occurred
      Could not start instance sanlan1 on node lancer (localhost). Command failed on
      node lancer (localhost): CLASSPATH=
      /export/home/j2eetest/v3.1/glassfish3/glassfish/bin/../modules/admin-cli.jar
      Commands: [start-local-instance, --node, lancer, --nodedir, , sanlan1] asadmin
      extension directory: /export/home/j2eetest/v3.1/glassfish3/glassfish/lib/asadmin
      Prepare Process program options Parsing program options Parse command options
      params:

      {node: [lancer] nodedir: [] }

      operands: [sanlan1] Prevalidate...

      Either user should be able to change node directory without further errors or
      this choice should be disabled.

      1. server.log
        6 kB
        lidiam
      2. server.log
        11 kB
        lidiam
      1. node-chnodedir.JPG
        90 kB
      2. nodedir-change.JPG
        82 kB
      3. nodedir-startup-error.JPG
        94 kB

        Activity

        Hide
        lidiam added a comment -

        Created an attachment (id=5107)
        screenshot

        Show
        lidiam added a comment - Created an attachment (id=5107) screenshot
        Hide
        lidiam added a comment -

        Created an attachment (id=5108)
        server.log

        Show
        lidiam added a comment - Created an attachment (id=5108) server.log
        Hide
        Anissa Lam added a comment -

        I think backend needs to fix this, ie to be able to handle changing of the
        node-dir after the node is created.
        User maybe using CLI set to modify that as well.

        If this is going to cause problem, it should disallow it so user cannot use CLI
        to change it, and notify GUI to make that field read-only after creation.
        If user should be able to change node directory after creation, then the current
        behavior is wrong.

        Show
        Anissa Lam added a comment - I think backend needs to fix this, ie to be able to handle changing of the node-dir after the node is created. User maybe using CLI set to modify that as well. If this is going to cause problem, it should disallow it so user cannot use CLI to change it, and notify GUI to make that field read-only after creation. If user should be able to change node directory after creation, then the current behavior is wrong.
        Hide
        Joe Di Pol added a comment -

        A couple of options:

        1) Don't allow changing the nodedir once a node has been created. Or

        2) Don't allow changing the nodedir if there are any instances using the node.

        I'm leaning towards #2. update-node-* can check if any instances are using
        the node and report an error if nodedir is being changed.

        Show
        Joe Di Pol added a comment - A couple of options: 1) Don't allow changing the nodedir once a node has been created. Or 2) Don't allow changing the nodedir if there are any instances using the node. I'm leaning towards #2. update-node-* can check if any instances are using the node and report an error if nodedir is being changed.
        Hide
        Joe Di Pol added a comment -

        The code has been changed so that if you try to change installdir or nodedir and
        the node is referenced by one or more server instances then the update command
        will fail with an error.

        Author: jfdipol
        Date: 2010-11-03 21:44:43+0000
        New Revision: 42431

        Modified:

        trunk/v3/cluster/admin/src/main/java/com/sun/enterprise/v3/admin/cluster/LocalStrings.properties

        trunk/v3/cluster/admin/src/main/java/com/sun/enterprise/v3/admin/cluster/NodeUtils.java

        trunk/v3/cluster/admin/src/main/java/com/sun/enterprise/v3/admin/cluster/UpdateNodeCommand.java

        Show
        Joe Di Pol added a comment - The code has been changed so that if you try to change installdir or nodedir and the node is referenced by one or more server instances then the update command will fail with an error. Author: jfdipol Date: 2010-11-03 21:44:43+0000 New Revision: 42431 Modified: trunk/v3/cluster/admin/src/main/java/com/sun/enterprise/v3/admin/cluster/LocalStrings.properties trunk/v3/cluster/admin/src/main/java/com/sun/enterprise/v3/admin/cluster/NodeUtils.java trunk/v3/cluster/admin/src/main/java/com/sun/enterprise/v3/admin/cluster/UpdateNodeCommand.java
        Hide
        lidiam added a comment -

        I can still change node directory on an SSH node that has an instance underneath. Here are the steps to reproduce:

        1. In Admin Console create an SSH node on localhost, filling in only node name and host.
        2. Create a standalone instance under the new node with default settings.
        3. Start the instance.
        4. Stop the instance.
        5. Go to Nodes page and type a bogus name for Node Directory field - the change is accepted. I tried it three times to make sure...
        6. If you attempt to start the instance you will get the following error:

        An error has occurred
        Could not start instance in2 on node ln (localhost). Command failed on node ln (localhost): Command start-local-instance failed. Node directory /export/home/j2eetest/v3.1/glassfish3/glassfish/bogusDir does not exist or is not a directory To complete this operation run the following command locally on host localhost from the GlassFish install location /export/home/j2eetest/v3.1/glassfish3: asadmin start-local-instance --node ln --nodedir /export/home/j2eetest/v3.1/glassfish3/glassfish/bogusDir --sync normal in2

        7. Go back to the node page and wipe out bogus node dir name - this time the change is not accepted with the expected error message of:

        An error has occurred
        Cannot update node ln. It is in use by a server instance and therefore attribute nodedir cannot be changed.

        I'm not sure why this message does not show up the first time around...

        Show
        lidiam added a comment - I can still change node directory on an SSH node that has an instance underneath. Here are the steps to reproduce: 1. In Admin Console create an SSH node on localhost, filling in only node name and host. 2. Create a standalone instance under the new node with default settings. 3. Start the instance. 4. Stop the instance. 5. Go to Nodes page and type a bogus name for Node Directory field - the change is accepted. I tried it three times to make sure... 6. If you attempt to start the instance you will get the following error: An error has occurred Could not start instance in2 on node ln (localhost). Command failed on node ln (localhost): Command start-local-instance failed. Node directory /export/home/j2eetest/v3.1/glassfish3/glassfish/bogusDir does not exist or is not a directory To complete this operation run the following command locally on host localhost from the GlassFish install location /export/home/j2eetest/v3.1/glassfish3: asadmin start-local-instance --node ln --nodedir /export/home/j2eetest/v3.1/glassfish3/glassfish/bogusDir --sync normal in2 7. Go back to the node page and wipe out bogus node dir name - this time the change is not accepted with the expected error message of: An error has occurred Cannot update node ln. It is in use by a server instance and therefore attribute nodedir cannot be changed. I'm not sure why this message does not show up the first time around...
        Hide
        lidiam added a comment -

        Attaching screenshots and server.log.

        Show
        lidiam added a comment - Attaching screenshots and server.log.
        Hide
        Joe Di Pol added a comment -

        The current code lets you update a node with new values if the attributes you are updating do not already have values, even if the node is in use by an instance. Here is the relevant comment from UpdateNodeCommand:

        // If the current (config) value is null or "" then let it be changed.
        // We need to do this for the offline config case where the user has
        // created a config node with no values, created instances using those
        // nodes, then updates the values later. This has the undersireable
        // effect of letting you, for example, set a nodedir on a node
        // that was created without one.
        if (!StringUtils.ok(currentvalue))

        { return true; }

        In order to truly fix this we need to be able to tell the difference between the offline config use case (where instances exists in the config, but do not physically exist on a server yet) versus the case where the physical instances do exist.

        In the former case update-node-* should allow all attributes to be changed.

        In the later case update-node-* should be strict about what attributes can be changed.

        This is related to bug GLASSFISH-15414.

        To fix this we may need to add a "rendezvous" attribute to the server instance domain.xml element to indicate that the physical instance has been created.

        Show
        Joe Di Pol added a comment - The current code lets you update a node with new values if the attributes you are updating do not already have values, even if the node is in use by an instance. Here is the relevant comment from UpdateNodeCommand: // If the current (config) value is null or "" then let it be changed. // We need to do this for the offline config case where the user has // created a config node with no values, created instances using those // nodes, then updates the values later. This has the undersireable // effect of letting you, for example, set a nodedir on a node // that was created without one. if (!StringUtils.ok(currentvalue)) { return true; } In order to truly fix this we need to be able to tell the difference between the offline config use case (where instances exists in the config, but do not physically exist on a server yet) versus the case where the physical instances do exist. In the former case update-node-* should allow all attributes to be changed. In the later case update-node-* should be strict about what attributes can be changed. This is related to bug GLASSFISH-15414 . To fix this we may need to add a "rendezvous" attribute to the server instance domain.xml element to indicate that the physical instance has been created.
        Hide
        Joe Di Pol added a comment -

        Lowering priority since this is a lack of an error check and not a significant loss of functionality.

        Show
        Joe Di Pol added a comment - Lowering priority since this is a lack of an error check and not a significant loss of functionality.

          People

          • Assignee:
            Joe Di Pol
            Reporter:
            lidiam
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated: