Refactor OS installation progress

data/profiles/install-centos.ipxe

@@ -1,11 +1,23 @@
 echo Starting CentOS/RHEL <%=version%> installer for ${hostidentifier}
+
+# The progress notification is just something nice-to-have, so progress notification failure should
+# never impact the normal installation process
+<% if( typeof progressMilestones !== 'undefined' && progressMilestones.enterProfileUri ) { %>
+    # since there is no curl like http client in ipxe, so use imgfetch instead
+    # note: the progress milestones uri must be wrapped in unescaped format, otherwise imgfetch will fail
+    imgfetch --name fakedimage http://<%=server%>:<%=port%><%-progressMilestones.enterProfileUri%> ||
+    imgfree fakedimage ||
+<% } %>
+
 set base-url <%=repo%>/images/pxeboot
 set params initrd=initrd.img ks=<%=installScriptUri%> hostname=<%=hostname%> ksdevice=bootif BOOTIF=01-${netX/mac} console=<%=comport%>,115200n8 console=tty0
 kernel ${base-url}/vmlinuz repo=<%=repo%> ${params}
 initrd ${base-url}/initrd.img
 
-imgfetch --name fakedimage http://<%=server%>:<%=port%>/api/current/notification/progress?taskId=<%=taskId%>&totalSteps=<%=totalSteps%>&currentStep=2&description=kernel+download+done%2C+starting+initiating+installer
-imgfree fakedimage
+<% if( typeof progressMilestones !== 'undefined' && progressMilestones.startInstallerUri ) { %>
+    imgfetch --name fakedimage http://<%=server%>:<%=port%><%-progressMilestones.startInstallerUri%> ||
+    imgfree fakedimage ||
+<% } %>
 
 boot || prompt --key 0x197e --timeout 2000 Press F12 to investigate || exit shell
 

data/templates/centos-ks

@@ -101,11 +101,22 @@ net-tools
 %end
 
 %pre
-/usr/bin/curl -X POST -H 'Content-Type:application/json' -d "{\"taskId\": \"<%=taskId%>\", \"progress\": { \"description\": \"Installer started, starting OS installtion\", \"maximum\": \"<%=totalSteps%>\", \"value\": 3}}" http://<%=server%>:<%=port%>/api/current/notification;
+# The progress notification is just something nice-to-have, so progress notification failure should
+# never impact the normal installation process
+<% if( typeof progressMilestones !== 'undefined' && progressMilestones.preConfigUri ) { %>
+    # the url may contain query, the symbol '&' will mess the command line logic, so the whole url need be wrapped in quotation marks
+    /usr/bin/curl -X POST -H 'Content-Type:application/json' "http://<%=server%>:<%=port%><%-progressMilestones.preConfigUri%>" || true
+<% } %>
 %end
 
 %post --log=/root/install-post.log
 (
+#notify the current progress
+<% if( typeof progressMilestones !== 'undefined' && progressMilestones.postConfigUri ) { %>
+    # the url may contain query, the symbol '&' will mess the command line logic, so the whole url need be wrapped in quotation marks
+    /usr/bin/curl -X POST -H 'Content-Type:application/json' "http://<%=server%>:<%=port%><%-progressMilestones.postConfigUri%>" || true
+<% } %>
+
 # PLACE YOUR POST DIRECTIVES HERE
 PATH=/bin:/sbin:/usr/bin:/usr/sbin
 export PATH

lib/api/2.0/notification.js

@@ -6,30 +6,43 @@ var injector = require('../../../index.js').injector;
 var controller = injector.get('Http.Services.Swagger').controller;
 var notificationApiService = injector.get('Http.Services.Api.Notification');
 var _ = injector.get('_');    // jshint ignore:line
+var Promise = injector.get('Promise');
+var assert = injector.get('Assert');
 
 var notificationPost = controller({success: 201}, function(req) {
     var message = _.defaults(req.swagger.query || {}, req.query || {}, req.body || {});
     return notificationApiService.postNotification(message);
 });
 
 /**
- * @api {get} /api/2.0/notification/progress
+ * @api {post} /api/2.0/notification/progress
  * @apiDescription deeply customized notification for task progress
  *  :taskId: active (OS installation) taskId
- *  :totalSteps: total steps for the task
- *  :currentStep: current setp sequence the API stands for
- * @apiName notification get
+ *  :maximum: the maximum progress value
+ *  :value: the current progress value
+ * @apiName notification post
  * @apiGroup notification
  */
-var notificationProgressGet = controller(function(req, res){
-    var message = _.defaults(req.swagger.query || {}, req.query || {});
-    return notificationApiService.postNotification({
-        taskId: message.taskId,
-        progress: {
-            maximum: message.totalSteps,
-            value: message.currentStep,
-            description: message.description 
+var notificationProgressPost = controller(function(req, res){
+    var message = _.defaults(req.swagger.query || {}, req.query || {}, req.body || {});
+    return Promise.try(function() {
+        assert.string(message.taskId, 'taskId is required for progress notification');
+        assert.ok(message.maximum, 'maximum is required for progress notification');
+        assert.ok(message.value, 'maximum is required for progress notification');
+
+        if (message.value) {
+            message.value = parseInt(message.value);
         }
+        if (message.maximum) {
+            message.maximum = parseInt(message.maximum);
+        }
+        return {
+            taskId: message.taskId,
+            progress: _.pick(message, ['maximum', 'value', 'description'])
+        };
+    })
+    .then(function(progressData) {
+        return notificationApiService.postNotification(progressData);
     })
     .then(function(){
         //Send any feedback is OK, just to cheat ipxe engine
@@ -39,5 +52,5 @@ var notificationProgressGet = controller(function(req, res){
 
 module.exports = {
     notificationPost: notificationPost,
-    notificationProgressGet: notificationProgressGet
+    notificationProgressPost: notificationProgressPost
 };

lib/services/common-api-presenter.js

@@ -157,6 +157,11 @@ function CommonApiPresenterFactory(
             try {
                 output = ejs.render(template.contents, options);
             } catch (err) {
+                logger.error('Unable to render template', {
+                    error: err,
+                    templateName: name,
+                    context: options
+                });
                 return self.renderError(err, options);
             }
             self.res.status(status).send(output);
@@ -208,9 +213,10 @@ function CommonApiPresenterFactory(
                     } catch (error) {
                         // If we failed to render the error then we've got larger problems.
 
-                        logger.error('Unable to render error template.', {
-                            macaddress: options.macaddress,
-                            error: error
+                        logger.error('Unable to render profile', {
+                            error: error,
+                            profileName: profile,
+                            context: options
                         });
 
                         return Promise.reject(error);

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

@@ -87,29 +87,188 @@ describe('Http.Api.Notification', function () {
         });
     });
 
+    describe('POST /notification/progress', function () {
+        beforeEach(function() {
+            sinon.stub(notificationApiService, 'postNotification').resolves();
+        });
+
+        afterEach(function() {
+            notificationApiService.postNotification.restore();
+        });
+
+        it('should post progress notification via body', function () {
+            return helper.request()
+            .post('/api/2.0/notification/progress')
+            .set('Content-Type', 'application/json')
+            .send({
+                taskId: 'test',
+                maximum: 5,
+                value: 2,
+                description: 'foo bar'
+            })
+            .expect(200)
+            .expect(function(res){
+                expect(res.text).to.equal('Notification response, no file will be sent');
+            })
+            .then(function() {
+                expect(notificationApiService.postNotification).to.be.calledWith({
+                    taskId: 'test',
+                    progress: {
+                        maximum: 5,
+                        value: 2,
+                        description: 'foo bar'
+                    }
+                });
+            });
+        });
+
+        it('should post progress notification via query', function () {
+            return helper.request()
+            .post('/api/2.0/notification/progress?taskId=testid&maximum=5&value=2&description=foo%20bar%20%202') //jshint ignore: line
+            .set('Content-Type', 'application/json')
+            .expect(200)
+            .expect(function(res){
+                expect(res.text).to.equal('Notification response, no file will be sent');
+            })
+            .then(function() {
+                expect(notificationApiService.postNotification).to.be.calledWith({
+                    taskId: 'testid',
+                    progress: {
+                        maximum: 5,
+                        value: 2,
+                        description: 'foo bar  2'
+                    }
+                });
+            });
+        });
+
+        it('should return 400 if taskId is missing in query', function() {
+            return helper.request()
+            .post('/api/2.0/notification/progress?maximum=5&value=2&description=foo')
+            .set('Content-Type', 'application/json')
+            .expect(400);
+        });
+
+        it('should return 400 if maximum is missing in query', function() {
+            return helper.request()
+            .post('/api/2.0/notification/progress?taskId=testid&value=2&description=foo')
+            .set('Content-Type', 'application/json')
+            .expect(400);
+        });
+
+        it('should return 400 if value is missing in query', function() {
+            return helper.request()
+            .post('/api/2.0/notification/progress?taskId=testid&maximum=4&description=foo')
+            .set('Content-Type', 'application/json')
+            .expect(400);
+        });
+
+        it('should be success if description is missing in query', function() {
+            return helper.request()
+            .post('/api/2.0/notification/progress?taskId=testid&maximum=4&value=2')
+            .set('Content-Type', 'application/json')
+            .expect(200);
+        });
+
+        it('should return 400 if taskId is missing in body', function() {
+            return helper.request()
+            .post('/api/2.0/notification/progress')
+            .send({
+                maximum: 5,
+                value: 2,
+                description: 'foo bar'
+            })
+            .set('Content-Type', 'application/json')
+            .expect(400);
+        });
+
+        it('should return 400 if maximum is missing in body', function() {
+            return helper.request()
+            .post('/api/2.0/notification/progress')
+            .send({
+                taskId: 'test',
+                value: 2,
+                description: 'foo bar'
+            })
+            .set('Content-Type', 'application/json')
+            .expect(400);
+        });
+
+        it('should return 400 if value is missing in body', function() {
+            return helper.request()
+            .post('/api/2.0/notification/progress')
+            .send({
+                taskId: 'test',
+                maximum: 5,
+                description: 'foo bar'
+            })
+            .set('Content-Type', 'application/json')
+            .expect(400);
+        });
+
+        it('should be success if description is missing in body', function() {
+            return helper.request()
+            .post('/api/2.0/notification/progress')
+            .send({
+                taskId: 'test',
+                maximum: 5,
+                value: 2
+            })
+            .set('Content-Type', 'application/json')
+            .expect(200);
+        });
+    });
+
     describe('GET /notification/progress', function () {
-        var descript = "kernel download done, starting initiating installer";
-        var progress = {
-            taskId: 'taskid',
-            progress:
-                {maximum: "5", value: "2", description: descript}
-        };
-        before(function(){
+        beforeEach(function() {
             sinon.stub(notificationApiService, 'postNotification').resolves();
         });
 
-        it('should post progress notification', function () {
+        afterEach(function() {
+            notificationApiService.postNotification.restore();
+        });
+
+        it('should update progress notification via query', function () {
             return helper.request()
-            .get('/api/2.0/notification/progress?taskId=taskid&totalSteps=5&currentStep=2' +
-                 '&description=kernel+download+done%2C+starting+initiating+installer')
+            .get('/api/2.0/notification/progress?taskId=testid&maximum=5&value=2&description=foo%20bar%20%202') //jshint ignore: line
             .expect(200)
             .expect(function(res){
                 expect(res.text).to.equal('Notification response, no file will be sent');
             })
             .then(function() {
-                expect(notificationApiService.postNotification).to.be.calledWith(progress);
+                expect(notificationApiService.postNotification).to.be.calledWith({
+                    taskId: 'testid',
+                    progress: {
+                        maximum: 5,
+                        value: 2,
+                        description: 'foo bar  2'
+                    }
+                });
             });
         });
 
+        it('should return 400 if taskId is missing', function() {
+            return helper.request()
+            .get('/api/2.0/notification/progress?maximum=5&value=2&description=foo')
+            .expect(400);
+        });
+
+        it('should return 400 if maximum is missing', function() {
+            return helper.request()
+            .get('/api/2.0/notification/progress?taskId=testid&value=2&description=foo')
+            .expect(400);
+        });
+
+        it('should return 400 if value is missing', function() {
+            return helper.request()
+            .get('/api/2.0/notification/progress?taskId=testid&maximum=4&description=foo')
+            .expect(400);
+        });
+
+        it('should be success if description is missing', function() {
+            return helper.request()
+            .get('/api/2.0/notification/progress?taskId=testid&maximum=4&value=2')
+            .expect(200);
+        });
     });
 });