ksmonkey123 commented 3 months ago

⚠️ This issue respects the following points: ⚠️

[X] This is a bug, not a question or a configuration/webserver/proxy issue.
[X] This issue is not already reported on Github OR Nextcloud Community Forum (I've searched it).
[X] Nextcloud Server is up to date. See Maintenance and Release Schedule for supported versions.
[X] I agree to follow Nextcloud's Code of Conduct.

Bug description

The OCC-Command db:convert-type seems to ignore the DB properties (type, user, host, database, port, password) provided through the CLI and instead operates on the database configured in config.php

I believe this may be related to the commit 79c4986 as this reworked the parameter handling in ConnectionFactory.php.

If I read this correctly, the default parameters are now always generated based on the system config instead of the passed arguments.

Running db:convert-type --clean-schema drops the tables in the "old" database.

Steps to reproduce

Have Nextcloud 29.0.0 running with a MySQL database, and an empty PostgreSQL database
run php occ db:convert-type --password=<PASSWORD> --port=5432 pgsql nextcloud_app postgres nextcloud (note: the host postgres is a Kubernetes service name)
The command executes no matter if the Postgres Database is running or not, the command throws Constraint Violation errors in the table oc_accounts, I assume because it tries to migrate the content of the old database into itself.

Expected behavior

db:convert-type

Installation method

Community Docker image

Nextcloud Server version

29

Operating system

Debian/Ubuntu

PHP engine version

None

Web server

None

Database engine version

MySQL

Is this bug present after an update or on a fresh install?

None

Are you using the Nextcloud Server Encryption module?

None

What user-backends are you using?

[X] Default user-backend (database)
[ ] LDAP/ Active Directory
[ ] SSO - SAML
[ ] Other

Configuration report

{
    "system": {
        "htaccess.RewriteBase": "\/",
        "memcache.local": "\\OC\\Memcache\\APCu",
        "apps_paths": [
            {
                "path": "\/var\/www\/html\/apps",
                "url": "\/apps",
                "writable": false
            },
            {
                "path": "\/var\/www\/html\/custom_apps",
                "url": "\/custom_apps",
                "writable": true
            }
        ],
        "instanceid": "***REMOVED SENSITIVE VALUE***",
        "passwordsalt": "***REMOVED SENSITIVE VALUE***",
        "secret": "***REMOVED SENSITIVE VALUE***",
        "trusted_domains": [
            "192.168.178.32",
            "cloud.awae.ch"
        ],
        "trusted_proxies": "***REMOVED SENSITIVE VALUE***",
        "datadirectory": "***REMOVED SENSITIVE VALUE***",
        "dbtype": "mysql",
        "version": "29.0.0.19",
        "overwrite.cli.url": "https:\/\/cloud.awae.ch",
        "dbname": "***REMOVED SENSITIVE VALUE***",
        "dbhost": "***REMOVED SENSITIVE VALUE***",
        "dbport": "",
        "dbtableprefix": "oc_",
        "mysql.utf8mb4": true,
        "dbuser": "***REMOVED SENSITIVE VALUE***",
        "dbpassword": "***REMOVED SENSITIVE VALUE***",
        "installed": true,
        "overwriteprotocol": "https",
        "theme": "",
        "loglevel": 2,
        "maintenance": false,
        "mail_smtpmode": "smtp",
        "mail_sendmailmode": "smtp",
        "mail_smtpauth": 1,
        "mail_smtpauthtype": "LOGIN",
        "mail_smtphost": "***REMOVED SENSITIVE VALUE***",
        "mail_smtpport": "25",
        "mail_from_address": "***REMOVED SENSITIVE VALUE***",
        "mail_domain": "***REMOVED SENSITIVE VALUE***",
        "mail_smtpname": "***REMOVED SENSITIVE VALUE***",
        "mail_smtppassword": "***REMOVED SENSITIVE VALUE***",
        "mail_smtpsecure": "tls",
        "default_language": "de",
        "default_phone_region": "CH",
        "app_install_overwrite": [
            "polls"
        ]
    }
}

List of activated Apps

Enabled:
  - activity: 2.21.1
  - circles: 29.0.0-dev
  - cloud_federation_api: 1.12.0
  - comments: 1.19.0
  - contactsinteraction: 1.10.0
  - dashboard: 7.9.0
  - dav: 1.30.1
  - federatedfilesharing: 1.19.0
  - federation: 1.19.0
  - files: 2.1.0
  - files_downloadlimit: 2.0.0
  - files_pdfviewer: 2.10.0
  - files_reminders: 1.2.0
  - files_sharing: 1.21.0
  - files_trashbin: 1.19.0
  - files_versions: 1.22.0
  - firstrunwizard: 2.18.0
  - logreader: 2.14.0
  - lookup_server_connector: 1.17.0
  - nextcloud_announcements: 1.18.0
  - notes: 4.10.0
  - notifications: 2.17.0
  - oauth2: 1.17.0
  - password_policy: 1.19.0
  - photos: 2.5.0
  - polls: 7.0.3
  - privacy: 1.13.0
  - provisioning_api: 1.19.0
  - recommendations: 2.1.0
  - related_resources: 1.4.0
  - serverinfo: 1.19.0
  - settings: 1.12.0
  - sharebymail: 1.19.0
  - support: 1.12.0
  - survey_client: 1.17.0
  - systemtags: 1.19.0
  - text: 3.10.0
  - theming: 2.4.0
  - twofactor_backupcodes: 1.18.0
  - updatenotification: 1.19.1
  - viewer: 2.3.0
  - workflowengine: 2.11.0
Disabled:
  - admin_audit: 1.19.0
  - bruteforcesettings: 2.9.0 (installed 2.4.0)
  - encryption: 2.17.0
  - extract: 1.3.6 (installed 1.3.6)
  - files_external: 1.21.0
  - files_rightclick: 0.15.1 (installed 1.6.0)
  - maps: 1.1.0 (installed 1.1.0)
  - suspicious_login: 7.0.0
  - twofactor_totp: 11.0.0-dev
  - user_ldap: 1.20.0
  - user_status: 1.9.0 (installed 1.4.0)
  - weather_status: 1.9.0 (installed 1.4.0)

Nextcloud Signing status

No errors have been found.

Nextcloud Logs

No response

Additional info

Docker Image: nextcloud:29.0.0
MySQL Database: mysql:8.4.0
Postgres Databse: postgres:16.3

ksmonkey123 commented 3 months ago

Logging the Parameter $additionalConnectionParams in ConnectionFactory.php on Line 166 shows the following values for the "fromDB" and the "toDB":

FromDB

ToDB

It is notable that in the second case (which should be the postgres-database configured via the CLI arguments), the host, dbname and port in the top-level array are set correctly from the CLI, Both the primary and the replica fields and even some top-level fields obviously use the DB-configs from config.php

joshtrichards commented 3 months ago

Possibly related: #45097

I don't have time to look tonight, but your analysis seems sound (and, given the added evidence from the log output, even more so).

Cc: @juliushaertl

intersectRaven commented 3 months ago

I can verify but can't contribute logs. I used the --clear-schema option in my attempt and my original MariaDB DB got deleted. Good thing I have backups.

Delagen commented 3 months ago

I modified https://github.com/nextcloud/server/blob/master/core/Command/Db/ConvertType.php#L247 to

$connectionParams = $this->connectionFactory->getDefaultConnectionParams($type);

and seems all converted

Also have to remove mysql.utf8mb4 parameter from config.php to perform conversion to remove client_encoding from params of DbFactory

HammyHavoc commented 3 months ago

Got hit by this a week ago. Is that particular Nextcloud instance completely screwed?

joshtrichards commented 3 months ago

Yes, unfortunately. :cry: The end result is the parameters get swapped so it likely dropped the source database.

Fortunately, recovery should be a simple matter of restoring from backup (just the db portion): https://docs.nextcloud.com/server/latest/admin_manual/maintenance/restore.html#restore-database (or whatever other mechanism you utilize in your environment for database backup/recovery)

joshtrichards commented 3 months ago

@Delagen Mind pushing that as PR? I know there may be additional changes needed, but it'll start things in motion.

Delagen commented 3 months ago

@joshtrichards I don't mind that my changes solve the problem correctly. It only makes possible for me successful conversion from MySQL to Postgres

chimpboy commented 3 months ago

I have just experienced this problem on a new installation.

rotdrop commented 3 months ago

Would not be it the simplest solution, if the ConnectionFactory::getConnection() method would first check if the $additionalConnectionParams argument contains explicit connection parameters and in that case simply skip that fancy primary / replica stuff? At least for the case were the explicitly specified connection differs from the "internal" database connection configured in config/config.php. In principle this should restore the previous behaviour when using the factory for alternate database connections.

szaimen commented 3 months ago

Hi, is this https://github.com/nextcloud/server/pull/45013 fixing the problem?

jiriks74 commented 2 months ago

I just looked into it a bit and so far it looks like the databases get mixed up somewhere in the script.

When I hardcoded the toDB values it tried to connect to the target database (PostgreSQL) with the credentials of my current database (MariaDB/MySQL). It used the username from the old database on the new one.

juliushaertl commented 2 months ago

Possible patch candidate, but haven't had time to verify and test this yet:

diff --git a/core/Command/Db/ConvertType.php b/core/Command/Db/ConvertType.php
index 333f29625f6..078fd47965b 100644
--- a/core/Command/Db/ConvertType.php
+++ b/core/Command/Db/ConvertType.php
@@ -227,15 +227,22 @@ class ConvertType extends Command implements CompletionAwareInterface {
        protected function getToDBConnection(InputInterface $input, OutputInterface $output) {
                $type = $input->getArgument('type');
                $connectionParams = $this->connectionFactory->createConnectionParams();
-               $connectionParams = array_merge($connectionParams, [
+
+               $overrideConnectionParams = [
                        'host' => $input->getArgument('hostname'),
                        'user' => $input->getArgument('username'),
                        'password' => $input->getOption('password'),
                        'dbname' => $input->getArgument('database'),
-               ]);
+               ];
                if ($input->getOption('port')) {
-                       $connectionParams['port'] = $input->getOption('port');
+                       $overrideConnectionParams['port'] = $input->getOption('port');
                }
+
+               $connectionParams = array_merge($connectionParams, [
+                       'primary' => $overrideConnectionParams,
+                       'replica' => [$overrideConnectionParams],
+               ]);
+
                return $this->connectionFactory->getConnection($type, $connectionParams);
        }

szaimen commented 2 months ago

Hi @intersectRaven @ksmonkey123 @Delagen @HammyHavoc @chimpboy @rotdrop are you able to test the above patch? Thanks in advance! :)

jiriks74 commented 2 months ago

@szaimen I'll be probably playing around with a new (old) server this evening. I'll back up my database and try the patch out. I'll know practically immediately as I get the duplicate entry error right at the start if it tries to write into the original DB.

jiriks74 commented 2 months ago

I tried it out, it doesn't work. I get stuck on Creating schema in new database even after hardcoding the connections for my databases to be sure that it's passed right. I'll try to find out where the issue is.

jiriks74 commented 2 months ago

The conversion script gets stuck on this line

https://github.com/nextcloud/server/blob/34f5cba5b3cf84d5f85e95985b044ef9ab002f82/core/Command/Db/ConvertType.php#L210

leon1995 commented 1 month ago

Are there any new information on this?

zackgomez commented 1 month ago

Any progress?

Is the current guidance to create a fresh system losing an existing data?

Infamousbugg commented 1 month ago

I also have a migration on hold waiting for this issue to be resolved.

CorneliousJD commented 1 month ago

Unfortunately continuing to wait. I continue to have DB corruption w/ MariaDB and am anxious to switch over to Postgres but this seemingly keeps getting pushed back?

Investigamer commented 1 month ago

If this issue is still weeks/months away from a proper fix, could we at least stem the bleeding with a FAQ or Wiki entry on how to manually migrate from one database format to another without using occ, citing this current issue and giving notice that a fix is forthcoming?

rankerson commented 1 month ago

If this issue is still weeks/months away from a proper fix, could we at least stem the bleeding with a FAQ or Wiki entry on how to manually migrate from one database format to another without using occ, citing this current issue and giving notice that a fix is forthcoming?

I think it is a good sign, that there has been some action. I assume it is now on the direct todo list of the developers. So let us continue to hope :-)

lwenderoth commented 1 month ago

@Delagen wrote here :

I modified https://github.com/nextcloud/server/blob/master/core/Command/Db/ConvertType.php#L247 to
$connectionParams = $this->connectionFactory->getDefaultConnectionParams($type);
and seems all converted

Also have to remove mysql.utf8mb4 parameter from config.php to perform conversion to remove client_encoding from params of DbFactory

Did someone test this? It is now line 229 though. I came up with the same solution. The problem seems to be that createConnectionParams() always returns the currently configured DBs values while getDefaultConnectionParams($type) does what it's name suggests: Return default values for $type, NOT values for the configured DB.

jiriks74 commented 3 weeks ago

It seems like a fix is finally on it's way (https://github.com/nextcloud/server/pull/46931) and it's been approved for a backported to v29 (https://github.com/nextcloud/server/pull/47046)

joshtrichards commented 3 weeks ago

Now that #47046 is merged in stable29 (you'll need to manually patch until officially released), if you wish to test a db conversion, you will also need to revert the code change made in #45469 (which disabled the db:convert-type command to avoid data loss).

I have not had a chance to test #47046 so I've not submitted a PR to revert the disabling of the command yet. (P.S. This doesn't have to wait on me).

jiriks74 commented 3 weeks ago

I could get to it next week. I'm on holiday until Wednesday and I don't feel like converting remotely. If no one else gets to it I probably will by the end of the next week.

TheTimeWalker commented 2 weeks ago

I have tried the patch on my side, but it seems like that it still doesn't properly pass the sql type parameter. At least if you try to use something like mysql -> mysql or mysql -> sqlite3, occ properly rejects it. However, when you use pgsql then the connection param still defaults to pdo_mysql:

Command used: occ db:convert-type pgsql ncadmin cloud-pg-temp nextcloud_db (note: host is a named hostname)

{
    "adapter": "OC\\DB\\AdapterMySQL",
    "charset": "utf8mb4",
    "driver": "pdo_mysql",
    "wrapperClass": "OC\\DB\\Connection",
    "driverOptions": {
        "1005": true
    },
    "user": "ncadmin",
    "password": "test",
    "host": "cloud-pg-temp",
    "dbname": "nextcloud_db",
    "tablePrefix": "oc_",
    "sqlite.journal_mode": "WAL",
    "defaultTableOptions": {
        "collate": "utf8mb4_bin",
        "charset": "utf8mb4",
        "tablePrefix": "oc_"
    },
...

asmoker commented 2 weeks ago

Any progress? 🤨

Queuecumber commented 1 week ago

I have tried the patch on my side, but it seems like that it still doesn't properly pass the sql type parameter.

I can confirm this is still bugged even with the patch, the code as written doesn't initialize the "to" database connection correctly.

It looks like it's because https://github.com/nextcloud/server/blob/307608b26cc0492eda5657bf674ec634523a63ee/core/Command/Db/ConvertType.php#L229 calls the connection factory function createConnectionParams(); which populates a bunch of defaults from the "from" database configuration which in my case is mysql.

That comes from a change from 8 months ago: 79c4986354da2c7dd102de21174114e7687daf98 which adds the call to

$connectionParams = array_merge($this->getDefaultConnectionParams($type),  ...

namely getDefaultConnectionParams was not there before and that is what pulls in defaults for the wrong type of database. So I think this has been broken for some time now. I haven't been able to find a workaround of it yet.

CC @juliushaertl the author of the commit in question

Queuecumber commented 1 week ago

I finally got this running but it's quite hacky, I can post what I did if theres interest but there should be a proper solution to this

normen commented 1 week ago

I finally got this running but it's quite hacky, I can post what I did if theres interest but there should be a proper solution to this

Yes please, how did you get it to work?

Edit: Getting the same issue, also in 29.0.5 when removing the block. Apparently the database type for the target database is always the same as the current database. I am trying to move from postgresql to mariadb and it tries to connect to the right server but on the wrong port, because it tries to connect to a postgresql database instead of a mysql one..

Queuecumber commented 1 week ago

Unfortunately after upgrading to .0.5 my changes are gone but I can try to recreate it although it may require some debugging to get it right

In lib/private/DB/ConnectionFactory.php I changed the function createConnectionParams to take a type parameter, so something like

public function createConnectionParams(string $configPrefix = '', array $additionalConnectionParams = [], $type = '') {
                if ($type === '') {
                $type = $this->config->getValue('dbtype', 'sqlite');
                 }

Then I changed core/Command/Db/ConvertType.php to remove the guard and then in getToDBConnection I pass the type parameter to the updated function from above, should be something like:

$connectionParams = $this->connectionFactory->createConnectionParams($type);

I was converting from MariaDB to PG and I also have to set mysql.utf8mb4 = false in config.php for some reason, but after these updates it worked.

normen commented 1 week ago

Great, thanks, that works! For reference, the only line that I had to implement differently from the above was this (compatible to PHP 7):

$connectionParams = $this->connectionFactory->createConnectionParams('', [], $type);

nextcloud / server

[Bug]: db:convert-type uses systemConfig instead of command line parameters for connection to new target database #45257

⚠️ This issue respects the following points: ⚠️

Bug description

Steps to reproduce

Expected behavior

Installation method

Nextcloud Server version

Operating system

PHP engine version

Web server

Database engine version

Is this bug present after an update or on a fresh install?

Are you using the Nextcloud Server Encryption module?

What user-backends are you using?

Configuration report

List of activated Apps

Nextcloud Signing status

Nextcloud Logs

Additional info

FromDB

ToDB