voxpupuli / puppet-gitlab

Puppet module to manage Gitlab (Omnibus)
https://forge.puppet.com/puppet/gitlab/
BSD 3-Clause "New" or "Revised" License
74 stars 164 forks source link

Automation of Zero-Downtime Deployments (and use of Post Deployment Migrations logic) #264

Open esalberg opened 6 years ago

esalberg commented 6 years ago

Looking at #239 and its related PR #251

However, looking at the documentation, it suggests that it's still necessary to run the migrations, just after the installation has been upgraded:

https://docs.gitlab.com/ee/update/#upgrading-without-downtime

https://docs.gitlab.com/ee/development/post_deployment_migrations.html#deployment-integration

"The process is similar for other deployment techniques: first you would deploy with the environment variable set, then you'll essentially re-deploy a single server but with the variable unset."

Or here: https://docs.gitlab.com/omnibus/update/README.html#zero-downtime-updates

I don't see that the code runs the post-upgrade migration steps. Am I missing something? (I was assuming that these steps are run every upgrade, but I could be wrong.)

If there is consensus that the extra steps need to be added, I can work on a PR, but I wanted to make sure the theory was right first.

LongLiveCHIEF commented 6 years ago

This is mostly meant to be used in a HA deployment. As a result, you'll configure one of your deploy nodes without this setting, which will cause these migrations to be run by omnibus.

We do need a task added if you choose to run these manually via bolt, but it's not necesarry in order to support zero-downtime upgrades.

Related: https://github.com/voxpupuli/puppet-gitlab/issues/239

LongLiveCHIEF commented 6 years ago

The big challenge in full HA support right now, of which this is loosely related, is sequencing involved in the upgrade of multiple nodes. I still need to open an issue on that, but it will likely only be solved via tasks/plans.

As far as this issue goes, I think right now, the best we can do is improve the documentation and/or add a db:migrate task. As noted in #239 this feature isn't strictly tied to HA, so it's needed if the skip_post_deployment_migrations is true. Right now, there isn't any support in the module to run post migrations on a deploy node/node that was run with skip_post_deployment_migrations.

I added the enhancement label to indicate this issue requires some work and is more than just a question.

LongLiveCHIEF commented 6 years ago

The big challenge in full HA support right now, of which this is loosely related, is sequencing involved in the upgrade of multiple nodes.

This is the last bit I feel is part of the MVP for the "this module supports GitLab HA" statement I want to put in the README when we reaach that milestone.

esalberg commented 6 years ago

I can see your point, but there is also documentation for using the functionality in a single installation here: https://docs.gitlab.com/omnibus/update/README.html#single-deployment

I see the value in setting up the process as a task instead of code, although shouldn't the package installation still kick off the "SKIP_POST_DEPLOYMENT_MIGRATIONS=true sudo gitlab-ctl reconfigure" in code, and the task consist of the db:migrate and service HUPs?

As a slightly tangential note, I just upgraded from 10.8.7 to 11.2.3, and it didn't kick off a gitlab-ctl reconfigure. It looks like the module only does that on config changes, not on package updates. Is a reconfigure no longer needed on a major package version change? It's a little hard to tell from the omnibus update doc page (reconfigure is documented for skip auto but not for regular - perhaps the reconfiguration is built into the package now?).

Thanks for the review. :)

LongLiveCHIEF commented 6 years ago

It looks like the module only does that on config changes, not on package updates. Is a reconfigure no longer needed on a major package version change?

@esalberg see the information provided in https://github.com/voxpupuli/puppet-gitlab/issues/240

LongLiveCHIEF commented 6 years ago

although shouldn't the package installation still kick off the "SKIP_POST_DEPLOYMENT_MIGRATIONS=true sudo gitlab-ctl reconfigure" in code, and the task consist of the db:migrate and service HUPs?

Yes, and we still need to implement the task part of that.