search

docker / compose-on-kubernetes

Deploy applications described in Compose onto Kubernetes clusters
Apache License 2.0
1.42k stars 159 forks source link

Add option for healthz check port. Use it to disable server with installer option skip liveness probe. #72

Closed guillaumerose closed 5 years ago

guillaumerose commented 5 years ago

Fix #70

guillaumerose commented 5 years ago

I updated the PR according to comments. I tried to beat gocyclo by extracting the checkAPIPresent() method.

silvin-lubecki commented 5 years ago

@guillaumerose the validate step is complaining:

cmd/compose-controller/main.go:1::warning: file is not goimported (goimports)
silvin-lubecki commented 5 years ago

e2e tests are failing too:

Compose fry with permission 
  Should allow user to create stack
  /go/src/github.com/docker/compose-on-kubernetes/e2e/rbac_test.go:32

STEP: Creating namespace "e2e-tests-compose-1261616337108845555" for this suite.
error occurred, waiting: stacks.compose.docker.com "app" is forbidden: User "employee" cannot create resource "stacks/composefile" in API group "compose.docker.com" in the namespace "e2e-tests-compose-1261616337108845555"
STEP: Destroying namespace "e2e-tests-compose-1261616337108845555" for this suite.
simonferquel commented 5 years ago

Hmm, this changes the API server/controller behavior, but if I am not wrong, I think the deployment specs still reference port 8080 explicitly

guillaumerose commented 5 years ago

Oh yes, thanks

simonferquel commented 5 years ago

See https://github.com/docker/compose-on-kubernetes/blob/3b5db76eaff9253e1727ded79896c7ef1b2081ae/install/install.go#L849 and https://github.com/docker/compose-on-kubernetes/blob/3b5db76eaff9253e1727ded79896c7ef1b2081ae/install/install.go#L682

guillaumerose commented 5 years ago

Yes I just changed it.

simonferquel commented 5 years ago

https://github.com/docker/compose-on-kubernetes/pull/72/files#diff-cdae8f79a5c22908bba7c1ba91c8a357R697 -> test based on skipLivenessProbe option should now check for LivenessProbe port instead

guillaumerose commented 5 years ago

I removed SkipLivenessProbes and just kept HealthzCheckPort. The flag is still here and set the port to 0 if used.

codecov-io commented 5 years ago

Codecov Report

Merging #72 into master will decrease coverage by 0.37%. The diff coverage is 35.71%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #72      +/-   ##
==========================================
- Coverage   65.81%   65.43%   -0.38%     
==========================================
  Files          88       88              
  Lines       10936    10941       +5     
==========================================
- Hits         7197     7159      -38     
- Misses       3454     3481      +27     
- Partials      285      301      +16
Impacted Files Coverage Δ
install/types.go 0% <ø> (ø) :arrow_up:
install/install.go 0.35% <0%> (ø) :arrow_up:
cmd/installer/main.go 6.52% <0%> (-0.22%) :arrow_down:
cmd/compose-controller/main.go 57.29% <41.66%> (-3.58%) :arrow_down:
cmd/api-server/cli/root.go 64.32% <78.94%> (-1.43%) :arrow_down:
internal/controller/stackreconciler.go 53.19% <0%> (-13.83%) :arrow_down:
internal/controller/controller.go 87.5% <0%> (-12.5%) :arrow_down:
internal/controller/resourceupdater.go 31.48% <0%> (-11.12%) :arrow_down:
internal/convert/stack.go 78.5% <0%> (-2.81%) :arrow_down:
internal/convert/service.go 97.24% <0%> (-1.84%) :arrow_down:
... and 1 more

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 be2e1da...f1216d7. Read the comment docs.

simonferquel commented 5 years ago

This is merged, thanks!