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

Added the container index to the network-compute-ports trigger #4128

Merged
merged 1 commit into from
Nov 22, 2020

Conversation

fomojola
Copy link
Contributor

@fomojola fomojola commented Sep 8, 2020

Useful for alternative proxy implementations. Updated the documentation for the network-compute-ports trigger to clarify the arguments

For alternate proxy implementations, the network-compute-ports trigger in the local docker scheduler allows for individual port mappings for each container instance. This PR adds the container index to the trigger (so an independent index of port mappings to container index can easily be maintained) and updates the documentation of the trigger to match (also, the documentation was missing a couple of arguments).

…l for alternative proxy implementations. Updated the documentation for the network-compute-ports trigger to clarify the arguments
@josegonzalez
Copy link
Member

Why would the container index be useful? Can you give me an example of an alternative proxy implementation that would use this?

@fomojola
Copy link
Contributor Author

@josegonzalez I'm working on an alternate Haproxy proxy implementation: instead of regenerating the config file I'm using the Haproxy Runtime API (https://cbonte.github.io/haproxy-dconv/2.2/management.html) to link one of a number of backend servers to each of the Docker instances launched by the scheduler on each re-deploy. For each dokku app, I create a backend that looks like this:

backend ${APP}-backend
    mode http

    http-request track-sc0 src table st_src_global

    http-request set-header Host ${HOSTNAME}
    http-request replace-path /app/(.*) /\1
    option forwardfor
    option http-server-close
    server-template ${APP}-server- 1-1000 10.128.0.42:10000  maxconn 10 disabled

This starts up and instructs Haproxy to instantiate 100 servers inside that backend, all disabled. Whenever the scheduler is starting a new set of docker instances the proxy implementation does the following:

  1. In the network-compute-ports trigger I proxy the port for each of the web process containers (by adding a unique -p : port mapping) and I store that mapping (so I know that container X has container port Y mapped to host port Z).
  2. In the post-deploy trigger, I convert that mapping into instructions to feed to the Haproxy Runtime API that will enable/disable the appropriate set of Haproxy servers inside the Haproxy backend that maps to the APP (which I have as a parameter to the post-deploy trigger) as well as adjust the IP and port for each Haproxy server to link it to a specific container (so Haproxy server-X has its IP and port set to the appropriate host IP and port Z).

The container index makes it straightforward: ${APP}-server-${CONTAINER_INDEX} maps to the appropriate container, and I can disable all the other Haproxy backend servers that aren't in use (for example when a scale command reduces the number of containers of an app called v1 from 2 to 1, I can disable v1-server-2). I could introduce an alternate indexing scheme, but since one is already in use, I figured it would make the most sense to use it.

It works pretty well for the Docker Local Scheduler: Haproxy handles it seamlessly, and there's no rebuilding the config file. The only wrinkle is that I can't use zero-downtime deployment checks because the network-get-port trigger doesn't let me substitute the port properly, but I'm working on that.

@josegonzalez
Copy link
Member

Thats pretty smart, if different from what I'd imagined. What would be needed for this to handle zero downtime checks?

@fomojola
Copy link
Contributor Author

The only issue I'm having with zero downtime checks right now is that (at least for the docker local scheduler), this set of calls:

ipaddr=$(plugn trigger network-get-ipaddr "$APP" "$PROC_TYPE" "$cid") port=$(plugn trigger network-get-port "$APP" "$PROC_TYPE" "$cid" "$DOKKU_HEROKUISH")

at https://github.com/dokku/dokku/blob/master/plugins/scheduler-docker-local/scheduler-deploy#L117-L118 don't allow me to overwrite the port. So for example, if I implement that trigger in my proxy plugin and return the docker port mapping I get my port tacked on to the end of the internal port. So I see something like this for the host/port mapping (I decorated my local implementation with extra dokku_log_warn lines so I could get additional color on what was going on inside):

-----> Attempting pre-flight checks (web.1)
! [172.17.0.15][80
8211]

My port (8211) has been tacked on to the default port returned by the network-get-port trigger (80). Then the preflight checks fail because when it builds the curl request I see this:

CHECKS expected result:
CURL ARGS: -q --compressed --fail --location --noproxy 172.17.0.15 --max-time 10 -vv http://172.17.0.15:80
8211/ready.txt

My expectation is I'm doing something wrong here (just not sure what): this was my first plugin attempt and I was in a hurry as I'm vigorously defending against a much smarter person than me who wants to adopt Kubernetes, and that is a rabbit hole I'm desperate to avoid. Once this is cleaned up and documented would love to have the entire plugin released and available, along with the HAPROXY configuration parts.

Only took me a day from start to finish: I've been immensely pleased with Dokku. I am currently mapping out how to replace my home-grown deployment system across 20 servers with a single Dokku master. All I think I need is to figure out a plugin to publish the Docker images to a private registry, and then work through the Nomad scheduler config (it is missing a couple of the triggers that the local docker scheduler has, so might take a bit more of a slog) and I should be golden.

Thanks!

@josegonzalez
Copy link
Member

Checkout the dokku-registry plugin for pushing images.

Re: the port issue, is network-compute-ports returning port 80 for your app? What is returning 8211?

I'd be happy to help chat in slack on this to come up with a solution, I'm personally a huge fan of Nomad and would love to have good reasons to work on it :)

@fomojola
Copy link
Contributor Author

Would love to hop on slack: when/where should I go?

@josegonzalez
Copy link
Member

@josegonzalez josegonzalez merged commit 5b21cab into dokku:master Nov 22, 2020
josegonzalez pushed a commit that referenced this pull request Dec 1, 2020
# History

## 0.22.0

Install/update via the bootstrap script:

```shell
wget https://raw.githubusercontent.com/dokku/dokku/v0.22.0/bootstrap.sh
sudo DOKKU_TAG=v0.22.0 bash bootstrap.sh
```

See the [0.22.0 migration guide](/docs/appendices/0.22.0-migration-guide.md) for more information on migrating to 0.22.0.

### Bug Fixes

- #4204: @josegonzalez Ensure the image is not an empty string
- #4198: @josegonzalez Reference extracted Procfile
- #4194: @josegonzalez Drop appName check in apps:report
- #4183: @josegonzalez Embed pb.UnimplementedGreeterServer to avoid linting issues
- #4182: @josegonzalez Upgrade herokuish to fix nodejs tests
- #4173: @karimsan Add build-essential to Vagrant provision
- #4181: @josegonzalez Conditionally mount the dokku-arch folder if it exists
- #4130: @josegonzalez Clear proxy configs on boot
- #4131: @josegonzalez Bump minimum docker version
- #4123: @fomojola Support for expected CHECKS text with special characters
- #4115: @josegonzalez Add missing labels to built images
- #4116: @Schlepptop Ensure config:clear an be called

### New Features

- #4209: @josegonzalez Add experimental support for Cloud Native Buildpacks (CNB)
- #4210: @josegonzalez Add migration guide link to release notes
- #4203: @josegonzalez Cleanup log output for failure case
- #4208: @ankane Add ability to change the access log format
- #4202: @josegonzalez Schedule related images for cleanup when retiring containers
- #4197: @josegonzalez Retire intermediate containers after use
- #4128: @fomojola Added the container index to the network-compute-ports trigger
- #4191: @josegonzalez Create codeql-analysis.yml
- #4139: @Yihao-G Allow controlling Nginx proxy-buffer-size, proxy-buffering, proxy-buffers, proxy-busy-buffers-size
- #4156: @ltalirz When `cert:add` remove previous cert before copying the new cert
- #4129: @josegonzalez Prohibit non-dns names for apps and process types
- #4125: @josegonzalez Allow customizing the various nginx templates
- #4121: @josegonzalez Add ability to disable custom ninx.conf.sigil extraction

### Refactors

- #4160: @nerg4l Rewrite logs plugin in Go
- #4149: @josegonzalez Rewrite ps plugin in golang
- #4080: @hugopeixoto Stop using VHOST when listing app domains and urls
- #4117: @josegonzalez Rewrite app-json plugin in golang
- #4113: @josegonzalez Drop herokuish release code

### Documentation

- #4196: @josegonzalez Correctly doc apps:report output
- #4195: @luizpicolo Fix whitespace in process-management docs
- #4184: @badsyntax Add note regarding using plugin triggers instead of sourcing functions
- #4188: @fomojola Added the autosync community plugin
- #4187: @nahtnam Add `-p` flag to "Run on External Volume" tutorial
- #4186: @badsyntax Add dokku-discourse to community plugins list
- #4170: @chrisjsimpson Add note on how to enable buildkit usage
- #4168: @josegonzalez Revert "The default branch for ruby-getting-started is 'main', not 'master"
- #4167: @nateww The default branch for ruby-getting-started is 'main', not 'master
- #4135: @znz Fix broken table
- #4140: @swrobel Note Ubuntu 20.04 support in README
- #4136: @hugopeixoto Update push command for sample app in docs
- #4124: @josegonzalez Note that docker options require app rebuilds
- #4098: @carlosgeos Add a note on how nginx handles load balancing
- #4111: @josegonzalez Add large version of dokku image
- #4100: @turicas Fix markdown syntax in nginx docs

### Other

- #4201: @dependabot-preview[bot] chore(deps): bump jetty-servlet from 9.4.34.v20201102 to 9.4.35.v20201120 in /tests/apps/java
- #4200: @dependabot-preview[bot] chore(deps-dev): bump heroku/heroku-buildpack-php from 183 to 185 in /tests/apps/php
- #4189: @dependabot-preview[bot] chore(deps): bump thin from 1.7.2 to 1.8.0 in /tests/apps/ruby
- #4175: @dependabot-preview[bot] chore(deps): bump jetty-servlet from 9.4.33.v20201020 to 9.4.34.v20201102 in /tests/apps/java
- #4172: @dependabot-preview[bot] chore(deps): bump github.com/golang/protobuf from 1.4.2 to 1.4.3 in /tests/apps/gogrpc
- #4185: @dependabot-preview[bot] chore(deps): bump socket.io from 2.3.0 to 3.0.1 in /tests/apps/.websocket.disabled
- #4190: @dependabot-preview[bot] chore(deps-dev): bump heroku/heroku-buildpack-php from 182 to 183 in /tests/apps/php
- #4165: @dependabot-preview[bot] chore(deps): bump jetty-servlet from 9.4.31.v20200723 to 9.4.33.v20201020 in /tests/apps/java
- #4153: @dependabot-preview[bot] chore(deps-dev): bump heroku/heroku-buildpack-php from 181 to 182 in /tests/apps/php
- #4144: @dependabot-preview[bot] chore(deps-dev): bump heroku/heroku-buildpack-php from 180 to 181 in /tests/apps/php
- #4137: @dependabot-preview[bot] chore(deps-dev): bump heroku/heroku-buildpack-php from 179 to 180 in /tests/apps/php
- #4118: @dependabot[bot] chore(deps): bump django from 3.0.2 to 3.0.7 in /tests/apps/dockerfile-release
- #4103: @dependabot-preview[bot] chore(deps-dev): bump heroku/heroku-buildpack-php from 178 to 179 in /tests/apps/php
- #4091: @dependabot-preview[bot] chore(deps): bump jetty-servlet from 9.4.30.v20200611 to 9.4.31.v20200723 in /tests/apps/java
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants