search

PomeloFoundation / Pomelo.EntityFrameworkCore.MySql

Entity Framework Core provider for MySQL and MariaDB built on top of MySqlConnector
MIT License
2.67k stars 379 forks source link

Migration bundles does not work in the context of MySQL database and CICD pipeline in Github Action #1761

Open xboxeer opened 1 year ago

xboxeer commented 1 year ago

File a bug

I was following this document to deploy an app service with MySQL DB, the sample it uses is SQL Server code first and run the below 2 command in Github Action

      - name: dotnet ef install
        run: dotnet tool install dotnet-ef -g

      - name: dotnet ef create bundle
        run: dotnet ef migrations bundle -p DotNetCoreSqlDb/DotNetCoreSqlDb.csproj -o ${{env.DOTNET_ROOT}}/myapp/migrate -v

Everything works will with SQL server, but when I change to MySQL( I want to do a demo for Azure MySQL), CICD pipeline keeps failing, indicating it is can not connect to MySQL something I'm very confused is that:

  1. The connection string for SQL Server is just a dummy connection string but command dotnet ef migrations bundle is still able to create bundles even if it can not connect to actual SQL Server
  2. When it comes to MySQL, I'm still providing a dummy connection string, replaced db provider to Pomelo.EntityFrameworkCore.MySQL
  3. The reason I'm using dummy connection string is that the actual connection string is stored on App Service config, I hope none of the actual connection is stored in local
  4. Looks like when using Pomelo.EntityFrameworkCore.MySQL, dotnet ef migrations bundles requires the MySQL to be exist and try to connect to the MySQL server - which is impossible in a CICD pipeline, for example, github action
  5. I tried to run a mysql in CICD pipeline, turns out the efbundles do got created, however it can not read the actual appsettings on production env (a remote mysql instance), it is still try to connect to local mysql, which is not available in production env, looks like Pomelo.EntityFrameworkCore.MySQL will embed the connection string into the generated migration bundles?

    Include your code

    appsettings.json

    
{
"Logging": {
"LogLevel": {
  "Default": "Information"
}
},
"AllowedHosts": "*",
"ConnectionStrings": {
"AZURE_MYSQL_CONNECTIONSTRING": "Server=Server=localhost;UserID=root; Password=root;Database=mssqllocaldb"
}
} 

```csharp
services.AddDbContext<MyDatabaseContext>(options =>
                    options.UseMySql(ServerVersion.AutoDetect(Configuration.GetConnectionString("AZURE_MYSQL_CONNECTIONSTRING")))) ;

Include stack traces

MySqlConnector.MySqlException (0x80004005): Unable to connect to any of the specified MySQL hosts.
   at MySqlConnector.Core.ServerSession.ConnectAsync(ConnectionSettings cs, MySqlConnection connection, Int32 startTickCount, ILoadBalancer loadBalancer, Activity activity, IOBehavior ioBehavior, CancellationToken cancellationToken) in /_/src/MySqlConnector/Core/ServerSession.cs:line 448
   at MySqlConnector.Core.ConnectionPool.ConnectSessionAsync(MySqlConnection connection, String logMessage, Int32 startTickCount, Activity activity, IOBehavior ioBehavior, CancellationToken cancellationToken) in /_/src/MySqlConnector/Core/ConnectionPool.cs:line 403
   at MySqlConnector.Core.ConnectionPool.ConnectSessionAsync(MySqlConnection connection, String logMessage, Int32 startTickCount, Activity activity, IOBehavior ioBehavior, CancellationToken cancellationToken) in /_/src/MySqlConnector/Core/ConnectionPool.cs:line 408
   at MySqlConnector.Core.ConnectionPool.GetSessionAsync(MySqlConnection connection, Int32 startTickCount, Activity activity, IOBehavior ioBehavior, CancellationToken cancellationToken) in /_/src/MySqlConnector/Core/ConnectionPool.cs:line 98
   at MySqlConnector.Core.ConnectionPool.GetSessionAsync(MySqlConnection connection, Int32 startTickCount, Activity activity, IOBehavior ioBehavior, CancellationToken cancellationToken) in /_/src/MySqlConnector/Core/ConnectionPool.cs:line 128
   at MySqlConnector.MySqlConnection.CreateSessionAsync(ConnectionPool pool, Int32 startTickCount, Activity activity, Nullable`1 ioBehavior, CancellationToken cancellationToken) in /_/src/MySqlConnector/MySqlConnection.cs:line 929
   at MySqlConnector.MySqlConnection.OpenAsync(Nullable`1 ioBehavior, CancellationToken cancellationToken) in /_/src/MySqlConnector/MySqlConnection.cs:line 423
   at MySqlConnector.MySqlConnection.Open() in /_/src/MySqlConnector/MySqlConnection.cs:line [38](https://github.com/xboxeer/msdocs-app-service-sqldb-dotnetcore/actions/runs/4444325302/jobs/7802424749#step:7:39)2
   at Microsoft.EntityFrameworkCore.ServerVersion.AutoDetect(String connectionString)
   at DotNetCoreSqlDb.Startup.<ConfigureServices>b__4_0(DbContextOptionsBuilder options) in /home/runner/work/msdocs-app-service-sqldb-dotnetcore/msdocs-app-service-sqldb-dotnetcore/DotNetCoreSqlDb/Startup.cs:line 32

Include verbose output

Finding IDesignTimeDbContextFactory implementations...
Finding application service provider in assembly 'DotNetCoreSqlDb'...
Finding Microsoft.Extensions.Hosting service provider...
Using environment 'Development'.
Using application service provider from Microsoft.Extensions.Hosting.

Include provider and version information

EF Core version: Database provider: (e.g. Pomelo.EntityFrameworkCore.MySql) Target framework: (e.g. .NET 6.0) CICD Pipeline: Github Action

xboxeer commented 1 year ago

anyone look into this issue？

nothrow commented 1 year ago

The issue lies in the ServerVersion.AutoDetect() call - it tries to connect to the server to determine protocol version. Replace it with something like ServerVersion.Create(Version.Parse("1.0.0.0"), ServerType.MariaDb).

klinki commented 6 months ago

Hi, I have similar issue - is it possible to somehow use ServerVersion.AutoDetect() with migration bundle?

Sometimes you don't know the server version in advance and it depends on what customer uses.

lauxjpn commented 4 months ago

The issue lies in the ServerVersion.AutoDetect() call - it tries to connect to the server to determine protocol version. Replace it with something like ServerVersion.Create(Version.Parse("1.0.0.0"), ServerType.MariaDb).

That is indeed the reason a connection is opened. Specifying the server version manually, e.g. var serverVersion = ServerVersion.Parse("11.2.3-mariadb") solves this issue.

Our general recommendation is to always explicitly set the ServerVersion in your production code. ServerVersion.AutoDetect() exists primarily for examples or relatively simple demos and projects.

lauxjpn commented 4 months ago

Hi, I have similar issue - is it possible to somehow use ServerVersion.AutoDetect() with migration bundle?

@klinki Seems to me this is the exact opposite of the issue that @xboxeer had, if you want to use ServerVersion.AutoDetect() with migration bundle.

To answer the question, you can use ServerVersion.AutoDetect() wherever you want, including design time code that is hit when you generate a migration bundle. But it always requires a live database server to determine its version. It is really just a simple helper method. This is how the method is implemented:

https://github.com/PomeloFoundation/Pomelo.EntityFrameworkCore.MySql/blob/c8bf3ea8a25f4cc5d1a6a5f884d2a4993e6e6dff/src/EFCore.MySql/Infrastructure/ServerVersion.cs#L61-L82

In case you don't want to use ServerVersion.AutoDetect(), you need a way for your users to specify the ServerVersion they want to use. For example, you could require them to call a method of your library to specify the version, or you could require them to set an environment variable that your code checks. In both cases, you would throw an exception, if a user did not set the ServerVersion. There are many other ways to handle this, depending on the requirements that you have.

xboxeer commented 4 months ago

Does this Version AutoDetect detect if the server is 5.7 or 8.0, or it will tells you the exact minor version number? such as 8.0.34/8.0.35? I'm asking this because as a PaaS DB provider, a maintenance from our end will update the engine version(8.0.34->8.0.35), If AutoDetect tells you the exact minor version, I'm afraid it will somehow break the existing code

lauxjpn commented 4 months ago

Does this Version AutoDetect detect if the server is 5.7 or 8.0, or it will tells you the exact minor version number?

@xboxeer It uses whatever the database server returns. That should be the same as the following query returns:

select @@version;

For MySQL, that looks like 8.0.36. For MariaDB, that looks like 11.3.2-MariaDB-1:11.3.2+maria~ubu2204.

So it normally returns at least the major, minor and patch version.

This is also necessary, because there are feature differences between individual patch versions in MySQL (see MySqlServerVersion and MariaDbServerVersion for which features are supported since what versions).

I'm afraid it will somehow break the existing code

How is that? Please elaborate.

The returned server version should correspond to the Oracle/MariaDB version that the server implementation is compatible with.

If you want to change you individual server implementation version, without changing the MySQL compatibility version, you should probably append your maintenance information to the MySQL version string, similar to how MariaDB adds additional information to their version string.

For reference, this is how we parse the returned version string:

https://github.com/PomeloFoundation/Pomelo.EntityFrameworkCore.MySql/blob/d75b62e7573041d99e91b98378f598d3080ef828/src/EFCore.MySql/Infrastructure/ServerVersion.cs#L180-L208

klinki commented 4 months ago

Hi, I have similar issue - is it possible to somehow use ServerVersion.AutoDetect() with migration bundle?

@klinki Seems to me this is the exact opposite of the issue that @xboxeer had, if you want to use ServerVersion.AutoDetect() with migration bundle.

To answer the question, you can use ServerVersion.AutoDetect() wherever you want, including design time code that is hit when you generate a migration bundle. But it always requires a live database server to determine its version. It is really just a simple helper method. This is how the method is implemented:

It would be great if it was possible to use something like optionsBuilder.UseMySql where it would automatically call ServerVersion.AutoDetect but just when it needed it. Maybe overload with bool autodetectServer parameter?

Right now, it is needed to manually call ServerVersion.AutoDetect (which makes migrations hard to use as it would add requirement to have live DB available during build time - not very CI friendly) or manually specify server version. Hardcoding version is not a good idea, that would make migrations much less universal. So one has to do some workarounds with passing server version in command line argument or environment variable. This just adds unnecessary burden for migrations and it is more error prone - there could be mismatch between version user specified and version that is really on server.

lauxjpn commented 4 months ago

@klinki

It would be great if it was possible to use something like optionsBuilder.UseMySql where it would automatically call ServerVersion.AutoDetect but just when it needed it.

This is not going to happen any time soon, if at all. Currently, every .UseMySql() call needs to know the server version. The server version is always needed to ensure correct operation of Pomelo.

Right now, it is needed to manually call ServerVersion.AutoDetect (which makes migrations hard to use as it would add requirement to have live DB available during build time - not very CI friendly).

To generate migrations, a server version needs to be specified. How you do it is up to you. If you want to auto detect it, feel free to do so. If you want to specify the version manually, then do that. If you want to specify a fixed version for all your clients, it might result in fewer features being used (if you use an older version) or features being used that are not available on older servers (e.g. if you use MySqlServerVersion.LatestSupportedServerVersion or MariaDbServerVersion.LatestSupportedServerVersion), but you can still do it if you want.

We don't care how you specify the server version, or which one you specify (though you should care), but you have to specify a version (for migrations and for runtime). Pomelo's functionality depends heavily on knowing the server version it should support.

So either detect the server version, or specify it manually in some way. I don't see a third option to do this. The responsibility to choose a strategy that works for you and your clients is yours alone.

If it is you yourself who is adding new migrations centrally for all your clients, then you could e.g. try to use a fixed server version for that task and probably get away with it. Adding a new migration (generating the Up() and Down() method code) is usually not that server version specific (as long as you don't add custom SQL statements to it at least).

Your clients would then apply those migrations to their databases or generate the migration scripts using their individual real server version, e.g. by using AutoDetect() (that is necessary because those tasks are highly server version specific, e.g. whether or not the ALTER TABLE RENAME COLUMN syntax is supported).

Though I would still recommend when using a strategy like that, to test your migrations against typical server versions that your clients run, to ensure that your migrations actually work as intended.