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

Allow docker-options to be scoped to specific Procfile processes #2441

Open
4 tasks
marqu3z opened this issue Sep 29, 2016 · 5 comments
Open
4 tasks

Allow docker-options to be scoped to specific Procfile processes #2441

marqu3z opened this issue Sep 29, 2016 · 5 comments

Comments

@marqu3z
Copy link

marqu3z commented Sep 29, 2016

High Level Plan

  • Rewrite docker-options to be in golang. We'll still need the same bash functions, and can model this interaction after how the config plugin works (it executes the scripts directly to get/set config values).
  • Add support for flags at the beginning of the command. Something like dokku --global -flags docker-options:subcommand --subcommand flag-values --go-here regular arguments seems like what we're standardizing on.
    • This should initially support --process PROC_TYPE and --global flags, with global being default.
  • Add support for lists to the properties functions in both golang and bash.
  • Move docker-options to use the properties functions for handling configuration. We'll need to figure out how to handle namespacing as - at the moment - the current namespacing for properties is PLUGIN/APP/KEY, and our keys should represent both the step (build/deploy/run) and the process type (specific one vs global).

The ui will end up being something like:

# across all processes set
dokku docker-options:add node-js-app deploy,run "-v /var/log/node-js-app:/app/logs"

# across all processes, globally set
dokku docker-options:add --global node-js-app deploy,run "-v /var/log/node-js-app:/app/logs"

# set for a specific process
dokku docker-options:add --process web node-js-app deploy,run "-v /var/log/node-js-app:/app/logs"

# set for multiple processes
dokku docker-options:add --process web --process worker node-js-app deploy,run "-v /var/log/node-js-app:/app/logs"

# maybe instead of having the phase be a positional argument,  support flags for the phases?
dokku docker-options:add --phase deploy --phase run node-js-app "-v /var/log/node-js-app:/app/logs"

Open questions:

  • How does this affect reporting? I feel as though changing that will be a bc-break.
  • Should we also add a :set subcommand? My gut says yes, and we should make the semantics - outside of flag handling - similar to the existing :set subcommands.

@alexquick since you handled the config plugin refactor, thoughts on the feasibility of the above? I don't know how much work the flag handling was.


Original Comment

Scenario

A simple job queue application called jobs-worker with two process defined in Procfile:

web: node app.js
worker: node worker.js

The web process is an http service who register the tasks to be executed by the worker process.
The machine running the container is accessible only from the internal network and communicate only with other servers inside the network through private ip addresses.
For this reason the reverse proxy has to be disabled, and the container should expose a port.

Commands

dokku proxy:disable jobs-worker 
dokku docker-options:add jobs-worker deploy -p "6789:5000"

Issue

The container fails to start the worker process because the port is already allocated for the web process.

Discovering process types
       Procfile declares types -> web, worker
-----> Releasing jobs-worker (dokku/jobs-worker:latest)...
-----> Deploying jobs-worker (dokku/jobs-worker:latest)...
-----> Attempting to run scripts.dokku.predeploy from app.json (if defined)
-----> App Procfile file found (/home/dokku/jobs-worker/DOKKU_PROCFILE)
-----> DOKKU_SCALE file found (/home/dokku/jobs-worker/DOKKU_SCALE)
=====> web=1
=====> worker=1
-----> Attempting pre-flight checks
       For more efficient zero downtime deployments, create a file CHECKS.
       See http://dokku.viewdocs.io/dokku/deployment/zero-downtime-deploys/ for examples
       CHECKS file not found in container: Running simple container check...
-----> Waiting for 10 seconds ...
-----> Default container check successful!
docker: Error response from daemon: driver failed programming external connectivity on endpoint hungry_knuth (e6d4c48658787c0ebf49706a5c3a242514bed3d2811dd1a74ea8afdcf4f8c153): all ports are allocated.
-----> Attempting pre-flight checks
       Non web container detected: Running default container check...
-----> Waiting for 10 seconds ...
App container failed to start!!

Actually the worker process does not need to be exposed.
Is possible to define different docker-options for each process defined in the Procfile?
Is there an alternative way to have the same result?

@marqu3z marqu3z changed the title docker-options doesn't play well with different process in Procfile docker-options doesn't play well with different processes in Procfile Sep 29, 2016
@michaelshobbs
Copy link
Member

Generally speaking a web process doesn't communicate directly with a worker. Usually the work "producer" will place the unit of work on a queue (backed by something like rabbitMQ or redis) and the worker looks for items on that queue.

Most recent frameworks (in Python, Ruby, Node, etc.) give you this ability fairly easily and dokku even has an official redis plugin.

To your question about docker-options per process type, we don't currently support that. However if you want to take a stab at a PR, feel free.

@josegonzalez josegonzalez changed the title docker-options doesn't play well with different processes in Procfile Allow docker-options to be scoped to specific Procfile processes Jan 20, 2017
@josegonzalez
Copy link
Member

@alexquick I mentioned you in the OP, no rush on an answer :)

@alexquick
Copy link
Contributor

alexquick commented May 6, 2018

@josegonzalez I ended up using the flag package to avoid another dependency. I think that was a bad idea as flag is underpowered for a cli as expressive as dokku's. It has no support for positional arguments, which isn't too bad alone, but it also bails on the first non-flag argument which can get you into trouble (particularly with the command:subcommand APP -f --flag convention.

Before (or as part of) tackling this you might want to come up with an acceptable dependency to take and make the chosen parser for the golang core plugins. https://github.com/teris-io/cli looks pretty nice but I haven't spent a lot of time with it. Do you have any suggestions?

Another "issue" with the golang core plugins is that each subcommand is implemented as a separate binary. If they used a flag/argument parsing library that supported subcommands they could be each implemented as a single binary instead. I know having separate binaries for each subcommand is somewhat a feature, but it also makes for a some duplicate code and complicates the build process. Do you have any strong feelings either way?

@josegonzalez
Copy link
Member

I recently used argparse, but it doesn't have support for positional arguments, nargs, or repeatable flags. Ideally we use something that is as powerful as argparse from python. I don't have experience with many other CLI parsers unfortunately.

I'm fine with combining all binaries into one. The dokku bin supports both separate and combined clis. IIRC one of the reasons we separated them was to make parsing a bit easier...

Does anyone have a cli parser they enjoy using?

@josegonzalez
Copy link
Member

Note: we use urfave/cli at work.

@josegonzalez josegonzalez added this to Scheduled for future release in Release Board Dec 2, 2020
@josegonzalez josegonzalez moved this from Scheduled for future release to Backlog in Release Board Dec 2, 2020
@josegonzalez josegonzalez modified the milestones: v0.33.0, v0.34.0 Oct 16, 2023
@josegonzalez josegonzalez added the estimate: 10h Estimated time: 10 hours label Jan 30, 2024
@josegonzalez josegonzalez modified the milestones: v0.34.0, v0.35.0 Mar 13, 2024
@josegonzalez josegonzalez modified the milestones: v0.35.0, v0.36.0 Apr 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Development

No branches or pull requests

4 participants