Knative should not migrate traffic to revision if it fails to be created

[steren]

My knative version is 0.3

I created a new revision with an image that failed to start, I see its status:

  - type: ContainerHealthy
    status: 'False'
    reason: ExitCode1
    message: "Container failed with: \n> knative-serving-helloworld@1.0.0 start /usr/src/app\n\
      > node index.js\n\n/usr/src/app/index.js:10\n  results[\"memory.usage_in_bytes\"\
      ] = = fs.readFileSync(\"/sys/fs/cgroup/memory/memory.usage_in_bytes\", 'utf8');\n\
      \                                     ^\n\nSyntaxError: Unexpected token =\n\
      \    at new Script (vm.js:80:7)\n    at createScript (vm.js:274:10)\n    at\
      \ Object.runInThisContext (vm.js:326:10)\n    at Module._compile (internal/modules/cjs/loader.js:664:28)\n\
      \    at Object.Module._extensions..js (internal/modules/cjs/loader.js:712:10)\n\
      \    at Module.load (internal/modules/cjs/loader.js:600:32)\n    at tryModuleLoad\
      \ (internal/modules/cjs/loader.js:539:12)\n    at Function.Module._load (internal/modules/cjs/loader.js:531:3)\n\
      \    at Function.Module.runMain (internal/modules/cjs/loader.js:754:12)\n  \
      \  at startup (internal/bootstrap/node.js:283:19)\nnpm ERR! code ELIFECYCLE\n\
      npm ERR! errno 1\nnpm ERR! knative-serving-helloworld@1.0.0 start: `node index.js`\n\
      npm ERR! Exit status 1\nnpm ERR! \nnpm ERR! Failed at the knative-serving-helloworld@1.0.0\
      \ start script.\nnpm ERR! This is probably not a problem with npm. There is\
      \ likely additional logging output above.\n\nnpm ERR! A complete log of this\
      \ run can be found in:\nnpm ERR!     /root/.npm/_logs/2019-03-20T05_28_14_133Z-debug.log\n"
    lastTransitionTime: '2019-03-20T05:28:30Z'
  - type: Ready
    status: 'False'
    reason: ExitCode1
    message: "Container failed with: \n> knative-serving-helloworld@1.0.0 start /usr/src/app\n\
      > node index.js\n\n/usr/src/app/index.js:10\n  results[\"memory.usage_in_bytes\"\
      ] = = fs.readFileSync(\"/sys/fs/cgroup/memory/memory.usage_in_bytes\", 'utf8');\n\
      \                                     ^\n\nSyntaxError: Unexpected token =\n\
      \    at new Script (vm.js:80:7)\n    at createScript (vm.js:274:10)\n    at\
      \ Object.runInThisContext (vm.js:326:10)\n    at Module._compile (internal/modules/cjs/loader.js:664:28)\n\
      \    at Object.Module._extensions..js (internal/modules/cjs/loader.js:712:10)\n\
      \    at Module.load (internal/modules/cjs/loader.js:600:32)\n    at tryModuleLoad\
      \ (internal/modules/cjs/loader.js:539:12)\n    at Function.Module._load (internal/modules/cjs/loader.js:531:3)\n\
      \    at Function.Module.runMain (internal/modules/cjs/loader.js:754:12)\n  \
      \  at startup (internal/bootstrap/node.js:283:19)\nnpm ERR! code ELIFECYCLE\n\
      npm ERR! errno 1\nnpm ERR! knative-serving-helloworld@1.0.0 start: `node index.js`\n\
      npm ERR! Exit status 1\nnpm ERR! \nnpm ERR! Failed at the knative-serving-helloworld@1.0.0\
      \ start script.\nnpm ERR! This is probably not a problem with npm. There is\
      \ likely additional logging output above.\n\nnpm ERR! A complete log of this\
      \ run can be found in:\nnpm ERR!     /root/.npm/_logs/2019-03-20T05_28_14_133Z-debug.log\n"
    lastTransitionTime: '2019-03-20T05:28:30Z'

Observed:

The Service (in runLatest) is rolling out this failed revision and migrated 100% of the traffic to it.
Service seems to be Ready = False

Expected:
The Service should not be serving this revision. It should still be serving the previous one.

[mattmoor]

What version of Knative are you using?

[steren]

My knative version is 0.3

[markusthoemmes]

Can't reproduce right now but this might've been fixed via #2735

[mattmoor]

Pretty sure this was fixed in 0.4 which added tcp probes by default. Feel free to reopen if it occurs on 0.4 or HEAD. The workaround prior is to add a readiness probe.