search

deis / router

Edge router for Deis Workflow
https://deis.com
MIT License
80 stars 57 forks source link

fix(charts): enable hostports by default #298

Closed bacongobbler closed 7 years ago

bacongobbler commented 7 years ago

This was the old default behaviour and is expected. Changing it to disabled by default is a regression. change was introduced in #296

cc @lachie83 for visibility, does this impact you in any way?

deis-bot commented 7 years ago

@lachie83 and @kmala are potential reviewers of this pull request based on my analysis of git blame information. Thanks @bacongobbler!

bacongobbler commented 7 years ago

this actually doesn't fix the problem, one sec

bacongobbler commented 7 years ago

there, that should resolve it and charts that flag it with enabled: false to work as well.

codecov-io commented 7 years ago

Current coverage is 55.23% (diff: 100%)

Merging #298 into master will not change coverage

@@             master       #298   diff @@
==========================================
  Files             6          6          
  Lines           382        382          
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
  Hits            211        211          
  Misses          151        151          
  Partials         20         20

Powered by Codecov. Last update 919d078...a851f66

vdice commented 7 years ago

ci is hitting a template render error:

16:38:28 Installing workflow chart workflow-v2.9.1-20170104233738-sha.cc7dcfa 16:38:29 Error: render error in "workflow/charts/router/templates/router-deployment.yaml": template: workflow/charts/router/templates/router-deployment.yaml:52:7: executing "workflow/charts/router/templates/router-deployment.yaml" at <eq .Values.host_port...>: error calling eq: incompatible types for comparison

lachie83 commented 7 years ago

Confirming that this doesn't impact me. I'm fine with the default being enabled.

lachie83 commented 7 years ago

@bacongobbler appreciate you getting a fix in.

bacongobbler commented 7 years ago

Ah so the template originally worked, I'm just being dumb because I thought it was a string rather than a boolean value. Things should be working again. :)

@lachie83 not a problem, thanks for the PR! I know lots of people have asked for something like this.