-
Notifications
You must be signed in to change notification settings - Fork 77
remove node update based on enclosure node update #34
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
Merged
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,132 @@ | ||
| // Copyright 2015, EMC, Inc. | ||
|
|
||
| 'use strict'; | ||
|
|
||
| var di = require('di'); | ||
|
|
||
| module.exports = nodeApiServiceFactory; | ||
| di.annotate(nodeApiServiceFactory, new di.Provide('Http.Services.Api.Nodes')); | ||
| di.annotate(nodeApiServiceFactory, | ||
| new di.Inject( | ||
| 'Protocol.TaskGraphRunner', | ||
| 'Services.Waterline', | ||
| 'Errors', | ||
| 'Logger', | ||
| '_', | ||
| 'Promise' | ||
| ) | ||
| ); | ||
| function nodeApiServiceFactory( | ||
| taskGraphProtocol, | ||
| waterline, | ||
| Errors, | ||
| Logger, | ||
| _, | ||
| Promise | ||
| ) { | ||
| var logger = Logger.initialize(nodeApiServiceFactory); | ||
|
|
||
| function NodeApiService() { | ||
| } | ||
|
|
||
| /** | ||
| * Find Enclosure nodes which enclose compute node | ||
| * @param {Object} node | ||
| * @return {Promise} | ||
| */ | ||
| NodeApiService.prototype._findEnclNodes = function(node) { | ||
| // Find the enclosure nodes who enclose this compute node | ||
| if (!_.has(node, 'relations')) { | ||
| return Promise.resolve(); | ||
| } | ||
|
|
||
| var relation = _.find(node.relations, { relationType: 'enclosedBy' }); | ||
| if (!relation || !_.has(relation, 'targets') ) { | ||
| return Promise.resolve(); | ||
| } | ||
|
|
||
| return Promise.map(relation.targets, function (enclNodeId) { | ||
| return waterline.nodes.needByIdentifier(enclNodeId) | ||
| .catch(function (err) { | ||
| logger.error("Error Getting Enclosure Node", { error: err }); | ||
| return; | ||
| }); | ||
| }); | ||
| }; | ||
|
|
||
| /** | ||
| * Remove the relations between node and enclosure node, if enclosure node | ||
| * doesn't enclose any nodes, this enclosure node is also removed. | ||
| * @param {Object} node | ||
| * @param {Array} enclNodes | ||
| * @return {Promise} | ||
| */ | ||
| NodeApiService.prototype._removeEnclRelations = function(node, enclNodes) { | ||
| if (!enclNodes) { | ||
| return Promise.resolve(); | ||
| } | ||
|
|
||
| // Remove the relationship between enclosure and compute node | ||
| return Promise.map(enclNodes, function(enclNode) { | ||
| if (!_.has(enclNode, 'relations')) { | ||
| return; | ||
| } | ||
|
|
||
| var index = _.findIndex(enclNode.relations, { relationType: 'encloses' }); | ||
| if (index === -1 || !_.has(enclNode.relations[index], 'targets')) { | ||
| return; | ||
| } | ||
|
|
||
| if (_.indexOf(enclNode.relations[index].targets, node.id) !== -1) { | ||
| _.pull(enclNode.relations[index].targets, node.id); | ||
| } | ||
| // If Enclosure node doesn't have enclosed nodes | ||
| // remove this enclosure node, else remove relationships | ||
| if (enclNode.relations[index].targets.length === 0) { | ||
| return waterline.nodes.destroy({ id: enclNode.id }); | ||
| } else { | ||
| return waterline.nodes.updateByIdentifier( | ||
| enclNode.id, { relations : enclNode.relations }); | ||
| } | ||
| }); | ||
| }; | ||
|
|
||
| /** | ||
| * Remove node related data and remove its relations with other nodes | ||
| * @param {Object} node | ||
| * @return {Promise} | ||
| */ | ||
| NodeApiService.prototype.removeNode = function(node) { | ||
| var self = this; | ||
|
|
||
| return taskGraphProtocol.getActiveTaskGraph( { target: node.id }) | ||
| .then(function (graph) { | ||
| if (graph) { | ||
| throw new Errors.BadRequestError('Could not remove node ' + node.id + | ||
| ', active workflow is running'); | ||
| } | ||
| }) | ||
| .then(function () { | ||
| return Promise.settle([ | ||
| //lookups should be destoryed here, only clear node field | ||
| //as a workaround until the issue that when lookups are cleared | ||
| //it cannot be updated timely in nodes' next bootup is fixed | ||
| waterline.lookups.update({ node: node.id },{ node: '' } ), | ||
| waterline.nodes.destroy({ id: node.id }), | ||
| waterline.catalogs.destroy({ node: node.id }), | ||
| waterline.workitems.destroy({ node: node.id }) | ||
| ]); | ||
| }) | ||
| .then(function () { | ||
| return self._findEnclNodes(node); | ||
| }) | ||
| .then(function (enclNodes) { | ||
|
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. I think it would be better to make the code in these promise blocks their own named functions, that way we can more easily test their functionality individually, and also have a more easily readable promise chain. |
||
| return self._removeEnclRelations(node, enclNodes); | ||
| }) | ||
| .then(function () { | ||
| return node; | ||
| }); | ||
| }; | ||
|
|
||
| return new NodeApiService(); | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Good, glad we have a node api service now. Just as a note it might make sense going forward for us to start moving the rest of logic in the node routes into here (switch/pdu discovery, etc.).
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.
When I work on the bootstrap, I feel it is better if we can move the profile-api-service and template render service to the on-core, then some of the function (like rendering) can be shared in the job implementation (on-tasks).
Same to the nodes-api-service, in future we may have some internal logic to remove the nodes of a enclosure.
Of course, we can keep it here right now and move to on-core if we come across that situation.
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.
@yyscamper I don't think it's proper be put into on-core, some of the service would run specific taskgraph, it's not good that it appears in on-core.
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.
@anhou, the "Protocol.TaskGraphRunner" is just in on-core, so I think it's proper. Image future we implements the auto node removal, I think this node-api-service is exactly what we need for auto node removal.
Anyway, it's OK for me that move it to on-core after we come across that situation.