Skip to content

Conversation

@phisch
Copy link
Contributor

@phisch phisch commented Jun 29, 2017

Description

This PR adds a migration from stable8.2 to stable9. The migration runs before the stable9 to stable10 migration, therefore it is possible to upgrade from stable8.2 to stable10 directly.

Related Issue

https://github.com/owncloud/enterprise/issues/2010

Motivation and Context

So people can update more easily.

How Has This Been Tested?

Currently only by hand, by me. Will be tested automatically. Need help from qa though!

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation. <- maybe, to get awareness
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@phisch phisch added this to the 10.0.3 milestone Jun 29, 2017
@phisch phisch self-assigned this Jun 29, 2017
}

if ($schema->hasTable("${prefix}file_map")) {
$schema->dropTable("${prefix}file_map");
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@DeepDiver1975 @PVince81 i don't know what data is in this table. Is it important, do i have to transform and put it into another table?

Copy link
Member

Choose a reason for hiding this comment

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

no - this data is not needed

Copy link
Contributor

Choose a reason for hiding this comment

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

🔥

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍

@PVince81
Copy link
Contributor

PVince81 commented Jun 29, 2017

In the past, some apps had update routines in "apps/$appName/appinfo/update.php". We need to make sure these still exist and run on stable10.

The ones I know of that I checked:

  • stable8.2 <-> stable10
    • files: seems ok
    • 🚫 files_trashbin: file exists but some pieces were deleted in stable10
    • files_sharing update routine to flatten shares: update.php is still there
    • 🚫 files_external update routine is gone: it converts an older storage config format to "oc_mounts" etc tables
    • 🚫 files_versions: file deleted in stable10
    • encryption: no update file
    • 🚫 user_ldap: update.php was removed in the marketplace version
  • stable9 <-> stable10
  • stable9.1 <-> stable10

@PVince81
Copy link
Contributor

One worry is that some "update.php" code relies on other stuff being migrated first through other means.

So if we put these update.php routines into migrations, I suggest to order them exactly by order of major versions. So run the update.php codes from stable9 before running the stable9.1 migrations, etc. Just to be sure.

Ideal would be to also have test cases that cover whatever these update.php routines were fixing.

@PVince81 PVince81 added the p1-urgent Critical issue, need to consider hotfix with just that issue label Jul 11, 2017
@phisch
Copy link
Contributor Author

phisch commented Jul 12, 2017

files_trashbin: file exists but some pieces were deleted in stable10

The expire trash background task has been moved into the appinfo.xml: https://github.com/owncloud/core/blob/master/apps/files_trashbin/appinfo/info.xml

Might need to execute automatically on update?

files_external update routine is gone: it converts an older storage config format to "oc_mounts" etc tables

This is a real concern. We need to add the migration steps that are here:
https://github.com/owncloud/core/blob/stable9/apps/files_external/migration/storagemigrator.php

@PVince81 should i add it to the app again? Or should we include it in the core migrations?

files_versions: file deleted in stable10

Background job available as Command in https://github.com/owncloud/core/blob/master/apps/files_versions/appinfo/info.xml
No work required here!

user_ldap: update.php was removed in the marketplace version
The update groups logic has been moved to https://github.com/owncloud/user_ldap/blob/master/lib/Jobs/UpdateGroups.php and enabled in info.xml

The CleanUp job has been removed since the tables it was cleaning do not exist anymore.
This looks fine for me. No work required!

TODO:

  • check if files_trashbin OCA\Files_Trashbin\Command\ExpireTrash need to be executed on update

Trashbin expire during update was extracted into a command, because it can cause the update to take extremely long for large instances. This means we don't need to do this for this update.

@PVince81
Copy link
Contributor

if adding stuff back to update.php is quicker then I suggest we go that route. I expect that after the OC to 10.0 migration we might remove these again at some point...

@CLAassistant
Copy link

CLAassistant commented Jul 13, 2017

CLA assistant check
All committers have signed the CLA.

@PVince81
Copy link
Contributor

I'd like to see the checkboxes here updated before merging: #28256 (comment)

@phisch
Copy link
Contributor Author

phisch commented Jul 28, 2017

The storagemigration (https://github.com/owncloud/core/blob/stable9/apps/files_external/migration/storagemigrator.php) can't be added here because of missing services in the files_external app.

If we want to support migrating global and user mounts, we should add those legacy services again to files_external and add it as migration there.

@PVince81
Copy link
Contributor

PVince81 commented Aug 3, 2017

If we want to support migrating global and user mounts, we should add those legacy services again to files_external and add it as migration there.

that would be fine

@phisch phisch force-pushed the stable10_upgrade_from_stable8_2 branch from d3006a3 to 46c5a91 Compare August 7, 2017 07:32
@PVince81
Copy link
Contributor

PVince81 commented Aug 7, 2017

@phisch can you post a list of items/checkbox of the ton of testing you have done lately?

@PVince81
Copy link
Contributor

PVince81 commented Aug 7, 2017

and also please update the checkboxes above with the bits you have already fixed. Thanks

@phisch
Copy link
Contributor Author

phisch commented Aug 11, 2017

Tested the migration for all major databases:

  • sqlite
  • mysql
  • pgsql
  • oracle

Looked for missing actions/migrations from update.php files in apps. There are no missing actions except for the mounts that will be added in the files_external app.

Merging this PR.

@phisch phisch merged commit 52a6f04 into master Aug 11, 2017
@PVince81 PVince81 deleted the stable10_upgrade_from_stable8_2 branch August 11, 2017 11:00
@PVince81
Copy link
Contributor

@phisch thanks. Will you add this separately ?

add migrations for https://github.com/owncloud/core/blob/stable9/apps/files_external/migration/storagemigrator.php

@PVince81
Copy link
Contributor

please backport this PR to stable10

@phisch
Copy link
Contributor Author

phisch commented Aug 11, 2017

Will you add this separately ?

@PVince81 yes

Backport incomming.

phisch added a commit that referenced this pull request Aug 11, 2017
* added migration from stable8.2 to stable9

* addressbooks.description can be null

* added missing changes from diff between migrated mysql table and native mysql table

* added autoincrement, unsigned, notnull and collation explicitly

* needed to set notnull to false explicitly and remove the collation definition

* order mounts by storage_id, because postgres returns it not in the order they where added which fails some expectations in tests
PVince81 pushed a commit that referenced this pull request Aug 11, 2017
[stable10] migration from stable8.2 to stable9 (#28256)
@lock
Copy link

lock bot commented Aug 2, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Aug 2, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

3 - To Review p1-urgent Critical issue, need to consider hotfix with just that issue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants