dokku-persistent-storage plugin with latest buildstep fails in build phase

[michaelshobbs]

I believe our change to include docker-args in the build phase is causing some issues.

ref: https://github.com/progrium/dokku/blob/fb3492c8242521dec4ed8428983ea752db710f86/dokku#L58-L59

From /build/builder (using heroku-buildpack-python):

       =====> Downloading Buildpack: https://github.com/heroku/heroku-buildpack-python.git
       =====> Detected Framework: Python
remote: rm: cannot remove '/app/certs': Device or resource busy
To dokku@localhost:api-staging.xxx.com
 ! [remote rejected] deploy -> master (pre-receive hook declined)
error: failed to push some refs to 'dokku@localhost:api-staging.xxx.com'

From /build/builder (using heroku-buildpack-ruby):

-----> Discovering process types
       Procfile declares types -> web, db-migrate
       Default process types for Multipack -> rake, console, web
rm: cannot remove '/app/data': Device or resource busy

Same heroku-buildpack-ruby run but without the volume mounted:

+ /tmp/buildpacks/heroku-buildpack-multi/bin/release /tmp/build /cache
+ echo_title 'Discovering process types'
----->' Discovering process types
-----> Discovering process types
+ [[ -f /tmp/build/Procfile ]]
++ ruby -e 'require '\''yaml'\'';puts YAML.load_file('\''/tmp/build/Procfile'\'').keys().join('\'', '\'')'
+ types='web, db-migrate'
+ echo_normal 'Procfile declares types -> web, db-migrate'
      ' Procfile declares types '->' web, db-migrate
       Procfile declares types -> web, db-migrate
+ default_types=
+ [[ -f /tmp/build/.release ]]
++ ruby -e 'require '\''yaml'\'';puts (YAML.load_file('\''/tmp/build/.release'\'')['\''default_process_types'\''] || {}).keys().join('\'', '\'')'
+ default_types='rake, console, web'
+ [[ -n rake, console, web ]]
+ echo_normal 'Default process types for Multipack -> rake, console, web'
      ' Default process types for Multipack '->' rake, console, web
       Default process types for Multipack -> rake, console, web
+ [[ -f /tmp/build/.release ]]
+ ruby -e 'require '\''yaml'\'';(YAML.load_file('\''/tmp/build/.release'\'')['\''config_vars'\''] || {}).each{|k,v| puts "export #{k}='\''#{v}'\''"}'
+ cat
+ chmod +x /start
+ cp -fp start exec
+ rm -rf /app
+ mv /tmp/build /app

So the python buildpack is doing something with the /app dir and buildstep is as well. (https://github.com/progrium/buildstep/blob/091eafdad7ccf5f4584aa21704397fd1dc1fd208/builder/builder#L70)

I have a PR open against dokku-persistent-storage (dyson/dokku-persistent-storage#12) but I wonder if we're just asking for more problems? Perhaps documenting the pluginhook phase argument would be helpful as well? In any case, I think we may run into more of these issues. If nothing else, it's documented here.

EDIT: for clarification

[josegonzalez]

The more I think about it, the more I feel we should have separate docker-args-PHASE pluginhooks. It jives with how we do all other pluginhooks that are for specific phases.

[michaelshobbs]

How would we migrate from the current situation? The docker-args plugin is kind of special snowflake in the ecosystem as well.

[josegonzalez]

Well we could just tag a minor release. Thats what semver is for, right?

[michaelshobbs]

Suppose that could work. 😄

[vincentfretin]

+1 for docker-args-PHASE pluginshooks

[michaelshobbs]

link #896

[michaelshobbs]

@josegonzalez @vincentfretin
One potential problem I see with docker-args-PHASE pluginhooks is that if an author wanted their plugin to be included in multiple phases, then they would need a soft link or similar for all desired phases.

Thoughts on this?

[dyson]

I think that's totally fine @michaelshobbs to have to add the multiple docker-args-PHASE files if you want to run during multiple phases; it's not a big problem. Plus, it will make people think more about if it needs to be run during those phases and if any unique things have to happen during the phase. Also, the added bonus is when you looks at a pluginhooks source you get a very quick overview of what phases it runs in.

[michaelshobbs]

Makes sense. Just wanted to make sure we're consciously moving in this direction. 😄

[josegonzalez]

+1 :)

[vincentfretin]

I agree with @dyson