-
Notifications
You must be signed in to change notification settings - Fork 77
Ssh obm service #505
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Ssh obm service #505
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| { | ||
| "id" : "<%=id%>", | ||
| <% if (node !== null) { %> | ||
| "node": "<%=basepath%>/nodes/<%=node%>", | ||
| <% } %> | ||
| "service": "<%= service %>", | ||
| "config": <%- JSON.stringify(config) %> | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,7 +19,8 @@ di.annotate(nodesRouterFactory, new di.Inject( | |
| 'Logger', | ||
| 'Promise', | ||
| '_', | ||
| 'Assert' | ||
| 'Assert', | ||
| 'Errors' | ||
| ) | ||
| ); | ||
|
|
||
|
|
@@ -34,28 +35,41 @@ function nodesRouterFactory ( | |
| Logger, | ||
| Promise, | ||
| _, | ||
| assert | ||
| assert, | ||
| Errors | ||
| ) { | ||
| var router = express.Router(); | ||
|
|
||
| /** | ||
| * Renders obmSettings property into a node. | ||
| * Renders obmSettings and sshSettings properties into a node. | ||
| * | ||
| * @param {object} node - node model instance | ||
| * @return {object} Promise that fulfills to the rendered node | ||
| */ | ||
| function _renderNodeObmSettings(node) { | ||
| function _renderNodeMgmtSettings(node) { | ||
| // This function exists to allow backward compatibility of the 1.1 API | ||
| // with 2.0 API OBM resources. | ||
| // with 2.0 API OBM and SSH resources. | ||
| assert.object(node, 'node should be a node model instance.'); | ||
| assert.ok(!node.obmSettings, 'node should not have obmSettings property'); | ||
|
|
||
| node = _.omit(node.toJSON(), 'obms'); | ||
| return Promise.map(waterline.obms.find({node: node.id}), function(obm) { | ||
| return _.pick(obm.toJSON(), 'service', 'config'); | ||
| }) | ||
| .then(function(obms) { | ||
| node = _.omit(node.toJSON(), ['obms', 'ibms']); | ||
|
|
||
| var obms = Promise.map(waterline.obms.find({node: node.id}), function(obm) { | ||
| return _.pick(obm.toJSON(), 'service', 'config'); | ||
| }); | ||
| var sshSettings = waterline.ibms.findByNode(node.id, 'ssh-ibm-service'); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since this is only for a 1.1 API backwards compatibility edge case, I'm not going to raise an issue. But in any 2.0 controller I would object to hardcoding for specific IBM service types, especially just for doing 'REDACT' which shouldn't be handled at this layer anyways.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Besides, the function name is
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @benbp : Thanks ben! @yyscamper : I will rename the function.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done |
||
| return Promise.all([obms, sshSettings]) | ||
| .spread(function(obms, sshSettings) { | ||
| node.obmSettings = obms; | ||
| if (sshSettings) { | ||
| node.sshSettings = { | ||
| host: sshSettings.config.host, | ||
| user: sshSettings.config.user, | ||
| password: 'REDACTED' | ||
| }; | ||
| } else { | ||
| delete node.sshSettings; | ||
| } | ||
| return node; | ||
| }); | ||
| } | ||
|
|
@@ -73,7 +87,7 @@ function nodesRouterFactory ( | |
| return nodeApiService.getAllNodes(req.query) | ||
| .then(function (nodes){ | ||
| return Promise.map(nodes, function(node) { | ||
| return _renderNodeObmSettings(node); | ||
| return _renderNodeMgmtSettings(node); | ||
| }); | ||
| }); | ||
| })); | ||
|
|
@@ -140,7 +154,7 @@ function nodesRouterFactory ( | |
| router.get('/nodes/:identifier', rest(function (req) { | ||
| return nodeApiService.getNodeById(req.params.identifier) | ||
| .then(function (node){ | ||
| return _renderNodeObmSettings(node); | ||
| return _renderNodeMgmtSettings(node); | ||
| }); | ||
| })); | ||
|
|
||
|
|
@@ -184,7 +198,7 @@ function nodesRouterFactory ( | |
| }) | ||
| .then(function() { | ||
| // lastly render obmSettings into the node. | ||
| return _renderNodeObmSettings(node); | ||
| return _renderNodeMgmtSettings(node); | ||
| }); | ||
| })); | ||
|
|
||
|
|
@@ -233,7 +247,7 @@ function nodesRouterFactory ( | |
| router.get('/nodes/:identifier/obm', rest(function (req) { | ||
| return nodeApiService.getNodeObmById(req.params.identifier) | ||
| .then(function (node){ | ||
| return _renderNodeObmSettings(node) | ||
| return _renderNodeMgmtSettings(node) | ||
| .then(function (retNode){ | ||
| return retNode.obmSettings; | ||
| }); | ||
|
|
@@ -284,7 +298,7 @@ function nodesRouterFactory ( | |
| }); | ||
| }) | ||
| .then(function() { | ||
| return _renderNodeObmSettings(node); | ||
| return _renderNodeMgmtSettings(node); | ||
| }); | ||
| })); | ||
|
|
||
|
|
@@ -328,10 +342,13 @@ function nodesRouterFactory ( | |
| */ | ||
|
|
||
| router.get('/nodes/:identifier/ssh', rest(function (req) { | ||
| return nodeApiService.getNodeSshById(req.params.identifier); | ||
| }, { | ||
| serializer: 'Serializables.V1.Ssh', | ||
| isObject: true | ||
| return nodeApiService.getNodeById(req.params.identifier) | ||
| .then(function(node) { | ||
| return _renderNodeMgmtSettings(node) | ||
| .then(function (retNode){ | ||
| return retNode.sshSettings; | ||
| }); | ||
| }); | ||
| })); | ||
|
|
||
|
|
||
|
|
@@ -353,14 +370,32 @@ function nodesRouterFactory ( | |
| */ | ||
|
|
||
| router.post('/nodes/:identifier/ssh', rest(function (req) { | ||
| return nodeApiService.postNodeSshById(req.params.identifier, req.body); | ||
| var node; | ||
| var newBody; | ||
|
|
||
| return Promise.try(function() { | ||
| if (req.body.host && req.body.user && req.body.password) { | ||
| return waterline.nodes.needByIdentifier(req.params.identifier); | ||
| } else { | ||
| throw new Errors.BadRequestError('Invalid SSH body'); | ||
| } | ||
| }) | ||
| .then(function(queryNode) { | ||
| node = queryNode; | ||
| newBody = { | ||
| nodeId: req.params.identifier, | ||
| service: 'ssh-ibm-service', | ||
| config: req.body | ||
| }; | ||
| return waterline.ibms.upsertByNode(req.params.identifier, newBody); | ||
| }) | ||
| .then(function() { | ||
| return _renderNodeMgmtSettings(node); | ||
| }); | ||
| }, { | ||
| deserializer: 'Serializables.V1.Ssh', | ||
| serializer: 'Serializables.V1.Node', | ||
| renderOptions: { success: 201 } | ||
| })); | ||
|
|
||
|
|
||
| /** | ||
| * @api {get} /api/1.1/nodes/:identifier/catalogs GET /:id/catalogs | ||
| * @apiVersion 1.1.0 | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,118 @@ | ||
| // Copyright 2016, EMC, Inc. | ||
|
|
||
| 'use strict'; | ||
|
|
||
| var injector = require('../../../index.js').injector; | ||
| var controller = injector.get('Http.Services.Swagger').controller; | ||
| var schemaApiService = injector.get('Http.Api.Services.Schema'); | ||
| var nameSpace = '/api/2.0/ibms/definitions/'; | ||
| var waterline = injector.get('Services.Waterline'); | ||
| var eventsProtocol = injector.get('Protocol.Events'); | ||
| var Errors = injector.get('Errors'); | ||
| var _ = injector.get('_'); // jshint ignore:line | ||
|
|
||
|
|
||
| // GET /api/2.0/ibms | ||
| var ibmsGet = controller(function(req) { | ||
| return waterline.ibms.find(req.query); | ||
| }); | ||
|
|
||
| // PUT /api/2.0/ibms | ||
| var ibmsPut = controller({success: 201}, function(req) { | ||
| var nodeId = req.swagger.params.body.value.nodeId; | ||
| delete req.swagger.params.body.value.nodeId; | ||
| return waterline.ibms.upsertByNode(nodeId, req.swagger.params.body.value); | ||
| }); | ||
|
|
||
| // GET /api/2.0/ibms/identifier | ||
| var ibmsGetById = controller(function(req) { | ||
| return waterline.ibms.needByIdentifier(req.swagger.params.identifier.value); | ||
| }); | ||
|
|
||
| // PATCH /api/2.0/ibms/identifier | ||
| var ibmsPatchById = controller( function(req) { | ||
| var ibmId = req.swagger.params.identifier.value; | ||
| var values = req.swagger.params.body.value; | ||
| return waterline.ibms.needByIdentifier(ibmId) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. duplicated codes, it also exists in some other places for ibm and obm. two places will be maintained, and easy to miss something if one place is changed. it needs some optimization, or move the common codes to /lib/services/xxxx. maybe it also could be waterline.[xxx].needByxxxx().
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @anhou : I can create a ibm service and move the code for Patch and Delete. But the obm calls the obms waterline collection and ibms calls its own collection. Trying to think if thats what you meant? And not sure what you mean if one place changes ? They are 2 different collections that inherit from base class under on-core/models.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @anhou @srinia6 The duplicated code is in
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. some of the codes are copied from /lib/services/obm-api-services.js the code logic is the same. I think don't need another abstract layer to service api, if ibm and obm really almost the similar in document operation. maybe just a map/dict could reduce the duplication. maintain obm/ibm in /lib/services/bm-api-service.js or lib/services/nm-api-service.js(network management) all the document openration like waterline.obms.xxxx could be waterline.nm[type].xxx, just a method I could think of to de-duplication. or you could use other better ways to do it. |
||
| .then(function(oldIbm) { | ||
| /* Get nodes that need to publish events */ | ||
| if (oldIbm.node && !values.nodeId) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. srinia6 Is this logic needed? Couldn't we just compare the old node to the new node and alert if anything has changed? Or maybe even just always alert?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this part of codes are copied from /lib/service/obm-api-service.js they are written and tested by me several months ago. the logic could be referenced from the unit test cases. the behavior is documented in RackHD/docs in 'event notification' section about obms.assigned/unassigned/updated
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks @anhou
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @anhou - we have created another story to handle slightly similar code in this story - https://www.pivotaltracker.com/story/show/133740825 |
||
| return Promise.all([waterline.nodes.getNodeById(oldIbm.node)]); | ||
| } else if (!oldIbm.node && values.nodeId) { | ||
| return Promise.all([waterline.nodes.getNodeById(values.node)]); | ||
| } else if (oldIbm.node && values.nodeId && oldIbm.node === values.nodeId) { | ||
| return Promise.all([waterline.nodes.getNodeById(oldIbm.node)]); | ||
| } else if (oldIbm.node && values.nodeId && oldIbm.node !== values.nodeId) { | ||
| return Promise.all([waterline.nodes.getNodeById(oldIbm.node), | ||
| waterline.nodes.getNodeById(values.nodeId)]); | ||
| } | ||
| }) | ||
| .then(function(oldNodes) { | ||
| return waterline.ibms.updateByIdentifier(ibmId, values) | ||
| .tap(function() { | ||
| /* Publish events of nodes got beofre update */ | ||
| _.forEach(oldNodes, function(oldNode) { | ||
| if (oldNode) { | ||
| /* asynchronous, don't wait promise return for performance*/ | ||
| return waterline.nodes.getNodeById(oldNode.id) | ||
| .then(function(newNode) { | ||
| return eventsProtocol.publishNodeAttrEvent(oldNode, newNode, 'ibms'); | ||
| }) | ||
| .catch(function (error) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 'error' is defined but never used.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @srinia6 It's probably worth appending |
||
| throw new Errors.BaseError('Error Occured', error.message); | ||
| }); | ||
| } | ||
| }); | ||
| }); | ||
| }); | ||
| }); | ||
|
|
||
| // DELETE /api/2.0/ibms/identifier | ||
| var ibmsDeleteById = controller({success: 204}, function(req) { | ||
| var ibmId = req.swagger.params.identifier.value; | ||
| return waterline.ibms.needByIdentifier(ibmId) | ||
| .then(function (ibm) { | ||
| return waterline.nodes.getNodeById(ibm.node); | ||
| }) | ||
| .then(function (oldNode) { | ||
| return waterline.ibms.destroyByIdentifier(ibmId) | ||
| .tap(function () { | ||
| if (oldNode) { | ||
| /* asynchronous, don't wait promise return for performance*/ | ||
| waterline.nodes.getNodeById(oldNode.id) | ||
| .then(function (newNode) { | ||
| return eventsProtocol.publishNodeAttrEvent(oldNode, newNode, 'ibms'); | ||
| }) | ||
| .catch(function (error) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 'error' is defined but never used. |
||
| throw new Errors.BaseError('Error Occured', error.message); | ||
| }); | ||
| } | ||
| }); | ||
| }); | ||
|
|
||
| }); | ||
|
|
||
| // GET /ibms/definitions | ||
| var ibmsDefinitionsGetAll = controller(function() { | ||
| return schemaApiService.getNamespace(nameSpace); | ||
| }); | ||
|
|
||
| // GET /ibms/definitions/{name} | ||
| var ibmsDefinitionsGetByName = controller(function(req) { | ||
| var schemaUid = nameSpace + req.swagger.params.name.value; | ||
| var schema = schemaApiService.getSchema(schemaUid); | ||
| if (schema) { | ||
| return schema; | ||
| } | ||
| throw new Errors.NotFoundError(schemaUid + ' Not Found'); | ||
| }); | ||
|
|
||
| module.exports = { | ||
| ibmsGet: ibmsGet, | ||
| ibmsPut: ibmsPut, | ||
| ibmsGetById: ibmsGetById, | ||
| ibmsPatchById: ibmsPatchById, | ||
| ibmsDeleteById: ibmsDeleteById, | ||
| ibmsDefinitionsGetAll: ibmsDefinitionsGetAll, | ||
| ibmsDefinitionsGetByName: ibmsDefinitionsGetByName | ||
| }; | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A minor suggestion, put the ibm together with obm will help people to compare the difference between them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this has been tested with these changes, i plan to keep them as is for now. Will change them in the snmp story.