Add container image validation

[f-higashi]

Added container image validation. #422

Please review it.

[codecov-io]

Current coverage is 83.18%

Merging #485 into master will increase coverage by +0.46% as of 0f1e8b3

@@            master    #485   diff @@
======================================
  Files           83      84     +1
  Stmts          660     678    +18
  Branches         0       0       
  Methods          0       0       
======================================
+ Hit            546     564    +18
  Partial          0       0       
  Missed         114     114       

Review entire Coverage Diff as of 0f1e8b3

Powered by Codecov. Updated on successful CI builds.

[bryk]

Reviewed 24 of 26 files at r1.
Review status: 24 of 26 files reviewed at latest revision, 3 unresolved discussions.

---

src/app/frontend/deploy/deployfromsettings.html, line 60 [r1] (raw file):
Can you maybe include the reason here?

---

src/app/frontend/deploy/validimagereference_directive.js, line 26 [r1] (raw file):
We need tests for this file. See uniquename_directive_test how to do this. Aim for 100% incremental coverage.

---

src/app/frontend/deploy/validimagereference_directive.js, line 64 [r1] (raw file):
Should we assume it is valid? This is what we do with name validation. Check out how it is done and documented there.

---

Comments from the review on Reviewable.io

[bryk]

PTAL. All we need is more tests and one style comment.

---

Review status: 24 of 26 files reviewed at latest revision, 3 unresolved discussions.

---

Comments from the review on Reviewable.io

[f-higashi]

Review status: 24 of 28 files reviewed at latest revision, 3 unresolved discussions.

---

src/app/frontend/deploy/deployfromsettings.html, line 60 [r1] (raw file):
I added server response message.

---

src/app/frontend/deploy/validimagereference_directive.js, line 26 [r1] (raw file):
I added test for validimagereference.

---

src/app/frontend/deploy/validimagereference_directive.js, line 64 [r1] (raw file):
It has no problem as a result of my check.

Resource action methods can be invoked with the following paramters:

  non-GET "class" actions: Resource.action([parameters], postData, [success], [error])

https://code.angularjs.org/1.5.0/docs/api/ngResource/service/$resource

In this case, spec, '(validty)=>', and '()=>' are coressponding to postData, success callback, and error callback respectively.
And name validation code does not use err response. so err arugment is omitted.

But in this modifiction, I added err arugment to show server error message.

Does that answer your comment?

---

Comments from the review on Reviewable.io

[f-higashi]

PTAL

[bryk]

Last comments and PTAL. I'm very happy about the tests.

---

Reviewed 1 of 4 files at r2.
Review status: 25 of 28 files reviewed at latest revision, 4 unresolved discussions.

---

a discussion (no related file):
When I test this on my machine, there is huge jitter when entering, e.g, 'asdsadlas;'dl'sa;ld;;;;;;;;;' and then deleting it char by char. Can you add ng-model-options with debounce, the same as in name validation?

---

src/app/frontend/deploy/validimagereference_directive.js, line 26 [r1] (raw file):
Thanks!

---

src/app/frontend/deploy/validimagereference_directive.js, line 64 [r1] (raw file):
Yes.

---

src/app/frontend/deploy/validimagereference_directive.js, line 18 [r2] (raw file):
Hmm... This seems very generic. How about invalidImageErrorMessage?

---

Comments from the review on Reviewable.io

[f-higashi]

Review status: 24 of 28 files reviewed at latest revision, 3 unresolved discussions.

---

a discussion (no related file):
Thank you for your comment.
I set ng-model-options.

---

src/app/frontend/deploy/validimagereference_directive.js, line 18 [r2] (raw file):
It's good.
I modified it.

---

Comments from the review on Reviewable.io

[bryk]

Thank you for this. Merging :)

---

Reviewed 1 of 4 files at r2, 3 of 3 files at r3.
Review status: all files reviewed at latest revision, 1 unresolved discussion.

---

Comments from the review on Reviewable.io

[floreks]

@bryk @f-higashi this PR causes integration test deploy not existing image to fail.

[bryk]

Oh, is this really this one? Do you know the reason why is that?

[floreks]

Async validation of container image blocks deploy of not existing image test. It's trying to deploy app test with container image set to test and there is error Container image is invalid: 404: Page Not Found.

[f-higashi]

Async validation does not check whether image is existing or not in repository.

Which environment did tests fail?
If it is travis environment, can you tell me build id?

[floreks]

I'll once it's finished on my local repo. It wasn't clear before because deploy from file test was failing and all other test also because of that. Now it is disabled and it will fail on master.

[f-higashi]

Hmm..
I cannot reproduce it in my local machine.

This PR updated backend api.
So I needed 'gulp clean' after that under development.
Please execute 'gulp clean' and try it once.

[floreks]

I'm always executing gulp clean && gulp serve on restart. Problem was elsewhere. Stopping serve does not kill backend and I've had multiple stale dashboard processes running in background. After killing them with killall dashboard local tests have passed. Still there are some random fails of tests but probably not related to this.

[f-higashi]

Okay
Thanks for your investigation.