search

odo-devfiles / registry

A repository of devfile 2.0 stacks for odo
7 stars 33 forks source link

update nodejs devfile #7

Closed skoh7645 closed 4 years ago

skoh7645 commented 4 years ago

Update the nodejs devfile to work with the changes to the stack made here: https://github.com/odo-devfiles/nodejs-ex/pull/1. This PR also adds the deploy artefacts, for now these files will do nothing until the implementation for odo deploy has gone in. I have the tested this devfile.yaml with the current master branch of odo and it does not break anything

skoh7645 commented 4 years ago

In general, how generic this stack is? Is it supposed to only work with the provided example or is it supposed to work with other node js apps? I just tried this stack with the original node sample that we currently have https://github.com/odo-devfiles/nodejs-ex (master), the app failed to start up. I'd expect the stack to be generic and be able to work with a range of apps.

This stack should work with any nodejs app, not just the provided example. I think it didn't work with the original node sample because this devfile runs npm start in the run command, but no start command has been set in the package.json here https://github.com/odo-devfiles/nodejs-ex/blob/master/package.json

neeraj-laad commented 4 years ago

Running npm start makes npm look for a start script in package.json.

So, the devfile in this PR is more generic that the existing one as it calls npm start instead of node app.js giving the user flexibility to call their app anything and provide a startup script.

maysunfaisal commented 4 years ago

@skoh7645 this PR reverted a lot of things that are breaking tests on odo repo:

  1. npm start does not actually start the app - this was fixed before in PR https://github.com/odo-devfiles/registry/pull/4. With the current change, my pod log says: 
    
$ oc logs -f mynode-5cb7fbcdc-g7xsf
time="2020-06-30T19:15:53Z" level=info msg="create process:devrun" 
time="2020-06-30T19:15:53Z" level=info msg="create process:debugrun" 
time="2020-06-30T19:16:02Z" level=debug msg="no auth required" 
time="2020-06-30T19:16:02Z" level=info msg="stop the program" program=devrun 
time="2020-06-30T19:16:02Z" level=info msg="stop the program" program=debugrun 
time="2020-06-30T19:16:02Z" level=info msg="force to kill the program" program=devrun 
time="2020-06-30T19:16:02Z" level=info msg="force to kill the program" program=debugrun 
time="2020-06-30T19:16:02Z" level=debug msg="no auth required" 
time="2020-06-30T19:16:02Z" level=debug msg="succeed to find process:devrun" 
time="2020-06-30T19:16:02Z" level=info msg="try to start program" program=devrun 
time="2020-06-30T19:16:02Z" level=info msg="success to start program" program=devrun 
ODO_COMMAND_RUN is npm start
Changing directory to ${CHE_PROJECTS_ROOT}/nodejs-starter
Executing command cd ${CHE_PROJECTS_ROOT}/nodejs-starter && npm start
npm ERR! missing script: start

npm ERR! A complete log of this run can be found in: npm ERR! /opt/app-root/src/.npm/_logs/2020-06-30T19_16_03_320Z-debug.log time="2020-06-30T19:16:03Z" level=debug msg="wait program exit" program=devrun time="2020-06-30T19:16:03Z" level=error msg="program stopped with error:exit status 1" program=devrun time="2020-06-30T19:16:03Z" level=info msg="Don't start the stopped program because its retry times 0 is greater than start retries 0" program=devrun 


I confirmed this by exec into the pod and manually executing the commands:

bash-4.4$ pwd /projects/nodejs-starter bash-4.4$ ls LICENSE README.md app devfile.yaml node_modules package-lock.json package.json bash-4.4$ ls -la total 60 drwxr-xr-x. 4 default root 169 Jun 30 19:15 . drwxrwxrwx. 3 root root 28 Jun 30 19:15 .. -rw-r--r--. 1 default root 6148 Apr 29 19:18 .DS_Store -rw-r--r--. 1 default root 48 Jun 30 19:14 .gitignore -rw-r--r--. 1 default root 14197 Apr 28 16:53 LICENSE -rw-r--r--. 1 default root 449 Apr 28 16:53 README.md drwxr-xr-x. 2 default root 20 Jun 30 19:15 app -rw-r--r--. 1 default root 1277 Jun 30 19:14 devfile.yaml drwxr-xr-x. 52 default root 4096 Jun 30 19:16 node_modules -rw-r--r--. 1 default root 14289 Apr 28 16:53 package-lock.json -rw-r--r--. 1 default root 265 Apr 28 16:53 package.json bash-4.4$ bash-4.4$ npm install npm WARN nodejs-sample@1.0.0 No repository field.

audited 50 packages in 1.054s found 0 vulnerabilities

bash-4.4$ bash-4.4$ bash-4.4$ npm start npm ERR! missing script: start

npm ERR! A complete log of this run can be found in: npm ERR! /opt/app-root/src/.npm/_logs/2020-06-30T19_21_28_505Z-debug.log



Are you on a different template and the devfile.yaml needs to be updated?(EDIT- just checked that the template had been updated)

2. Also changing the project name to `nodejs-starter` broke some odo tests on the odo repo
elsony commented 4 years ago
  1. npm start should start the application as well if the node app is "well-defined", i.e. either a server.js or defining the "start" script in the node app. (see https://github.com/odo-devfiles/registry/pull/7#issuecomment-651797488). The current test broken issue is caused by the PR test is using a static copy of the test instead of using the current one from the repo. We'll fix that on the odo PR test side to sync up to the latest project.
  2. For our test, why do we need to hardcode the project name? I can see we need the node stack name to stay the same, but why the project name that a stack can change at any tiime?