Marathon apps can have multiple service ports

[MiLk]

Fixes #54 Replaces #84

You can have apps which listen on different ports (nginx with 80 and 443), and each task will also have two ports.

84 doesn't map the app ports to the right task ports.

[j1n6]

Thanks for the PR, I have to review this together with the user interface & API changes.

[hyper0x]

I need it eagerly! Please speed up this process. Thanks.

[timoreimann]

@activars What's keeping us from merging this PR exactly? The feature looks pretty useful and is small enough so that it could be refactored fairly easily once the project aims for something bigger. Also, the changes are just implementation details, so there's no public API that could be broken. (Except for the template maybe, but I personally do not consider that to be part of any API.)

[j1n6]

Thank you for the suggestion, I think that's valid concern. I will test very soon this week. If there's no major backwards compatibility breakage, I will merge (expect a release soon)

[timoreimann]

Sounds good. Thanks!

[kikito]

:+1:

[j1n6]

I had a look and tested it locally, I see a couple of issues here.

• I would like to allow bamboo to overrides the random assigned hosted port by Marathon. If I'm correct, marathon would not allow 443 to be used multiple times.
• It's missing TCP use cases (this one should be easy to fix)

Looks like this is most wanted feature, I'm working on a branch that has:

• A REST endpoint so you can override the service ports
• Otherwise keep the default one that marathon offers (random or preconfigured)
• UI to allow configure this change
• Some backward compatibility (likely to break a little)

[MiLk]

To support TCP, what about:

{{ range $index, $app := .Apps }}
    {{ range $serviceIndex, $servicePort := $app.ServicePorts }}
        {{ if ne $app.Env.BAMBOO_IS_HTTP 1 }}
listen {{ $app.EscapedId }}_{{ $servicePort }}
        bind *:{{ $servicePort }}
        mode tcp
        option tcplog
        balance leastconn

            {{ range $page, $task := $app.Tasks }}
        server {{ $app.EscapedId }}-{{ $task.Host }}-{{ index $task.Ports $serviceIndex }} {{ $task.Host }}:{{ index $task.Ports $serviceIndex }}
            {{ end }}
        {{ end }}
    {{ end }}
{{ end }}

{{ range $index, $app := .Apps }}
  {{ range $serviceIndex, $servicePort := $app.ServicePorts }}
    {{ if eq $app.Env.BAMBOO_IS_HTTP 1 }}
 listen {{ $app.EscapedId }}_{{ $servicePort }}
   bind *:{{ $servicePort }}
   mode http
   {{ if $app.HealthCheckPath }}
     option httpchk GET {{ $app.HealthCheckPath }}
   {{ end }}
   balance leastconn
   option forwardfor
         {{ range $page, $task := $app.Tasks }}
         server {{ $app.EscapedId }}-{{ $task.Host }}-{{ index $task.Por    ts $serviceIndex }} {{ $task.Host }}:{{ index $task.Ports $serviceIndex }}     {{ if $app.HealthCheckPath }} check inter 30000 {{ end }} {{ end }}
     {{ end }}
   {{ end }}
 {{ end }}

About the fact we can't use 443 or 80 multiple times, it's something introduced with marathon 0.8.1, and I didn't have solved this problem on my infrastructure. We are still on 0.8.0 at the time being.

[timoreimann]

@activars, what's the state on this one? I'm fairly eager to use this feature, so I am wondering if there is a way to split the effort into smaller chunks. To me it seems worth considering to pass the PR without the additional features you proposed (port overriding, REST endpoint, UI) as long as configuration can be managed via the template, and follow up with another PR once things are done.

What do you think?

[MiLk]

My PR only adds two new variables, so it's totally safe to merge it and implement the other features at a later time.

[timoreimann]

@activars, what are your thoughts regarding @MiLk's latest comment?

[j1n6]

Hey, I'm looking into merging it today. I want to make bigger change to the project, but it might take longer so I'm considering merging this avoid blocking you guys.

On 8 May 2015, at 08:04, Timo Reimann notifications@github.com wrote:

@activars https://github.com/activars, what are your thoughts regarding @MiLk https://github.com/MiLk's latest comment?

— Reply to this email directly or view it on GitHub https://github.com/QubitProducts/bamboo/pull/96#issuecomment-100128475.

[j1n6]

apologise for the late reply, I think it's ok to merge. Thanks for the contribution.

[MiLk]

Thanks :dancers: