Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Set Load Balancer internal/external per process instead of per application #1082

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions procfile/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -85,5 +85,6 @@ inside the `Procfile` for a specific process.
Name | Default value | Available options | Description
-----|---------------|-------------------|------------
`EMPIRE_X_LOAD_BALANCER_TYPE` | `elb` | `alb`, `elb`| Determines whether you will use an ALB or ELB
`EMPIRE_X_EXPOSURE` | `private` | `private`, `public` | Sets whether your ALB or ELB will be public (internet-facing) or private (internal), the default is private, however if you have used the deprecated `domain-add` command then the load balancer will become public. **If you change this setting, the load balancer will be recreated as soon as you deploy**
`EMPIRE_X_TASK_DEFINITION_TYPE` | not set | `custom` | Determines whether we use the Custom::ECSTaskDefinition (better explanation needed)
`EMPIRE_X_TASK_ROLE_ARN` | not set | any IAM role ARN | Sets the IAM role for that app/process. **Your ECS cluster MUST have Task Role support enabled before this can work!**
40 changes: 26 additions & 14 deletions releases.go
Original file line number Diff line number Diff line change
Expand Up @@ -306,6 +306,11 @@ func releasesCreate(db *gorm.DB, release *Release) (*Release, error) {
func newSchedulerApp(release *Release) (*twelvefactor.Manifest, error) {
var processes []*twelvefactor.Process

env := environment(release.Config.Vars)
env["EMPIRE_APPID"] = release.App.ID
env["EMPIRE_APPNAME"] = release.App.Name
env["EMPIRE_RELEASE"] = fmt.Sprintf("v%d", release.Version)

for name, p := range release.Formation {
if p.NoService {
// If the entry is marked as "NoService", don't send it
Expand All @@ -319,18 +324,13 @@ func newSchedulerApp(release *Release) (*twelvefactor.Manifest, error) {
continue
}

process, err := newSchedulerProcess(release, name, p)
process, err := newSchedulerProcess(release, name, p, env)
if err != nil {
return nil, err
}
processes = append(processes, process)
}

env := environment(release.Config.Vars)
env["EMPIRE_APPID"] = release.App.ID
env["EMPIRE_APPNAME"] = release.App.Name
env["EMPIRE_RELEASE"] = fmt.Sprintf("v%d", release.Version)

labels := map[string]string{
"empire.app.id": release.App.ID,
"empire.app.name": release.App.Name,
Expand All @@ -347,7 +347,7 @@ func newSchedulerApp(release *Release) (*twelvefactor.Manifest, error) {
}, nil
}

func newSchedulerProcess(release *Release, name string, p Process) (*twelvefactor.Process, error) {
func newSchedulerProcess(release *Release, name string, p Process, appEnv map[string]string) (*twelvefactor.Process, error) {
env := make(map[string]string)
for k, v := range p.Environment {
env[k] = v
Expand All @@ -365,14 +365,25 @@ func newSchedulerProcess(release *Release, name string, p Process) (*twelvefacto
exposure *twelvefactor.Exposure
err error
)
mergedEnv := twelvefactor.Merge(appEnv, env)

isExternal := release.App.Exposure == exposePublic
if v, ok := mergedEnv["EMPIRE_X_EXPOSURE"]; ok {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we should add some tests similar to https://github.com/remind101/empire/blob/master/tests/empire/empire_test.go#L77-L180 that exercises this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep definitely makes sense. Let me have a stab at it

if v == exposePublic {
isExternal = true
} else if v == exposePrivate {
isExternal = false
}
}

// For `web` processes defined in the standard procfile, we'll
// generate a default exposure setting and also set the PORT
// environment variable for backwards compatability.
// environment variable for backwards compatibility.
if name == webProcessType && len(p.Ports) == 0 {
exposure = standardWebExposure(release.App)
exposure = standardWebExposure(release.App, isExternal)
env["PORT"] = "8080"
} else {
exposure, err = processExposure(release.App, name, p)
exposure, err = processExposure(release.App, name, p, isExternal)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -411,7 +422,7 @@ func environment(vars Vars) map[string]string {

// standardWebExposure generates a scheduler.Exposure for a web process in the
// standard Procfile format.
func standardWebExposure(app *App) *twelvefactor.Exposure {
func standardWebExposure(app *App, isExternal bool) *twelvefactor.Exposure {
ports := []twelvefactor.Port{
{
Container: 8080,
Expand All @@ -432,12 +443,12 @@ func standardWebExposure(app *App) *twelvefactor.Exposure {
}

return &twelvefactor.Exposure{
External: app.Exposure == exposePublic,
External: isExternal,
Ports: ports,
}
}

func processExposure(app *App, name string, process Process) (*twelvefactor.Exposure, error) {
func processExposure(app *App, name string, process Process, isExternal bool) (*twelvefactor.Exposure, error) {
// No ports == not exposed
if len(process.Ports) == 0 {
return nil, nil
Expand Down Expand Up @@ -474,8 +485,9 @@ func processExposure(app *App, name string, process Process) (*twelvefactor.Expo
Protocol: protocol,
})
}

return &twelvefactor.Exposure{
External: app.Exposure == exposePublic,
External: isExternal,
Ports: ports,
}, nil
}
Expand Down
2 changes: 1 addition & 1 deletion runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ func (r *runnerService) Run(ctx context.Context, opts RunOpts) error {
if err != nil {
return err
}
p, err := newSchedulerProcess(release, procName, proc)
p, err := newSchedulerProcess(release, procName, proc, a.Env)
if err != nil {
return err
}
Expand Down
6 changes: 5 additions & 1 deletion scheduler/cloudformation/template.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"errors"
"fmt"
"io"
"log"
"reflect"
"regexp"
"sort"
Expand Down Expand Up @@ -403,11 +404,13 @@ func (t *EmpireTemplate) addService(tmpl *troposphere.Template, app *twelvefacto
scheme := schemeInternal
sg := t.InternalSecurityGroupID
subnets := t.InternalSubnetIDs
targetGroupPrefix := "Internal"

if p.Exposure.External {
scheme = schemeExternal
sg = t.ExternalSecurityGroupID
subnets = t.ExternalSubnetIDs
targetGroupPrefix = "External"
}

loadBalancerType := loadBalancerType(app, p)
Expand Down Expand Up @@ -435,7 +438,8 @@ func (t *EmpireTemplate) addService(tmpl *troposphere.Template, app *twelvefacto

tmpl.AddResource(loadBalancer)

targetGroup := fmt.Sprintf("%sTargetGroup", key)
targetGroup := fmt.Sprintf("%s%sTargetGroup", key, targetGroupPrefix)
log.Printf("using TargetGroup name %s\n", targetGroup)
tmpl.Resources[targetGroup] = troposphere.Resource{
Type: "AWS::ElasticLoadBalancingV2::TargetGroup",
Properties: map[string]interface{}{
Expand Down
6 changes: 3 additions & 3 deletions twelvefactor/twelvefactor.go
Original file line number Diff line number Diff line change
Expand Up @@ -187,16 +187,16 @@ type Scheduler interface {
// Env merges the App environment with any environment variables provided
// in the process.
func Env(app *Manifest, process *Process) map[string]string {
return merge(app.Env, process.Env)
return Merge(app.Env, process.Env)
}

// Labels merges the App labels with any labels provided in the process.
func Labels(app *Manifest, process *Process) map[string]string {
return merge(app.Labels, process.Labels)
return Merge(app.Labels, process.Labels)
}

// merges the maps together, favoring keys from the right to the left.
func merge(envs ...map[string]string) map[string]string {
func Merge(envs ...map[string]string) map[string]string {
merged := make(map[string]string)
for _, env := range envs {
for k, v := range env {
Expand Down