refactor southbound templates api #531

lanchongyizu · 2016-11-16T07:30:39Z

Background

Currently, RackHD is heavily coupled with and dependent on lookups, especially the 1 to 1 relationship between MAC and IP, which causes the static IP problems and maybe other problems.
There are two kinds of relationships between MAC and IP, which should be taken into consideration now.

one MAC address could map to multiple IP addresses, which exists in ARP cache.
one IP address could map to multiple MAC addresses, when users want to set multiple nodes with the same IP address, and it may exist in ARP cache sometime.

When the nodeId assigned, we can use it to identify the node rather than depend on lookups, since the latter introduces more complexity into lookups.
Also, MACAddress and maybe serial board number can be as identifiers.

Implementation

Thereafter, the PRs

#531 (this PR)
RackHD/on-tasks#374
RackHD/on-taskgraph#187
RackHD/on-skupack#57
RackHD/docs#362

are to refactor

/templates/:name

to

/templates/:name?nodeId=:nodeId
/templates/:name?macs=:macs

Additional work:

make option downloadUrl to use FULL PATH
remove postLookup() in esx-ks

Test

I have tested the PRs by successfully installing below OSes which would be functionally equal to Regression Test.
Note: For Windows Server 2012, you should regenerate a new WinPE image.

Ubuntu 14.04
ESXi 6.0
CentOS 6.5
CentOS 7.0
openSUSE 42.1
CoreOS
Windows Server 2012
Photon OS

@RackHD/corecommitters @panpan0000 @iceiilin @keedya @johren

houndci-bot · 2016-11-16T07:30:47Z

spec/lib/api/1.1/nodes-spec.js

    var configuration;
+    var templates;
+    var taskProtocol;
+    var commonApiPresenter;


'commonApiPresenter' is defined but never used.

houndci-bot · 2016-11-16T07:30:47Z

lib/api/1.1/southbound/nodes.js

+     * @api {get} /api/1.1/nodes/:indentifier/templates/:templateName GET /:indentifier/templates/:templateName
+     * @apiVersion 1.1.0
+     * @apiDescription used internally by the system -- will NOT the template with the
+     *                                  <code>templateName</code>, use /api/current/templates/library


Line is too long.

houndci-bot · 2016-11-16T07:30:47Z

lib/api/1.1/southbound/nodes.js

+    var router = express.Router();
+
+    /**
+     * @api {get} /api/1.1/nodes/:indentifier/templates/:templateName GET /:indentifier/templates/:templateName


Line is too long.

lanchongyizu · 2016-11-16T07:37:05Z

jenkins: depends on RackHD/on-tasks#374 RackHD/on-taskgraph#187

changev · 2016-11-16T10:56:27Z

test this please

A new setup vmslave took the test task unexpected, now has been fixed

lanchongyizu · 2016-11-18T09:24:09Z

test this please.

changev · 2016-11-18T10:20:59Z

test this please

brianparry · 2016-11-22T15:34:37Z

@keedya Would you mind taking a look through this? I know you had some similar ideas about using nodeId in place of a lookup.

brianparry

👍

brianparry · 2016-11-22T15:40:47Z

@lanchongyizu Changes look good, but we need to make sure that functional and regression tests pass before merging. Please work with @anhou and @yyscamper to coordinate the merge.

keedya · 2016-11-22T15:52:07Z

looks good 👍

yyscamper · 2016-11-22T23:48:50Z

lib/api/2.0/templates.js

-        ]);
-    })
-    .spread(function(workflow, node) {
+    var nodeId = req.swagger.query.nodeId;


some suggestion on follow-up work (not a must-to-have for this PR):

We had talked the unique identifier can be either of nodeId or mac or board-serial-number or something else, so I think there would be a function to handle all of these identifier.

Now your work only touches the /templates API, in future when touching more and more southbound APIs, you need to take care of reducing the code duplication. How to get the unique identifier (not only nodeId) from request should be some code code that shared by all southbound APIs or even should be configured in swagger as a fitting to let it works automatically.

We have created story https://www.pivotaltracker.com/story/show/134938873 to follow the suggestions.
Thanks.

yyscamper · 2016-11-22T23:49:47Z

spec/lib/api/2.0/templates-spec.js

            this.sandbox.stub(swagger, 'makeRenderableOptions').resolves({});
            taskProtocol.requestProperties.resolves({});
-            return helper.request().get('/api/2.0/templates/123')
+            return helper.request().get('/api/2.0/templates/123?nodeId=123')


Please add a negative test case to prove the API will fail if missing nodeId.

Done, thanks.

yyscamper

+1 after adding a negative test case.

cgx027

Looks good 👍

lanchongyizu · 2016-11-24T06:06:59Z

@yyscamper Currently, the Jenkins-Dependency-PR can't work properly.

lanchongyizu · 2016-11-24T13:35:23Z

test this please.

lanchongyizu · 2016-11-25T02:36:48Z

@brianparry OK, thanks for your review.

lanchongyizu · 2016-11-25T14:23:08Z

test this please.

houndci-bot · 2016-11-28T10:07:24Z

lib/services/templates-api-service.js

+'use strict';
+
+var di = require('di'),
+    ejs = require('ejs');


'ejs' is defined but never used.

houndci-bot · 2016-11-28T10:07:24Z

lib/services/templates-api-service.js

+    nodeApiService
+) {
+
+    var logger = Logger.initialize(templateApiServiceFactory);


'logger' is defined but never used.

houndci-bot · 2016-11-28T10:07:25Z

lib/services/nodes-api-service.js

+     */
+    NodeApiService.prototype.getNodeByIdentifier = function(identifier) {
+        return waterline.nodes.findByIdentifier(identifier);
+    }


Missing semicolon.

leoyjchang · 2016-11-29T07:52:18Z

@lanchongyizu Very appreciate you updated the PR description following the template, which makes people to understand what been done clearly. Awesome~~

…emplates/:name?macs=:macs * make option downloadUrl to use FULL PATH * remove postLookup() in esx-ks

JenkinsRHD · 2016-11-29T08:52:11Z

BUILD on-http #2890 : UNSTABLE

** BUILD build-config-setup #171987: SUCCESS
** BUILD unit-tests #10223: SUCCESS
** BUILD smoke-test #3858: UNSTABLE

BUILD smoke-test #3858 Error Logs ▼

Test Name: test_list_simple_storage Error Details: (404) Reason: Not Found HTTP response headers: HTTPHeaderDict({'Content-Length': '742', 'X-Powered-By': 'Express', 'Connection': 'keep-alive', 'ETag': 'W/"2e6-R1p0eb9wQS2jPLEGXhEJyA"', 'Date': 'Tue, 29 Nov 2016 08:50:00 GMT', 'Access-Control-Allow-Origin': '*', 'Content-Type': 'application/json; charset=utf-8'}) HTTP response body: {"error":{"code":"Base.1.0.GeneralError","message":"A general error has occurred. See ExtendedInfo for more information.","@Message.ExtendedInfo":[{"@odata.type":"#Message.1.0.0.Message","MessageId":"Base.1.0.Messages.InvalidObject","Description":"Indicates that the object in question is invalid according to the implementation. Examples include a firmware update malformed URI.","Message":"The object at %1 is invalid.","Resolution":"Either the object is malformed or the URI is not correct. Correct the condition and resubmit the request if it failed.","Severity":"Critical"},{"MessageId":"RackHD.1.0.DetailedErrorMessage","Message":"No Catalogs Found for Source (smart).","Description":"Contains the detailed error message contents"}]}}

Stack Trace: Traceback (most recent call last):
File "/usr/lib/python2.7/unittest/case.py", line 331, in run
testMethod()
File "/usr/lib/python2.7/unittest/case.py", line 1043, in runTest
self._testFunc()
File "/tmp/.venv/local/lib/python2.7/site-packages/proboscis/case.py", line 296, in testng_method_mistake_capture_func
compatability.capture_type_error(s_func)
File "/tmp/.venv/local/lib/python2.7/site-packages/proboscis/compatability/exceptions_2_6.py", line 27, in capture_type_error
func()
File "/tmp/.venv/local/lib/python2.7/site-packages/proboscis/case.py", line 350, in func
func(test_case.state.get_state())
File "/home/jenkins/workspace/on-http/RackHD/test/tests/api/redfish_1_0/systems_tests.py", line 110, in test_list_simple_storage
redfish().list_simple_storage(dataId)
File "/tmp/.venv/local/lib/python2.7/site-packages/on_http_redfish_1_0/apis/redfishv_api.py", line 4047, in list_simple_storage
callback=params.get('callback'))
File "/tmp/.venv/local/lib/python2.7/site-packages/on_http_redfish_1_0/api_client.py", line 322, in call_api
response_type, auth_settings, callback)
File "/tmp/.venv/local/lib/python2.7/site-packages/on_http_redfish_1_0/api_client.py", line 149, in __call_api
post_params=post_params, body=body)
File "/tmp/.venv/local/lib/python2.7/site-packages/on_http_redfish_1_0/api_client.py", line 342, in request
headers=headers)
File "/tmp/.venv/local/lib/python2.7/site-packages/on_http_redfish_1_0/rest.py", line 184, in GET
query_params=query_params)
File "/tmp/.venv/local/lib/python2.7/site-packages/on_http_redfish_1_0/rest.py", line 177, in request
raise ApiException(http_resp=r)
ApiException: (404)
Reason: Not Found
HTTP response headers: HTTPHeaderDict({'Content-Length': '742', 'X-Powered-By': 'Express', 'Connection': 'keep-alive', 'ETag': 'W/"2e6-R1p0eb9wQS2jPLEGXhEJyA"', 'Date': 'Tue, 29 Nov 2016 08:50:00 GMT', 'Access-Control-Allow-Origin': '*', 'Content-Type': 'application/json; charset=utf-8'})
HTTP response body: {"error":{"code":"Base.1.0.GeneralError","message":"A general error has occurred. See ExtendedInfo for more information.","@Message.ExtendedInfo":[{"@odata.type":"#Message.1.0.0.Message","MessageId":"Base.1.0.Messages.InvalidObject","Description":"Indicates that the object in question is invalid according to the implementation. Examples include a firmware update malformed URI.","Message":"The object at %1 is invalid.","Resolution":"Either the object is malformed or the URI is not correct. Correct the condition and resubmit the request if it failed.","Severity":"Critical"},{"MessageId":"RackHD.1.0.DetailedErrorMessage","Message":"No Catalogs Found for Source (smart).","Description":"Contains the detailed error message contents"}]}}

Test Name: test_catalogs
Error Details: Catalog smart not found in node 583d3f13d4bd2df6087fe2a3!
Stack Trace: Traceback (most recent call last):
File "/usr/lib/python2.7/unittest/case.py", line 331, in run
testMethod()
File "/usr/lib/python2.7/unittest/case.py", line 1043, in runTest
self._testFunc()
File "/tmp/.venv/local/lib/python2.7/site-packages/proboscis/case.py", line 296, in testng_method_mistake_capture_func
compatability.capture_type_error(s_func)
File "/tmp/.venv/local/lib/python2.7/site-packages/proboscis/compatability/exceptions_2_6.py", line 27, in capture_type_error
func()
File "/tmp/.venv/local/lib/python2.7/site-packages/proboscis/case.py", line 350, in func
func(test_case.state.get_state())
File "/home/jenkins/workspace/on-http/RackHD/test/tests/api/v2_0/catalogs_tests.py", line 48, in test_catalogs
fail('Catalog {0} not found in node {1}!'.format(source,node.get('id')))
File "/tmp/.venv/local/lib/python2.7/site-packages/proboscis/asserts.py", line 220, in fail
raise ASSERTION_ERROR(message)
AssertionError: Catalog smart not found in node 583d3f13d4bd2df6087fe2a3!

lanchongyizu · 2016-11-29T09:49:31Z

test this please.

JenkinsRHD · 2016-11-29T10:15:24Z

BUILD on-http #2891 : UNSTABLE

** BUILD build-config-setup #172055: SUCCESS
** BUILD unit-tests #10224: SUCCESS
** BUILD smoke-test #3859: UNSTABLE

BUILD smoke-test #3859 Error Logs ▼

Test Name: test_list_simple_storage Error Details: (404) Reason: Not Found HTTP response headers: HTTPHeaderDict({'Content-Length': '742', 'X-Powered-By': 'Express', 'Connection': 'keep-alive', 'ETag': 'W/"2e6-R1p0eb9wQS2jPLEGXhEJyA"', 'Date': 'Tue, 29 Nov 2016 10:13:03 GMT', 'Access-Control-Allow-Origin': '*', 'Content-Type': 'application/json; charset=utf-8'}) HTTP response body: {"error":{"code":"Base.1.0.GeneralError","message":"A general error has occurred. See ExtendedInfo for more information.","@Message.ExtendedInfo":[{"@odata.type":"#Message.1.0.0.Message","MessageId":"Base.1.0.Messages.InvalidObject","Description":"Indicates that the object in question is invalid according to the implementation. Examples include a firmware update malformed URI.","Message":"The object at %1 is invalid.","Resolution":"Either the object is malformed or the URI is not correct. Correct the condition and resubmit the request if it failed.","Severity":"Critical"},{"MessageId":"RackHD.1.0.DetailedErrorMessage","Message":"No Catalogs Found for Source (smart).","Description":"Contains the detailed error message contents"}]}}

Stack Trace: Traceback (most recent call last):
File "/usr/lib/python2.7/unittest/case.py", line 331, in run
testMethod()
File "/usr/lib/python2.7/unittest/case.py", line 1043, in runTest
self._testFunc()
File "/tmp/.venv/local/lib/python2.7/site-packages/proboscis/case.py", line 296, in testng_method_mistake_capture_func
compatability.capture_type_error(s_func)
File "/tmp/.venv/local/lib/python2.7/site-packages/proboscis/compatability/exceptions_2_6.py", line 27, in capture_type_error
func()
File "/tmp/.venv/local/lib/python2.7/site-packages/proboscis/case.py", line 350, in func
func(test_case.state.get_state())
File "/home/jenkins/workspace/on-http/RackHD/test/tests/api/redfish_1_0/systems_tests.py", line 110, in test_list_simple_storage
redfish().list_simple_storage(dataId)
File "/tmp/.venv/local/lib/python2.7/site-packages/on_http_redfish_1_0/apis/redfishv_api.py", line 4047, in list_simple_storage
callback=params.get('callback'))
File "/tmp/.venv/local/lib/python2.7/site-packages/on_http_redfish_1_0/api_client.py", line 322, in call_api
response_type, auth_settings, callback)
File "/tmp/.venv/local/lib/python2.7/site-packages/on_http_redfish_1_0/api_client.py", line 149, in __call_api
post_params=post_params, body=body)
File "/tmp/.venv/local/lib/python2.7/site-packages/on_http_redfish_1_0/api_client.py", line 342, in request
headers=headers)
File "/tmp/.venv/local/lib/python2.7/site-packages/on_http_redfish_1_0/rest.py", line 184, in GET
query_params=query_params)
File "/tmp/.venv/local/lib/python2.7/site-packages/on_http_redfish_1_0/rest.py", line 177, in request
raise ApiException(http_resp=r)
ApiException: (404)
Reason: Not Found
HTTP response headers: HTTPHeaderDict({'Content-Length': '742', 'X-Powered-By': 'Express', 'Connection': 'keep-alive', 'ETag': 'W/"2e6-R1p0eb9wQS2jPLEGXhEJyA"', 'Date': 'Tue, 29 Nov 2016 10:13:03 GMT', 'Access-Control-Allow-Origin': '*', 'Content-Type': 'application/json; charset=utf-8'})
HTTP response body: {"error":{"code":"Base.1.0.GeneralError","message":"A general error has occurred. See ExtendedInfo for more information.","@Message.ExtendedInfo":[{"@odata.type":"#Message.1.0.0.Message","MessageId":"Base.1.0.Messages.InvalidObject","Description":"Indicates that the object in question is invalid according to the implementation. Examples include a firmware update malformed URI.","Message":"The object at %1 is invalid.","Resolution":"Either the object is malformed or the URI is not correct. Correct the condition and resubmit the request if it failed.","Severity":"Critical"},{"MessageId":"RackHD.1.0.DetailedErrorMessage","Message":"No Catalogs Found for Source (smart).","Description":"Contains the detailed error message contents"}]}}

Test Name: test_catalogs
Error Details: Catalog smart not found in node 583d528dab9f5ec308385a55!
Stack Trace: Traceback (most recent call last):
File "/usr/lib/python2.7/unittest/case.py", line 331, in run
testMethod()
File "/usr/lib/python2.7/unittest/case.py", line 1043, in runTest
self._testFunc()
File "/tmp/.venv/local/lib/python2.7/site-packages/proboscis/case.py", line 296, in testng_method_mistake_capture_func
compatability.capture_type_error(s_func)
File "/tmp/.venv/local/lib/python2.7/site-packages/proboscis/compatability/exceptions_2_6.py", line 27, in capture_type_error
func()
File "/tmp/.venv/local/lib/python2.7/site-packages/proboscis/case.py", line 350, in func
func(test_case.state.get_state())
File "/home/jenkins/workspace/on-http/RackHD/test/tests/api/v2_0/catalogs_tests.py", line 48, in test_catalogs
fail('Catalog {0} not found in node {1}!'.format(source,node.get('id')))
File "/tmp/.venv/local/lib/python2.7/site-packages/proboscis/asserts.py", line 220, in fail
raise ASSERTION_ERROR(message)
AssertionError: Catalog smart not found in node 583d528dab9f5ec308385a55!

houndci-bot reviewed Nov 16, 2016

View reviewed changes

lanchongyizu mentioned this pull request Nov 16, 2016

refactor southbound templates api RackHD/on-tasks#374

Merged

lanchongyizu force-pushed the refactor-southbound-template-api branch 2 times, most recently from 46691a8 to 19baff4 Compare November 16, 2016 09:37

lanchongyizu force-pushed the refactor-southbound-template-api branch from 19baff4 to c6fa250 Compare November 17, 2016 10:09

This was referenced Nov 18, 2016

make option downloadUrl to use FULL PATH RackHD/on-skupack#57

Merged

make option downloadUrl to use FULL PATH RackHD/on-taskgraph#187

Merged

lanchongyizu force-pushed the refactor-southbound-template-api branch from c6fa250 to c21bac2 Compare November 18, 2016 08:08

lanchongyizu force-pushed the refactor-southbound-template-api branch 2 times, most recently from c4e901c to b786882 Compare November 21, 2016 10:16

brianparry approved these changes Nov 22, 2016

View reviewed changes

yyscamper reviewed Nov 22, 2016

View reviewed changes

yyscamper approved these changes Nov 22, 2016

View reviewed changes

yyscamper mentioned this pull request Nov 23, 2016

Updated SKU PACK for cisco-nexus-C9332PQ RackHD/on-skupack#58

Merged

cgx027 approved these changes Nov 24, 2016

View reviewed changes

lanchongyizu force-pushed the refactor-southbound-template-api branch 2 times, most recently from 48bf699 to 27f8016 Compare November 24, 2016 09:53

lanchongyizu force-pushed the refactor-southbound-template-api branch 2 times, most recently from 4a4f468 to 2611de0 Compare November 25, 2016 13:21

lanchongyizu force-pushed the refactor-southbound-template-api branch from 2611de0 to 29b2f64 Compare November 28, 2016 10:07

houndci-bot reviewed Nov 28, 2016

View reviewed changes

lanchongyizu force-pushed the refactor-southbound-template-api branch 2 times, most recently from f53cc4d to eac0ad4 Compare November 29, 2016 02:04

lanchongyizu mentioned this pull request Nov 29, 2016

refactor southbound templates api RackHD/docs#362

Merged

lanchongyizu mentioned this pull request Nov 29, 2016

Remove postLookup() logic, and fix the failure of installing Esxi when setting static IP. #500

Closed

* refactor /templates/:name to /templates/:name?nodeId=:nodeId and /t…

71cc464

…emplates/:name?macs=:macs * make option downloadUrl to use FULL PATH * remove postLookup() in esx-ks

lanchongyizu force-pushed the refactor-southbound-template-api branch from eac0ad4 to 71cc464 Compare November 29, 2016 08:20

anhou merged commit dc86cc0 into RackHD:master Nov 30, 2016

JenkinsRHD mentioned this pull request Sep 15, 2017

Redfish refactor #776

Merged

refactor southbound templates api #531

refactor southbound templates api #531

Uh oh!

Conversation

lanchongyizu commented Nov 16, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Background

Implementation

Test

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lanchongyizu commented Nov 16, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

changev commented Nov 16, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lanchongyizu commented Nov 18, 2016

Uh oh!

changev commented Nov 18, 2016

Uh oh!

brianparry commented Nov 22, 2016

Uh oh!

brianparry left a comment

Choose a reason for hiding this comment

Uh oh!

brianparry commented Nov 22, 2016

Uh oh!

keedya commented Nov 22, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yyscamper left a comment

Choose a reason for hiding this comment

Uh oh!

cgx027 left a comment

Choose a reason for hiding this comment

Uh oh!

lanchongyizu commented Nov 24, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lanchongyizu commented Nov 24, 2016

Uh oh!

lanchongyizu commented Nov 25, 2016

Uh oh!

lanchongyizu commented Nov 25, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

leoyjchang commented Nov 29, 2016

Uh oh!

JenkinsRHD commented Nov 29, 2016

Uh oh!

lanchongyizu commented Nov 29, 2016

Uh oh!

JenkinsRHD commented Nov 29, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

lanchongyizu commented Nov 16, 2016 •

edited

Loading

lanchongyizu commented Nov 16, 2016 •

edited

Loading

changev commented Nov 16, 2016 •

edited

Loading

lanchongyizu commented Nov 24, 2016 •

edited

Loading