Skip to content

Changes database upgrade script names to be consistent for the 4.9.1.0 release - #1665

Merged
asfgit merged 1 commit into
apache:4.9from
shapeblue:jsb/4.9.1.0-version
Sep 2, 2016
Merged

Changes database upgrade script names to be consistent for the 4.9.1.0 release#1665
asfgit merged 1 commit into
apache:4.9from
shapeblue:jsb/4.9.1.0-version

Conversation

@jburwell

Copy link
Copy Markdown
Contributor
  • Changes the names of the schema-490to491* scripts to
    schema-490to4910*
  • Changes the name of the Upgrade490to491 class to Upgrade490to4910
  • Modifies the Marvin setup.py script to use version 4.9.1.0-SNAPSHOT

/cc @rhtyd @karuturi

@jburwelljburwell changed the title Changes database upgrade script names consistent for the 4.9.1.0 releaseChanges database upgrade script names to be consistent for the 4.9.1.0 releaseAug 29, 2016
@yadvr

Copy link
Copy Markdown
Member

@jburwell -1
The db upgrade version only considers the major/minor fields such as a.b.c, the last .d is ignored (for that matter any other suffix in the version). Due to the current mechanism of how db versions are implemented, used and run, the change cannot be accepted as it will cause db upgrade failures when the upgrade path is run (as you can see Travis has failed all jobs at the deploydb step). The db upgrade paths therefore won't run for any security release (for example, b/w 4.9.1.0 and 4.9.1.1), unless we want to have db changes between security releases for the same major.minor release?

@yadvr

Copy link
Copy Markdown
Member

With new db schema/versioning and upgrade changes in #1654 we can merge this with some additional changes once #1654 is merged and fwd-merged.

@yadvr

yadvr commented Sep 1, 2016

Copy link
Copy Markdown
Member

@jburwell this needs to be reworked now that #1654 is merged. Good to have the changes but not a blocker for 4.8, 4.9 RCs.

@jburwell

Copy link
Copy Markdown
ContributorAuthor

@rhtyd I have fixed the DatabaseUpgradeCheckerTest to reflect 4.9.1.0 being a NoopUpgrade. I also removed the unnecessary 4.9.0->4.9.1.0 upgrade scripts and class.

@yadvr

yadvr commented Sep 2, 2016

Copy link
Copy Markdown
Member

@jburwell we'll need that upgrade path class, we have some alter table changes we want for the 4.9.0->4.9.1.0 upgrade path, see the diff for changes you've removed.

…version scheme
* Renames schema-490to491*.sql to schema490to4910*.sql
* Renames the Upgrade490to491 class to Upgrade490to4910
* Removes the unused s_logger contant from Upgrade490to4910
* Updates the version in tools/marvin/setup to 4.9.1.0-SNAPSHOT
@jburwell

Copy link
Copy Markdown
ContributorAuthor

@rhtyd good catch -- sorry for that oversight. I have redone the PR to only rename the files and class.

@yadvr

yadvr commented Sep 2, 2016

Copy link
Copy Markdown
Member

LGTM

@yadvr

yadvr commented Sep 2, 2016

Copy link
Copy Markdown
Member

Tests performed:

- Upgraded from 4.9.0, upgrade path ran (whose sql file names were changed)
- Clean db deployment

@yadvr

yadvr commented Sep 2, 2016

Copy link
Copy Markdown
Member

Tests performed after fwd-merge, all pass:

- Upgraded from 4.9.1.0
- Clean db deployment

@milamberspace

Copy link
Copy Markdown
Contributor

LGTM.

Tested on CS 4.9.0 to 4.9.1.0-snapshot installation.
(perhaps just change the comment below inside the SQL file to indicate 4.9.1.0 (missing .0?)
"-- Schema upgrade from 4.9.0 to 4.9.1;"

Logs:
2016-09-02 15:13:45,721 INFO c.c.u.DatabaseUpgradeChecker (logid:) DB version = 4.9.0 Code Version = 4.9.1.0
2016-09-02 15:13:45,722 INFO c.c.u.DatabaseUpgradeChecker (logid:) Database upgrade must be performed from 4.9.0 to 4.9.1.0
2016-09-02 15:13:45,723 DEBUG c.c.u.DatabaseUpgradeChecker (logid:) Running upgrade Upgrade490to4910 to upgrade from 4.9.0-4.9.1.0 to 4.9.1.0
[...]
2016-09-02 15:13:45,984 DEBUG c.c.u.DatabaseUpgradeChecker (logid:) Cleanup script /usr/share/cloudstack-management/setup/db/schema-490to4910-cleanup.sql is executed successfully
2016-09-02 15:13:46,019 DEBUG c.c.u.DatabaseUpgradeChecker (logid:) Upgrade completed for version 4.9.1.0

@yadvr

yadvr commented Sep 2, 2016

Copy link
Copy Markdown
Member

Thanks @milamberspace
Based on the tests and LGTMs, I'll proceed with merging this on 4.9. On fwd-merging, I've fixed conflicts and renamed a subsequent class to match the change. Fwd-merged master branch has also passed my tests.

@asfgit
asfgit merged commit eabda0e into apache:4.9Sep 2, 2016
asfgit pushed a commit that referenced this pull request Sep 2, 2016
Changes database upgrade script names to be consistent for the 4.9.1.0 release * Changes the names of the schema-490to491* scripts to
schema-490to4910*
* Changes the name of the Upgrade490to491 class to Upgrade490to4910
* Modifies the Marvin setup.py script to use version 4.9.1.0-SNAPSHOT
/cc @rhtyd @karuturi
* pr/1665:
Renames of 4.9.0->4.9.1.0 upgrade scripts to match the four position version scheme
Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>
Sign up for freeto 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

@jburwell@yadvr@milamberspace@asfgit