[GLASSFISH-13894] update-node-* lets you set a nodedir on in-use node if original node does not have one Created: 08/Oct/10  Updated: 15/Apr/14

Status: Reopened
Project: glassfish
Component/s: admin
Affects Version/s: 3.1
Fix Version/s: future release

Type: Bug Priority: Minor
Reporter: lidiam Assignee: Joe Di Pol
Resolution: Unresolved Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified
Environment:

Operating System: All
Platform: All


Attachments: JPEG File node-chnodedir.JPG     JPEG File nodedir-change.JPG     JPEG File nodedir-startup-error.JPG     Text File server.log     Text File server.log    
Issuezilla Id: 13,894
Tags: 3_1_1-scrubbed

 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.



 Comments   
Comment by lidiam [ 08/Oct/10 ]

Created an attachment (id=5107)
screenshot

Comment by lidiam [ 08/Oct/10 ]

Created an attachment (id=5108)
server.log

Comment by Anissa Lam [ 08/Oct/10 ]

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.

Comment by Joe Di Pol [ 11/Oct/10 ]

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.

Comment by Joe Di Pol [ 03/Nov/10 ]

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

Comment by lidiam [ 25/Feb/11 ]

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...

Comment by lidiam [ 25/Feb/11 ]

Attaching screenshots and server.log.

Comment by Joe Di Pol [ 12/May/11 ]

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.

Comment by Joe Di Pol [ 06/Jul/11 ]

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

Generated at Tue Jun 30 21:56:06 UTC 2015 using JIRA 6.2.3#6260-sha1:63ef1d6dac3f4f4d7db4c1effd405ba38ccdc558.