deis / workflow-cli

The CLI for Deis Workflow
http://deis.com
MIT License
31 stars 43 forks source link

fix(cmd): check for invalid probe types #297

Closed bacongobbler closed 7 years ago

bacongobbler commented 7 years ago

closes deis/workflow#704

deis-bot commented 7 years ago

@ultimateboy and @Joshua-Anderson are potential reviewers of this pull request based on my analysis of git blame information. Thanks @bacongobbler!

codecov-io commented 7 years ago

Codecov Report

Merging #297 into master will increase coverage by 0.13%. The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #297      +/-   ##
==========================================
+ Coverage   72.58%   72.72%   +0.13%     
==========================================
  Files          59       59              
  Lines        4082     4095      +13     
==========================================
+ Hits         2963     2978      +15     
+ Misses        792      791       -1     
+ Partials      327      326       -1
Impacted Files Coverage Δ
parser/healthchecks.go 88.77% <100%> (+1.89%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 1ffbad5...dfdf7af. Read the comment docs.

vdice commented 7 years ago

Code change LGTM; however, a corresponding update to e2e expectations will be needed (see linked jenkins ci run).

For example, we still expect Applying livenessProbe healthcheck but we now get Applying liveness healthcheck:

16:25:00 • Failure [60.125 seconds]
16:25:00 deis healthchecks
16:25:00 /go/src/github.com/deis/workflow-e2e/tests/healthcheck_test.go:204
16:25:00   with an existing user
16:25:00   /go/src/github.com/deis/workflow-e2e/tests/healthcheck_test.go:170
16:25:00     who owns an existing app that has already been deployed
16:25:00     /go/src/github.com/deis/workflow-e2e/tests/healthcheck_test.go:169
16:25:00       that user can set a tcpSocket liveness healthcheck [It]
16:25:00       /go/src/github.com/deis/workflow-e2e/vendor/github.com/onsi/ginkgo/ginkgo_dsl.go:365
16:25:00 
16:25:00       No future change is possible.  Bailing out early after 32.316s.
16:25:00       Got stuck at:
16:25:00           Applying liveness healthcheck... ...o...o...o...o...o...o...o...o...o...o...o...o...o...o...o...o...o...o...o...o...o...o...o...o...o...o...o...o...o...o...o...o...o...o...o...o...o...o...o...o...o...o...o...o...o...o...o...o...o...o...o...o...o...o...o...o...o...o...o...o...done
16:25:00           
16:25:00           === test-91480123 Healthchecks
16:25:00           
16:25:00           web/cmd:
16:25:00           --- Liveness
16:25:00           Initial Delay (seconds): 50
16:25:00           Timeout (seconds): 50
16:25:00           Period (seconds): 10
16:25:00           Success Threshold: 1
16:25:00           Failure Threshold: 3
16:25:00           Exec Probe: N/A
16:25:00           HTTP GET Probe: N/A
16:25:00           TCP Socket Probe: Port=1500
16:25:00           
16:25:00           --- Readiness
16:25:00           No readiness probe configured.
16:25:00           
16:25:00       Waiting for:
16:25:00           Applying livenessProbe healthcheck...
16:25:00 
16:25:00       /go/src/github.com/deis/workflow-e2e/tests/healthcheck_test.go:86
bacongobbler commented 7 years ago

Okay if that's the expected output I'll keep that change in there. Lemme do some code switcheroo and revert some of the deletions I made.

bacongobbler commented 7 years ago

done, changes made and now it's passing e2e normally.