Allow any process to expose an HTTP/TCP service #800

ejholmes · 2016-04-26T05:55:48Z

This adds a new ports setting to the extended Procfile format, which allows you to attach a load balancer to any process in the app, not just the "web" process.

This is definitely a WIP and looking for feedback. There's some open questions, and a lot of polish needed.

If you want to play around with this locally, you can emp deploy remind101/acme-inc:extended

TODO

Moar tests.
~~Allow tcp/ssl load balancers to actually get created.~~ First version of this will be http/https only
If the process originally had no load balancer, you can't add a load balancer to it (ECS limitation). The workaround is to remove the process definition, then add it again with the expose setting. This is awkward though, because the existing ECS service kinda sticks around in a draining state for a while. Not this PR.
Currently, I'm thinking that the extended Procfile format shouldn't make any assumptions about the web process. But, when people switch to the extended Procfile format, they'll almost certainly forget to add the expose setting. Need to think about a good way to prevent this. I think this is fine once Only destroy load balancers when the app is destroyed. #801 is merged.
~~Right now, I'm not creating CNAME's for anything other than the web process. That could probably be better.~~ Probably not in this PR.
Don't remove ELB's when deleting a process? Only destroy load balancers when the app is destroyed. #801
~~For https/ssl exposures, it will use the single attached certificate on the app. People might want to use different certs for different processes.~~ Not this PR.
Migration

ejholmes · 2016-04-26T06:09:08Z

If the process originally had no load balancer, you can't add a load balancer to it (ECS limitation). The workaround is to remove the process definition, then add it again with the expose setting. This is awkward though, because the existing ECS service kinda sticks around in a draining state for a while.

To account for this, I've considered changing how we name ECS services to include a hash of the exposure settings. So if you change a processes exposure, it'll be a new ECS service and new ELB (like <app-id>--web--<sha1 of exposure>).

Probably wouldn't do that in this PR though.

ejholmes · 2016-04-26T06:10:52Z

scheduler/ecs/ecs.go

 			opts.SSLCert = e.Cert
+		case *scheduler.HTTPExposure:
+		default:
+			return nil, fmt.Errorf("can't create %s load balancer", e.Protocol())


ejholmes · 2016-04-27T02:50:23Z

Ok, I think this is probably mergable once #801 is merged. I've addressed all of the todo's, added some tests and added a migration.

ejholmes · 2016-04-27T03:57:21Z

procfile/README.md

+
+```yaml
+expose:
+  type: tcp // Possible options are tcp/http/https/ssl


One scenario that I haven't really thought through with this yet is when you want to have multiple listeners. For example, our existing behavior when creating https load balancers is to create both an https and an http listener. Maybe we should do something like this:

web: visibility: public ports: - 80:80 # http listener - 443:80 # https listener - "1234:1234" # tcp listener - # ??? not really a good way to infer an ssl listener.

This might be nice because it offers pretty close parity with ports in docker-compose.yml.

The awkward bit is how we communicate the protocol when it can't be inferred. We could extend the port to be a map, like so:

web: visibility: public ports: - "80:8080": protocol: http - "443:8080": protocol: https - "1234:8081": protocol: tcp - "4321:8081": protocol: ssl

This would also mean we could avoid setting the PORT environment variable and just let containers bind to the port they specify.

Actually, I need to check if this would even be possible with ECS. To do this, I think we'd need to allocate an instance port per container port (e.g. port 8080 and port 8081 get different instance ports). Last time I checked, you could only add 1 LoadBalancer object when creating an ECS service, and the load balancers can't be updated once the service is created.

Maybe the format above is good, but we just need some validations for ECS/ELB limitations.

Just tested this with ECS. It looks like it's possible, but their docs don't make it very intuitive. When we create an ECS, we provide an array of ecs.LoadBalancer. That type makes you provide a ContainerPort and ContainerName, but those don't seem to serve any purpose other than validation (ensure that the task definition has a matching port mapping). So, if we create a load balancer with multiple listeners, and a task definition with multiple port mappings, that should be fine.

i like having the ability for each port mapping to define its protocol.

you could also support inference for the http/https use case just because it is pretty common.

ie:

web: visibility: public ports: - 80:8080 - 443:8080 - 4321:8081 protocol: tcp

I'm sooo looking forward to this

mwildehahn · 2016-04-27T20:16:12Z

looks awesome 👍

ejholmes · 2016-05-04T00:26:45Z

For the record. Putting this on hold until CloudFormation is the default scheduling backend, which will make everything we want to do with extended Procfile much simpler.

ejholmes · 2016-07-13T03:51:39Z

I've updated this with complete support in the CloudFormation backend. You can now define a Procfile like this:

web: 
  command: acme-inc server
  ports:
    - "80:80"
    - "8080:8080":
        protocol: "tcp"
api:
  command: acme-inc api
  ports:
    - "80:80"

Allowing you to define pretty sophisticated scenarios. HTTP, HTTPS, TCP and SSL protocols are all supported.

The web process will still get some default exposure settings if a ports: key is not provided. It's essentially equivalent to:

web:
  ports:
    - "80:8080":
        protocol: "http"

And web is still special, in that it gets a CNAME at <app>.empire.

Also, the visibility (external/internal) is still controlled via emp domains, but I'd plan on allowing that to be overridden with a visibility: key, like this:

web:
  visibility: public
  ports:
    - "80:8080"

If you wanna try it out, you can run this and deploy the remind101/acme-inc:extended image.

This requires some changes in scheduler.Exposure that would be really hard to support in the legacy ECS backend, so you'll notice that this PR currently removes the legacy ECS backend. I think, we should hold off on merging this until we've released 0.11.0, which is the release where we'll make the CloudFormation backend the default, at which point master can become the tip for 1.0.0.

phobologic · 2016-07-15T00:18:32Z

👍 awesome

ejholmes · 2016-10-06T06:30:54Z

Ok, rebased this. This can be merged once #1001 has been merged.

ejholmes · 2016-10-06T08:55:15Z

ALB wasn't released when I originally opened this, and there are some limitations when using ALB (see the last commit). Basically, when using ALB + dynamic port mapping, you cannot have multiple listeners going to multiple container ports. It makes sense when you think about it; ECS is registering the container in a target group with a single dynamically allocated port, so alb has no way to know how to route to multiple ports.

The below will work fine when using ELB + instance ports, however, if ALB is enabled, it will yield an error now:

web:
  ports:
    - "80:80"
    - "8080:8080"

This is totally fine with ALB + dynamic ports (all host ports map to a single container port):

web:
  ports:
    - "80:8080"
    - "443:8080":
         protocol: https
    - "8080:8080"

I think this is only a limitation with dynamic port mapping, so if we allocated an instance port for ALB, instead of passing HostPort: 0, then it would work the same as ELB. Maybe we could make that configurable? This just isn't possible with ALB, since ECS only supports a single target group per service, and the target group can only have 1 port. Se la vie.

phobologic

👍 small comment, LGTM though

phobologic · 2016-10-07T00:41:53Z

releases.go

+		exposure = standardWebExposure(release.App)
+		env["PORT"] = "8080"
+	} else {
+		exposure = processExposure(release.App, p)


Is there no way to share the port info? SEems like we should keep the env var around, no matter what, if at all possible. Users may be relying on it.

We could do something like what docker-compose does, where you would have environment vars like:

PORT_80_PORT PORT_443_PORT PORT_8080_PORT

(PORT doesn't really make sense when you're specifying ports in the Procfile)

But, I kinda feel like it doesn't add much value, since you could just do that in the Procfile and it'd be more explicit:

web: environment: PORT: "80" ADMIN_PORT: "8080" ports: - "80:80" - "80:8080"

Ahh, good point - so this only happens when there are multiple ports defined? That seems fine.

Right. For backwards compatibility, if you defined a web proc, but no ports: then it's equivalent to:

web: environment: PORT: "8080" ports: - "80:8080": protocol: "http" - "443:8080": # If a cert is provided protocol: "https"

Sounds good! Ignore me!

Unlike ELB, ALB will not transparently map listener ports to container ports. When using dynamically configured host ports, ALB can only route multiple listeners to a single container port.

This will allow different processes to define what load balancer they want to use (e.g. a `web` proc can use ELB, while an `grpc` proc uses ALB)

Ref, not GetAtt

ejholmes · 2016-10-07T02:23:39Z

I'll go ahead and merge this for now. There will probably be some follow up polish, and additional docs, but overall this seems to be working great.

ejholmes added the in progress label Apr 26, 2016

ejholmes reviewed Apr 26, 2016
View reviewed changes

ejholmes mentioned this pull request Apr 27, 2016

Add support for creating TCP load balancers in ECS scheduler. #802

Closed

ejholmes reviewed Apr 27, 2016
View reviewed changes

ejholmes modified the milestone: 1.0.0 May 6, 2016

ejholmes removed the in progress label Jun 24, 2016

ejholmes force-pushed the extended-expose branch from 06bdabe to 15f811e Compare July 13, 2016 03:39

ejholmes added the needs review label Jul 13, 2016

ejholmes force-pushed the extended-expose branch from 15f811e to 90e2f38 Compare July 15, 2016 00:54

ejholmes added reviewed and removed needs review labels Jul 18, 2016

ejholmes removed the reviewed label Sep 14, 2016

ejholmes mentioned this pull request Oct 6, 2016

Remove legacy ECS scheduler #1001

Merged

ejholmes force-pushed the extended-expose branch from 90e2f38 to d57e899 Compare October 6, 2016 06:24

ejholmes changed the base branch from master to remove-ecs October 6, 2016 06:30

ejholmes force-pushed the extended-expose branch from 6d89999 to 68e2e60 Compare October 6, 2016 08:42

ejholmes changed the base branch from remove-ecs to master October 6, 2016 22:23

ejholmes force-pushed the extended-expose branch from 68e2e60 to 2ace70f Compare October 6, 2016 22:24

phobologic approved these changes Oct 7, 2016

View reviewed changes

ejholmes added 4 commits October 6, 2016 18:14

Support a ports attribute in the extended Procfile.

d8ada80

Better AWS Application Load Balancing support.

6433653

Unlike ELB, ALB will not transparently map listener ports to container ports. When using dynamically configured host ports, ALB can only route multiple listeners to a single container port.

Move automatic protocol determination to named function.

1322f80

Add support for setting environment variables in extended procfile.

6a36e75

ejholmes force-pushed the extended-expose branch from 13602e6 to 6a36e75 Compare October 7, 2016 01:15

ejholmes modified the milestones: 0.12.0, 1.0.0 Oct 7, 2016

ejholmes added 2 commits October 6, 2016 18:52

Fully expand process environment, before checking load balancer type.

01ce4f6

This will allow different processes to define what load balancer they want to use (e.g. a `web` proc can use ELB, while an `grpc` proc uses ALB)

Fix HTTPS listener when using ALB.

166c55b

Ref, not GetAtt

ejholmes merged commit b2f4ad3 into master Oct 7, 2016

ejholmes deleted the extended-expose branch October 7, 2016 02:23

ejholmes mentioned this pull request Oct 8, 2016

Dual ELB/ALB #991

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow any process to expose an HTTP/TCP service #800

Allow any process to expose an HTTP/TCP service #800

ejholmes commented Apr 26, 2016 •

edited

Loading

ejholmes commented Apr 26, 2016

ejholmes Apr 26, 2016

ejholmes commented Apr 27, 2016

ejholmes Apr 27, 2016 •

edited

Loading

ejholmes Apr 27, 2016 •

edited

Loading

ejholmes Apr 27, 2016 •

edited

Loading

mwildehahn May 2, 2016

v-yarotsky Jun 6, 2016

mwildehahn commented Apr 27, 2016

ejholmes commented May 4, 2016

ejholmes commented Jul 13, 2016

phobologic commented Jul 15, 2016

ejholmes commented Oct 6, 2016 •

edited

Loading

ejholmes commented Oct 6, 2016 •

edited

Loading

phobologic left a comment

phobologic Oct 7, 2016

ejholmes Oct 7, 2016

phobologic Oct 7, 2016

ejholmes Oct 7, 2016

phobologic Oct 7, 2016

ejholmes commented Oct 7, 2016

Allow any process to expose an HTTP/TCP service #800

Allow any process to expose an HTTP/TCP service #800

Conversation

ejholmes commented Apr 26, 2016 • edited Loading

ejholmes commented Apr 26, 2016

Choose a reason for hiding this comment

ejholmes commented Apr 27, 2016

ejholmes Apr 27, 2016 • edited Loading

Choose a reason for hiding this comment

ejholmes Apr 27, 2016 • edited Loading

Choose a reason for hiding this comment

ejholmes Apr 27, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mwildehahn commented Apr 27, 2016

ejholmes commented May 4, 2016

ejholmes commented Jul 13, 2016

phobologic commented Jul 15, 2016

ejholmes commented Oct 6, 2016 • edited Loading

ejholmes commented Oct 6, 2016 • edited Loading

phobologic left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ejholmes commented Oct 7, 2016

ejholmes commented Apr 26, 2016 •

edited

Loading

ejholmes Apr 27, 2016 •

edited

Loading

ejholmes Apr 27, 2016 •

edited

Loading

ejholmes Apr 27, 2016 •

edited

Loading

ejholmes commented Oct 6, 2016 •

edited

Loading

ejholmes commented Oct 6, 2016 •

edited

Loading