Bug #3874: API: Changing pending node status doesn't work - Rudder - Issue Tracker

Actions

Copy link

#1

Updated by Vincent MEMBRÉ over 10 years ago

Status changed from New to In progress
Assignee set to Vincent MEMBRÉ

Thanks Olivier!

I'll look into this immediatly

Actions

Copy link

#2

Updated by Vincent MEMBRÉ over 10 years ago

Olivier Mauras wrote:

# curl -X GET -H "X-API-Version: 2" -H "X-API-Token: wRY5qwr8k6XTY7aaRdxFl3csDanZfuZG"  http://buildbox.internal/rudder/api/nodes/pending/8c979d92-8c21-4ce0-a383-27ce7cc0997b
<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
<html> <body>The Requested URL /rudder/api/nodes/pending/8c979d92-8c21-4ce0-a383-27ce7cc0997b was not found on this server</body> </html>

GET api/nodes/pending/id is not a function in Rudder : there is only /nodes/pending (list all pending nodes) and /nodes/id (get a node Details, but should not depend on its status...)

# curl -GET -H "X-API-Version: 2" -H "X-API-Token: wRY5qwr8k6XTY7aaRdxFl3csDanZfuZG"  http://buildbox.internal/rudder/api/nodes/pending
{"action":"listPendingNodes","result":"success","data":{"nodes":[{"id":"8c979d92-8c21-4ce0-a383-27ce7cc0997b","status":"pending","hostname":"ovz2.internal","osName":"Scientific"}]}}

Cool one thing is working :)

# curl -X GET -H "X-API-Version: 2" -H "X-API-Token: wRY5qwr8k6XTY7aaRdxFl3csDanZfuZG"  http://buildbox.internal/rudder/api/nodes/pending -d "id=8c979d92-8c21-4ce0-a383-27ce7cc0997b" 
{"action":"listPendingNodes","result":"success","data":{"nodes":[{"id":"8c979d92-8c21-4ce0-a383-27ce7cc0997b","status":"pending","hostname":"ovz2.internal","osName":"Scientific"}]}}

The id parameters (usually) are URL parameters, it means that they should be used as in URL like /nodes/id and are not used.

This need to be at least in the documentation that it is not supported, or be corrected to support id as query parameter. Need to add examples to the documentation too. Here /api/nodes/pending (list pending nodes) resulting in good result for your case.

# curl -X POST -H "X-API-Version: 2" -H "X-API-Token: wRY5qwr8k6XTY7aaRdxFl3csDanZfuZG"  http://buildbox.internal/rudder/api/nodes/pending -d "id=8c979d92-8c21-4ce0-a383-27ce7cc0997b" -d "status=accepted" 
{"action":"changePendingNodeStatus","result":"error","errorDetails":"You must add a node id as target"}

Same as above, id should be in URL, here pending is used as node id (change node status function is POST /api/nodes/id), maybe we need to add a function to change pending node status. We need at least to correct the error message.

# curl -X POST -H "X-API-Version: 2" -H "X-API-Token: wRY5qwr8k6XTY7aaRdxFl3csDanZfuZG"  http://buildbox.internal/rudder/api/nodes/pending/8c979d92-8c21-4ce0-a383-27ce7cc0997b -d "status=accepted" 
<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
<html> <body>The Requested URL /rudder/api/nodes/pending/8c979d92-8c21-4ce0-a383-27ce7cc0997b was not found on this server</body> </html>

Same as above : POST /api/nodes/pending/id is not the function to change node status it is /api/nodes/id . Adding the /api/nodes/pending/id would allow be a good idea!

# curl -X POST -H "X-API-Version: 2" -H "X-API-Token: wRY5qwr8k6XTY7aaRdxFl3csDanZfuZG"  http://buildbox.internal/rudder/api/nodes/8c979d92-8c21-4ce0-a383-27ce7cc0997b -d "status=accepted" 
<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
<html> <body>The Requested URL /rudder/api/nodes/8c979d92-8c21-4ce0-a383-27ce7cc0997b was not found on this server</body> </html>

This one is a correct call and should accept node 8c979d92-8c21-4ce0-a383-27ce7cc0997b ... need to look at it deeper

# curl -X POST -H "X-API-Version: 2" -H "X-API-Token: wRY5qwr8k6XTY7aaRdxFl3csDanZfuZG"  http://buildbox.internal/rudder/api/nodes -d "id=8c979d92-8c21-4ce0-a383-27ce7cc0997b" -d "status=accepted" 
<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
<html> <body>The Requested URL /rudder/api/nodes was not found on this server</body> </html>

using id as parameters not in URL is not taken into account, maybe we really need to accept it

To sum up:

What to look and fix

POST /api/nodes/id with status "accept" has not change a pending node status, need to look a it

Documentation Upgrade:

Explain that URL parameters needs to be in URL, and will not be taken into account
Add nodes examples...

Possible upgrade :

Accept id passed as parameters instead of ignoring them
Add a function to upgrade pending nodes POST ,_/api/nodes/pending/id_ ? I wonder if it would fit with the API organisation (action is based on nodes), but it seems you wanted to use that function because it seems logic that it would exists

Thank you for all of this Olivier!

Actions

Copy link

#3

Updated by Vincent MEMBRÉ over 10 years ago

There is an error in documentation :

POST /api/nodes/id does not exists. the function to change a pending node status is : /api/nodes/pending with paramaters nodeId (can be multi valued) and status.

Olivier this one should be good :

 curl -X POST -H "X-API-Version: 2" -H "X-API-Token: wRY5qwr8k6XTY7aaRdxFl3csDanZfuZG" http://buildbox.internal/rudder/api/nodes/pending -d "nodeId=8c979d92-8c21-4ce0-a383-27ce7cc0997b" -d "status=accepted"

Sorry about that, fixing immediatly

Actions

Copy link

#4

Updated by Olivier Mauras over 10 years ago

# curl -X GET -H 'Content-Type: application/json' -H "X-API-Token: NeCk6maZMEgL555YNYquJEToYyIIuCei" -H "X-API-Version: 2" http://rudder/rudder/api/nodes/pending 2> /dev/null | sed 's/,/\n/g' | grep $HOSTNAME -B 2
"data":{"nodes":[{"id":"f858fe6c-2e16-40a0-b400-2661a7c7f79c" 
"status":"pending" 
"hostname":"ruddertest-01" 

# curl -X POST -H "X-API-Version: 2" -H "X-API-Token: NeCk6maZMEgL555YNYquJEToYyIIuCei" http://rudder/rudder/api/nodes/pending -d "nodeId=f858fe6c-2e16-40a0-b400-2661a7c7f79c" -d "status=accepted" 
{"action":"changePendingNodeStatus","result":"success","data":{"nodes":[]}}

# curl -X GET -H 'Content-Type: application/json' -H "X-API-Token: NeCk6maZMEgL555YNYquJEToYyIIuCei" -H "X-API-Version: 2" http://rudder.hq.k.grp/rudder/api/nodes/pending 2> /dev/null | sed 's/,/\n/g' | grep $HOSTNAME -B 2
"data":{"nodes":[{"id":"f858fe6c-2e16-40a0-b400-2661a7c7f79c" 
"status":"pending" 
"hostname":"ruddertest-01"

It returns success while not accepting the node :(

Actions

Copy link

#5

Updated by Olivier Mauras over 10 years ago

[2013-08-27 14:06:20] INFO  com.normation.rudder.web.rest.node.NodeApiService2 - List(NodeId(cdf1ca03-3300-47a7-8b0c-e8258ce22ed2))
[2013-08-27 14:06:30] ERROR com.normation.rudder.repository.EventLogRepository - PreparedStatementCallback; SQL []; ERROR: value too long for type character varying(64); nested exception is org.postgresql.util.PSQLException: ERROR: value too long for type character varying(64)
[2013-08-27 14:06:30] ERROR com.normation.rudder.services.servers.NewNodeManagerImpl - Error when trying to accept node cdf1ca03-3300-47a7-8b0c-e8258ce22ed2. Rollbaking. <- Error when executing accept node process named add_server_to_dyngroup <- Error when trying to update dynamic group NodeGroupId(8017932f-b941-4f47-b687-ddeb6e326fa3) with member cdf1ca03-3300-47a7-8b0c-e8258ce22ed2 <- Error when logging modification as an event <- Exception caught while trying to save an eventlog : PreparedStatementCallback; SQL []; ERROR: value too long for type character varying(64); nested exception is org.postgresql.util.PSQLException: ERROR: value too long for type character varying(64)
[2013-08-27 14:06:30] ERROR com.normation.rudder.services.servers.AcceptInventory - Error when rollbacking server node id cdf1ca03-3300-47a7-8b0c-e8258ce22ed2 in process 'accept_new_server:inventory', you should delete it by hand. Error messages: Can not move 'nodeId=cdf1ca03-3300-47a7-8b0c-e8258ce22ed2,ou=Nodes,ou=Accepted Inventories,ou=Inventories,cn=rudder-configuration' to new parent 'ou=Nodes,ou=Pending Inventories,ou=Inventories,cn=rudder-configuration': no such object
[2013-08-27 14:06:30] ERROR com.normation.rudder.services.servers.NewNodeManagerImpl - Unit acceptor error for a node <- Error when trying to accept node cdf1ca03-3300-47a7-8b0c-e8258ce22ed2 <- Error when executing accept node process named add_server_to_dyngroup <- Error when trying to update dynamic group NodeGroupId(8017932f-b941-4f47-b687-ddeb6e326fa3) with member cdf1ca03-3300-47a7-8b0c-e8258ce22ed2 <- Error when logging modification as an event <- Exception caught while trying to save an eventlog : PreparedStatementCallback; SQL []; ERROR: value too long for type character varying(64); nested exception is org.postgresql.util.PSQLException: ERROR: value too long for type character varying(64)
[2013-08-27 14:06:30] ERROR com.normation.rudder.repository.EventLogRepository - PreparedStatementCallback; SQL []; ERROR: value too long for type character varying(64); nested exception is org.postgresql.util.PSQLException: ERROR: value too long for type character varying(64)
[2013-08-27 14:06:30] WARN  com.normation.rudder.repository.xml.GitNodeGroupArchiverImpl - Auto-archive git failure: not found in git added files: 'groups/8c1c96b4-267a-4b2b-9815-3d911f945858/9946eefb-01fd-4e4b-a5a1-471b63f7084c/8017932f-b941-4f47-b687-ddeb6e326fa3.xml'. You can safelly ignore that warning if the file was already existing in Git and was not modified by that archive.
[2013-08-27 14:06:31] INFO  com.normation.rudder.services.servers.NodeConfigurationServiceImpl - No node configuration was updated, no promises to write
[2013-08-27 14:06:31] INFO  com.normation.rudder.batch.AsyncDeploymentAgent - Successful deployment 560 [2013/08/27 14:06:30 - 2013/08/27 14:06:31]
[2013-08-27 14:06:31] ERROR com.normation.rudder.repository.EventLogRepository - PreparedStatementCallback; SQL []; ERROR: value too long for type character varying(64); nested exception is org.postgresql.util.PSQLException: ERROR: value too long for type character varying(64)

The error in webapp log

Actions

Copy link

#6

Updated by Jonathan CLARKE over 10 years ago

Description updated (diff)

Actions

Copy link

#7

Updated by Jonathan CLARKE over 10 years ago

Vincent MEMBRÉ wrote:

[...]
GET api/nodes/pending/id is not a function in Rudder : there is only /nodes/pending (list all pending nodes) and /nodes/id (get a node Details, but should not depend on its status...)
[...]
Same as above : POST /api/nodes/pending/id is not the function to change node status it is /api/nodes/id . Adding the /api/nodes/pending/id would allow be a good idea!
[...]
POST /api/nodes/id does not exists. the function to change a pending node status is : /api/nodes/pending with paramaters nodeId (can be multi valued) and status.

This is confusing, and violates POLA (http://en.wikipedia.org/wiki/Principle_of_least_astonishment). I think we should:

Make {GET,POST} api/nodes/pending/id work, the same as {GET,POST} api/nodes/id
Make it so that we can enable a node with curl -X POST http://<server>/rudder/api/nodes/pending/id -d status=accepted

These two calls make a lot of sense and are "logical" according to the rest of the REST API.

Actions

Copy link

#8

Updated by Vincent MEMBRÉ over 10 years ago

Status changed from In progress to Pending technical review
Assignee changed from Vincent MEMBRÉ to François ARMAND
Pull Request set to https://github.com/Normation/rudder/pull/300

Pull request : https://github.com/Normation/rudder/pull/300

As you said Jon, I added two methods {GET,POST} api/nodes/pending/id, and they work ad intended

Olivier, The bug that make no errors when there is one in node acceptance/refusale will be treated in #3880

Actions

Copy link

#9

Updated by Vincent MEMBRÉ over 10 years ago

Status changed from Pending technical review to Pending release
% Done changed from 0 to 100

Applied in changeset 73486d1f6f1ebb9ff501699f5a373805ef5812df.

Actions

Copy link

#10

Updated by Anonymous over 10 years ago

Applied in changeset 647c1eda6b1d0d650f911425473207d7f1751213.

Actions

Copy link

#11

Updated by Nicolas PERRON over 10 years ago

Status changed from Pending release to Released

This bug has been fixed in Rudder 2.7.1, which was released today.
Check out:

The release announcement: http://www.rudder-project.org/pipermail/rudder-announce/2013-September/000046.html
The full ChangeLog: http://www.rudder-project.org/foswiki/bin/view/System/Documentation:ChangeLog27
Download information: http://www.rudder-project.org/foswiki/Download/

	Related to Rudder - Bug #3880: Errors when accepting/refusing nodes are ignored	Released	Nicolas CHARLES	2013-08-28			Actions
	Related to Rudder - Bug #3883: The limit of the eventlog length (64chars) could lead to SQL errors	Released	Nicolas CHARLES	2013-08-29			Actions

Project

General

Profile

Rudder

Custom queries

Bug #3874

API: Changing pending node status doesn't work

Updated by Vincent MEMBRÉ over 10 years ago

Updated by Vincent MEMBRÉ over 10 years ago

Updated by Vincent MEMBRÉ over 10 years ago

Updated by Olivier Mauras over 10 years ago

Updated by Olivier Mauras over 10 years ago

Updated by Jonathan CLARKE over 10 years ago

Updated by Jonathan CLARKE over 10 years ago

Updated by Vincent MEMBRÉ over 10 years ago

Updated by Vincent MEMBRÉ over 10 years ago

Updated by Anonymous over 10 years ago

Updated by Nicolas PERRON over 10 years ago