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

bifrost and bookshelf schemas explicit upgrade #715

Merged
merged 3 commits into from
Feb 3, 2016
Merged

bifrost and bookshelf schemas explicit upgrade #715

merged 3 commits into from
Feb 3, 2016

Conversation

marcparadise
Copy link
Member

Instead of upgrading bifrost and bookshelf schemas to the
latest with every chef-server-ctl reconfigure, we're taking the more
conservative approach used for the opscode_chef schema:

  1. Install the schema to the latest version on a new install
  2. Changes to schemas during upgrades must be explicitly performed
    via a partybus migration.

@marcparadise
Copy link
Member Author

Testing in progress, but opening for review now. Note that depending on ongoing discussion, we may expand this to include the necessary partybus changes to support the requirement. The current state of partybus means that we can't perform a schema migration against anything other than the opscode_chef database.

@stevendanna
Copy link
Contributor

we may expand this to include the necessary partybus changes to support the requirement

I think that is a good idea. I'd like the support to be there for the next person who has to do a schema update.

# EITHER: :service - name of key into 'secrets' that should contain
# sql_user and sql_password entries,
# OR: :username, :password for sql auth
def run_sqitch(target, path, database, credentials)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps this might be a bit nicer from the caller's perspective if we just had to pass in a single 'service' argument that would know how to look up the correct path, username, password, and database. We could leave the final param as an optional opts hash for when you need to set things manually:

def run_sqitch(target, service, opts={})
   username = opts[:username] || username_for_service(service)
   password =  opts[:password] || password_for_service(service)
   database = opts[:database] || database_for_service(service)
   path = opts[:path] || path_for_service(service)
   ...
end

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I started down that path, but currently can't recall why I didn't stick with it. In any case - that sounds good to me.

@stevendanna
Copy link
Contributor

Overall, looks good. Made a minor suggestion regarding the API of run_sqitch, let me know what you think.

@marcparadise
Copy link
Member Author

Updated with a variant on the comments above.

@@ -1,6 +1,7 @@
define_upgrade do
if Partybus.config.bootstrap_server
must_be_data_master
run_sqitch("@cbv-type", "@1.0.4")
# Performance improvements for cookbook fetching.
run_sqitch('@cbv-type', 'oc_erchef/schema')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be 'oc_erchef' I think?

@sdelano
Copy link
Contributor

sdelano commented Jan 29, 2016

👍

end

def default_opts_for_service(service)
username = Partybus.config.secrets[service]['sql_user']
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The sql_user isn't stored in the secrets hash, which is why this is failing tests.

Marc Paradise added 2 commits February 3, 2016 15:14
Instead of upgrading bifrost and bookshelf schemas to the
latest with every chef-server-ctl reconfigure, we're taking the more
conservative approach used for the opscode_chef schema:

1. Install the schema to the latest version on a new install
2. Changes to schemas during upgrades must be explicitly performed
   via a partybus migration.
Partybus.run_sqitch was previously hard-coded
to apply database migrations only to opscode_chef,
from the opscode_chef schema path. It also
re-applied the legacy 1.0.4 schema with every new schema change.

This change updates the function so that it only applies the migration
it's told to apply. It also expands it to permit other services
to invoke run_sqitch for updating their own databases.

The partybus migrations were retrofitted to use the new form.

In addition migration 014 was updated to explicitly apply the
legacy 1.0.4 schema.

This was added as a precaution -- there should be no path that
has migration 14 (or later) running, that does not already have
the baseline sqitch deployments applied:
 * new installations will already have the baseline properly applied.
 * EC 11.current will already have the baseline
   properly applied, as sqitch was introduced sometime early in the
   11.x timeframe.
 * Upgrading from 11.old (pre-sqitch) requires an upgrade to
   11.x current before proceeding.
sdelano added a commit that referenced this pull request Feb 3, 2016
bifrost and bookshelf schemas explicit upgrade
@sdelano sdelano merged commit 7fb3909 into master Feb 3, 2016
@sdelano sdelano deleted the mp/HA-23 branch February 10, 2016 20:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants