nodejs, remove runtime internal data from sandbox, avoid code concat

[cscheiber]

Issue Ref:
#632
Description:
Please see issue for details.

TODOs:

• Ready to review
• Automated Tests
• Docs

[andresmgot]

Just submitted some small suggestions. The approach looks good so far :)

[andresmgot]

here is where we could add the possibility of importing different functions. What I have tried:

    else if (p === 'kubeless')
        return (handler) => {
            let func = null
            if (typeof handler === "function") {
                func = handler;
            } else if (typeof handler === "object") {
                func = handler[funcHandler];
            } else {
                throw new Error(`Unable to load ${handler}`);
            }
            modExecute(handler, req, res, end)
        };

[cscheiber]

Yes, would work. Map could also be supported. Better check also for null, e.g.

} else if (handler && typeof handler === "object") {

Just a question: if there are multiple functions in one file, would kubeless also allow to run it in the same pod, e.g. take the FUNC_NAME from request and use it as key? Or is it just about developers convenience to use one file.

[cscheiber]

And maybe put (p === 'kubeless') to the begin to override any 'kubeless' from functions package.json.

[cscheiber]

modExecute(func, req, res, end)

[andresmgot]

Just a question: if there are multiple functions in one file, would kubeless also allow to run it in the same pod, e.g. take the FUNC_NAME from request and use it as key? Or is it just about developers convenience to use one file.

The second, it is just for convenience but only one function will run per pod.

[andresmgot]

you missed the data parsing here:

        if (req.get('content-type') === 'application/json') {
            event.data = JSON.parse(event.data)
        }

[cscheiber]

Sorry, could be a helper function.

[andresmgot]

I would move the context initialization out of the modExecute function so it is executed just once (instead of one per request). That way we can save some ms.

[cscheiber]

Yes. And at this point we could also check the Promise callbacks. Instead of (err) => { handleError(err); } just err => handleError(err, ...) at least under node 6 a little faster to my knowledge. Of course also the typical case, the success with then result => handleResult(result) using an additional plain function.

[cscheiber]

Added a commit according to comments:

• handler check is done inside modExecute, to keep concise arrow function body
• timer end is also called in handleError()

[andresmgot]

Thank you @cscheiber. The code LGTM. Now we need some testing before we merge this. Could you:

• Update the examples under ./examples/nodejs/*
• Build images for node6 and node8 and push them to your personal repository (or any public docker hub repository)
• Modify the references in kubeless.jsonnet for the images you just pushed
• Extra: In the file ./examples/Makefile there is a task timeout-nodejs that writes a nodejs function with an infinite loop, you will need to adapt it to the new format.

Once you do the above the CI will test everything with your changes included.

[cscheiber]

As discussed I adjusted the examples, but did not commit so far. The file 'index.js' leads to a question: if module.exports=myHandler is replaced by kubeless(myHandler) in the single files - how to deal with index.js in general?

[cscheiber]

To summarize today's discussions:

• one code line will be generated at the end of the main file require('kubeless')(module.exports);
• function file structure does not need to change
• index.js can be used, however it is not recommended to provide e.g. 50 functions with one index file where runtime will then select just the one to be used
• sandbox works only for the main file, not for any required module (unchanged behavior)

A final commit has been provided.

[andresmgot]

@cscheiber I have pushed the images to my repository, do you mind changing the reference of the images in the kubeless.jsonnet file to the following?

    "ID": "nodejs",
    "versions": [
      {
       	"name": "node6",
        "version": "6",
        "runtimeImage": "andresmgot/nodejs@sha256:37d496adee0896a4a192f7116d8075ae88ee14b1a3c41974d79609e0863ae464",
        "initImage": "node:6.10"
      },
      {
        "name": "node8",
	"version": "8",
        "runtimeImage": "andresmgot/nodejs@sha256:4327d0f7781463a8b0a843000592c2c0df17988662b38b33dbb35b1bdd5ccc2b",
        "initImage": "node:8"
      }
    ],

[cscheiber]

The file has been updated accordingly. Again, sorry that I had no own repository available for now.

[andresmgot]

Everything green in the CI so I pushed the images to the kubeless repository. I will leave the CI to run one more time and I will merge the changes. Thanks @cscheiber !