paratrooper icon indicating copy to clipboard operation
paratrooper copied to clipboard

Would a patch for deploying via a slug be of interest?

Open apsoto opened this issue 9 years ago • 16 comments

We use a regular git deploy to build a slug on a 'build' app, then push the generated slug using the platform-api around to the various apps in our pipeline.

I'm interested in moving our homegrown scripts to using paratrooper, but I'd need to add slug based deploys as well.

Before I go to the effort, would such a patch be of interest or would it be considered out of scope for this project?

apsoto avatar May 21 '15 22:05 apsoto

@apsoto I've never used that kind of deploy before but I'd be interested in seeing how you would go about it. Maybe you could pseudo-code something or do a small spike before going through a larger effort. Thanks for your interest in this library.

mattpolito avatar May 22 '15 00:05 mattpolito

I went ahead and implemented something that works, but I would need to add some specs to provide coverage and any changes based on your feedback before I submit the PR.

https://github.com/mattpolito/paratrooper/compare/master...apsoto:slug_based_deploys

Here's the simple way to use it:

require 'paratrooper'
paratrooper = Paratrooper::Deploy.new('my-app-qa', {slug_app_name: 'my-app-ci'})
paratrooper.deploy

This will deploy the most recently released slug on the app my-app-ci to the app my-app-qa

However, a typical scenario is that during your build process, after an app is deployed to my-app-ci, you store the slug-id as a build artifact that is passed along in your pipeline. Then when you deploy my-app-qa and my-app-production the same slug is deployed consistently across apps:

Therefore, I also added a method to grab the most recently released slug for an app so you can persist the slug-id for future reference:

require 'paratrooper'
# get most recently deployed slug id
slug_id = Paratrooper.deployed_slug('my-app-ci')
# deploy the specified slug id to qa app
Paratrooper::Deploy.new('my-app-qa', {slug_id: slug_id}).deploy

apsoto avatar May 22 '15 21:05 apsoto

any comments? I can move forward anytime, just let me know I'm barking up the right tree.

apsoto avatar May 26 '15 17:05 apsoto

@apsoto sorry about not getting back to you sooner about this. I've been thinking this workflow through and I have a few questions.

Deploying the compiled slug seems like it would simplify and quicken the deploy but what happens to tasks like migrations?

What I can tell after going through what this process would change, is that it bypasses any workflow that Paratrooper provides. Before moving forward I'd like to know more about how Paratrooper would help you if slug based deploys were added.

Thanks again for the pull request! Can't wait to hear more.

mattpolito avatar Jun 04 '15 00:06 mattpolito

Hey There,

I think it shouldn't change anything as far as I can tell.

See the diff in deploy.rb.

The only difference is during the step when it pushes the git repo, it will 'push' the slug. All the other steps should work as before.

apsoto avatar Jun 04 '15 18:06 apsoto

@apsoto after talking this over with some co-workers that are forced into long deploy times due to compilation tasks... these changes are really intriguing. I really like the idea... i'm just trying to wrap my head around how the workflow plays out in different scenarios.

Thank you so much for bringing this idea up.

Since the slug is being thrown at a new instance, the instance has no idea what place the database is at. So it would be up to paratrooper to figure out that migrations need to happen. This already works based on the diff of db. I want to make sure that still plays nicely in a slug based deploy. Can you take a look at your spike to see if you notice any issues with that?

mattpolito avatar Jun 08 '15 12:06 mattpolito

FYI, I've been using slug based deploys for a while, and they are really quick (seconds). I use preboot on all my apps so I haven't tested to see how fast before you are actually serving requests with the new code.

In regards to database, etc. It uses the config vars, so I don't understand why it wouldn't know. I have no problems with running migrations.

I'm already using it in some limited testing on my CI/CD pipelines. Keep an eye on my branch as I'll keep updating it as needed to replace all my custom scripting.

apsoto avatar Jun 09 '15 18:06 apsoto

Another question - I had to add the newer heroku platform-api gem as a dependency. You see any issues with that? I think there should be a separate issue to port the existing paratrooper codebase to the newer gem instead of having both versions as a dependency.

apsoto avatar Jun 09 '15 18:06 apsoto

@apsoto In regards to the migrations... I may not have been clear. Paratrooper figures out if migrations need to be run based on a diff and automatically issues the command for you. That's why I was worried about the diff.

As far as the platform api gem, once this functionality can be finalized. We'll figure out how to do cleanup.

mattpolito avatar Jun 09 '15 19:06 mattpolito

I looked at the code and I see what you mean. I need to test it out. At first glance it seems like it would still work, since you need to be running in the context of a git checkout, but you 'possibly' could run migrations that are not in the slug if the checkout has commits newer than when the slug was created.

However, in my opinion, I question why you need to even bother checking. Running migrations already behaves similarly, if the migration has not been applied, it applies them, otherwise nothing happens. The only benefit that paratrooper provides is the time savings of not running the db:migrate process, which IMO is not a big deal. What was the reasoning for this feature in paratrooper?

apsoto avatar Jun 10 '15 17:06 apsoto

So I've been playing around with the migration check. There's no 'clean' way to skip or force the migration. IMO, it would be simpler to override running the migrations (or not) via some config option like so:

# force running migration task
Paratrooper.deploy(args[:app_name], { migrate: true }).deploy

However, the simplest workaround I could find was using the migration_check option where you provide an object instance that replaces the default Paratrooper::PendingMigrationCheck class.

class RunMigration
  # @param run_migration [Boolean] true, runs migrations
  def initialize(run_migration)
    @run_migration = run_migration
  end

  # forced to implement this since it's invoked from Deploy#setup
  def last_deployed_commit
    'who cares?'
  end

  def migrations_waiting?
    @run_migration
  end
end


# force running migration task
Paratrooper.deploy(args[:app_name], { migration_check: RunMigration.new(true) }).deploy

It's not as clean as I'd like, because I'm forced to implement the #last_deployed_commit method, which IMO, is not needed. For some reason Paratrooper::Deploy#setup invokes that method on the migration checker, but does nothing with the result. In addition, that method is not used anywhere else , so it seems like dead code that can be removed. I'm going to open a separate PR to refactor that out.

apsoto avatar Jun 13 '15 01:06 apsoto

Correction, PendingMigrationCheck uses #last_deployed_commit. I think I've deduced why it's invoked at setup, so the value is looked up before the deploy and not incorrectly after the git push.

apsoto avatar Jun 13 '15 01:06 apsoto

correct

Matt Polito 904.534.0664

On Fri, Jun 12, 2015 at 9:26 PM, Alex Soto [email protected] wrote:

Correction, PendingMigrationCheck uses #last_deployed_commit. I think I've deduced why it's invoked at setup, so the value is looked up before the deploy and not incorrectly after the git push.

— Reply to this email directly or view it on GitHub https://github.com/mattpolito/paratrooper/issues/77#issuecomment-111651223 .

mattpolito avatar Jun 13 '15 01:06 mattpolito

I've submitted a PR #80 to refactor the interface a bit to make it easier to override using the migration_check option

apsoto avatar Jun 13 '15 02:06 apsoto

just pinging to see if we can get some movement on this. Thanks.

apsoto avatar Sep 28 '15 22:09 apsoto

just pinging again to see if we can get any traction on PR #80

apsoto avatar Oct 27 '15 19:10 apsoto