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

How to do a missing migration check in CI #26348

Closed
maxkoshevoi opened this issue Oct 14, 2021 · 26 comments · Fixed by #31164
Closed

How to do a missing migration check in CI #26348

maxkoshevoi opened this issue Oct 14, 2021 · 26 comments · Fixed by #31164
Assignees
Labels
area-migrations area-tools closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. community-contribution customer-reported type-enhancement
Milestone

Comments

@maxkoshevoi
Copy link

Ask a question

I have an idea to add "missing migration" check to my CI pipeline. This check should validate that DbSnapshot that is present in a branch matches models in that branch (in another words, it should validate that if I will create a migration, Up and Down would be empty).

Straightforward way of doing this would be to execute dotnet ef migrations add Test and validate that resulting files match "empty migration" ones, but this seems like a hack.

Is there an easier way of doing this? Like dotnet ef migrations --verify-snapshot-up-to-date or something?

Include provider and version information

EF Core version:
Database provider: Microsoft.EntityFrameworkCore.SqlServer
Target framework: .NET 6.0
Operating system: Linux (for CI pipeline)
IDE: Visual Studio 2022

@ajcvickers
Copy link
Contributor

@maxkoshevoi We discussed this in triage; there isn't an easy way to do this now, but we agree that it would be useful.

Notes from triage:

  • Issue Tools: Warn when pending model changes #22105 covers warning for pending migrations and possible API surface to support this
    • With API, a unit test could be added to check there are no pending migrations
    • It would be useful to check for pending migrations in migrations list
  • Using this issue to track command-line experience for asking specifically if there are pending migrations
    • A command that returns zero if there are no pending migrations and non-zero if there are would be useful in the C.I. where the return value is used to stop/continue the process.

@PavelStefanov
Copy link

Hey!

I found workaround, mb it'll be useful for somebody. You can use migration list command with --json parameter.

dotnet ef migrations list --json

It gives you structed list of migrations with applied property. You can use it to find pending migrations

[  
  {
    "id": "20220420211839_migration1",
    "name": "migration1",
    "safeName": "migration1",
    "applied": true
  },
  {
    "id": "20220621122349_migration2",
    "name": "migration2",
    "safeName": "migration2",
    "applied": false
  }
]

@maxkoshevoi
Copy link
Author

It gives you structed list of migrations with applied property. You can use it to find pending migrations

The issue here is not that migration hasn't been applied, but that it hasn't been created (there're some model changes that are not reflected in DbContextModelSnapshot.cs), so I don't think your solution is applicable here.

@PavelStefanov
Copy link

It gives you structed list of migrations with applied property. You can use it to find pending migrations

The issue here is not that migration hasn't been applied, but that it hasn't been created (there're some model changes that are not reflected in DbContextModelSnapshot.cs), so I don't think your solution is applicable here.

Oh sorry. I was confused cuz my issue was marked as related.

@ErikEJ
Copy link
Contributor

ErikEJ commented Jun 21, 2022

@aboryczko
Copy link

aboryczko commented Jul 4, 2022

if you are ok with using some internal EF Core this does what you're asking for:

[Fact]
public void Should_not_have_missing_migrations()
{
    var builder = new DesignTimeServicesBuilder(typeof(MyDbContext).Assembly, Assembly.GetExecutingAssembly(), new OperationReporter(null), Array.Empty<string>());
    var provider = builder.Build(GetDbContext());
    var dependencies = provider.GetRequiredService<MigrationsScaffolderDependencies>();
    var modelSnapshot = dependencies.MigrationsAssembly.ModelSnapshot;
    var model = dependencies.SnapshotModelProcessor.Process(modelSnapshot?.Model);
    var relationalModel = model?.GetRelationalModel();

    var hasDifferences = dependencies.MigrationsModelDiffer.HasDifferences(relationalModel, dependencies.Model.GetRelationalModel());

    hasDifferences.Should().BeFalse();
}

@maxkoshevoi
Copy link
Author

@ErikEJ @aboryczko Thank you! This should work as a workaround.
I'd still like to keep this issue open until a more straight forward way is added into the EF Core

@Pentadome
Copy link

Pentadome commented Sep 12, 2022

if you are ok with using some internal EF Core this does what you're asking for:

[Fact]
public void Should_not_have_missing_migrations()
{
    var builder = new DesignTimeServicesBuilder(typeof(MyDbContext).Assembly, Assembly.GetExecutingAssembly(), new OperationReporter(null), Array.Empty<string>());
    var provider = builder.Build(GetDbContext());
    var dependencies = provider.GetRequiredService<MigrationsScaffolderDependencies>();
    var modelSnapshot = dependencies.MigrationsAssembly.ModelSnapshot;
    var model = dependencies.SnapshotModelProcessor.Process(modelSnapshot?.Model);
    var relationalModel = model?.GetRelationalModel();

    var hasDifferences = dependencies.MigrationsModelDiffer.HasDifferences(relationalModel, dependencies.Model.GetRelationalModel());

    hasDifferences.Should().BeFalse();
}

For anyone using this, if the type 'DesignTimeServicesBuilder' can not be found, make sure "Microsoft.EntityFrameworkCore.Design" is not marked as a private asset in your .csproj file. (It is by default)

@aboryczko Thanks btw!

Checking if a migration is missing in your PR pipeline seems like a no-brainer to me, hoping ef core will get native support for this.

@BenMakesGames
Copy link

BenMakesGames commented Jan 17, 2023

  1. am I right in thinking I need to implement GetDbContext, or is that method provided by something else?
  2. if I do need to write it, any pointers on how to do so?

a unit test is a cool way to solve the problem! however, am I right in understanding that I need to implement GetDbContext myself? this is the path I took, and in order to write that method, I need (I think?) to pull a connection string from the environment-appropriate appsettings file (ex: appsettings.Staging.json), both during CI/CD pipeline (which I think I can do using info from this thread: microsoft/vstest#669 though I haven't tried it yet), AND for devs running tests via their IDE (haven't figured out a way to do this, yet). I've got something like this at the moment:

    private MyDbContext GetDbContext()
    {
        var builder = new ConfigurationBuilder()
            .AddJsonFile("appsettings.json", false, false)
            .AddJsonFile($"appsettings.{Environment.GetEnvironmentVariable("ASPNETCORE_ENVIRONMENT") ?? "Production"}.json", false, false)
            // ^ 🤷???
        ;

        var config = builder.Build();

        var connectionString = config.GetConnectionString("Db")!;

        var optionsBuilder = new DbContextOptionsBuilder<MyDbContext>();
        optionsBuilder.UseSqlServer(connectionString);

        var context = new MyDbContext(optionsBuilder.Options);

        return context;
    }

(maybe I could figure out how to get an IApplicationEnvironment in here, but grabbing ASPNETCORE_ENVIRONMENT manually felt like it ought to be good enough?)

@aboryczko
Copy link

@BenMakesGames yes, you need to create the context. I use something more complex to get the context via IServiceProvider in a base test class, but what you have written should work fine. For a unit test this would be enough:

var builder = new DesignTimeServicesBuilder(typeof(MyDbContext).Assembly, Assembly.GetExecutingAssembly(), new OperationReporter(null), Array.Empty<string>());
var optionsBuilder = new DbContextOptionsBuilder<MyDbContext>().UseSqlServer();
var dbContext = new MyDbContext(optionsBuilder.Options);
var provider = builder.Build(dbContext);
...

of course if you use a different Database Provider you should change UseSqlServer() to something else.

@BenMakesGames
Copy link

after starting to write a response, I think I'm understanding now; let me confirm: you're saying there's no need to connect to a specific database, because all the information needed to determine "is a new migration needed?" can be determined from code alone? that feels right, and I'll test that now.

@aboryczko
Copy link

Correct.

@justindbaur
Copy link
Contributor

I've started some work on implementing this but would need sign off from the team before making a PR.

A command that returns zero if there are no pending migrations and non-zero if there are would be useful in the C.I. where the return value is used to stop/continue the process.

Based on @ajcvickers comment it looks like the team was more leaning towards this being it's own command but from a simplicity standpoint it would be easier if it were an optional switch on the migrations add command. The experience for using such a switch would be like:

dotnet ef migrations add Test --fail-on-empty

The weirdness of this usage is that a lot of us want it to fail if a migration with actual operations is created. It's possible to invert that check based on exit code in a CI pipeline but generally it's more annoying. The other way would be to fail on there being actual operations but I personally haven't thought of a name that I like. Things I have thought about: --require-empty, --verify-no-migrations, and the one thrown out in the issue body --verify-snapshot-up-to-date. I'm completely open to suggestions.

If we were to make it it's own command the design I have thought about is:

dotnet ef migrations verify --no-changes

It feels a little extra to have it's own command for this to me since the code for it would essentially need emulate getting to this part of the code like add. I'm open to any design but would love to start a discussion here about it because I'm serious about implementing this if a PR would be welcome.

@ajcvickers ajcvickers removed this from the Backlog milestone Apr 17, 2023
@ajcvickers
Copy link
Contributor

@justindbaur We discussed this and we don't think it makes sense to have this be part of the add command. It's really not the same concept.

@ajcvickers ajcvickers added this to the Backlog milestone Apr 27, 2023
@justindbaur
Copy link
Contributor

@ajcvickers I don't disagree, it would feel a little weird in the add command, it just would have required the least code changes 😆.

Did the team have any thoughts about what a verify-like command might look like?

@ajcvickers ajcvickers removed this from the Backlog milestone May 1, 2023
@ajcvickers
Copy link
Contributor

@bricelam Could you add your thoughts here?

@bricelam
Copy link
Contributor

bricelam commented May 4, 2023

Dupe of #22105

@maxkoshevoi
Copy link
Author

@bricelam can warning fail CI pipeline?

@bricelam
Copy link
Contributor

bricelam commented May 4, 2023

Most people I've seen write a unit test:

[Fact]
public void No_pending_model_changes()
{
    using var context = new MyDbContext();

    var modelDiffer = context.GetService<IMigrationsModelDiffer>();

    var migrationsAssembly = context.GetService<IMigrationsAssembly>();
    var modelInitializer = context.GetService<IModelRuntimeInitializer>();
    var snapshotModel = migrationsAssembly.ModelSnapshot?.Model;
    if (snapshotModel is IMutableModel mutableModel)
        snapshotModel = mutableModel.FinalizeModel();
    if (snapshotModel is not null)
        snapshotModel = modelInitializer.Initialize(snapshotModel);

    var designTimeModel = context.GetService<IDesignTimeModel>();

    var pendingModelChanges = modelDiffer.HasDifferences(
        snapshotModel?.GetRelationalModel(),
        designTimeModel.Model.GetRelationalModel())

    Assert.Empty(pendingModelChanges);
}

#22105 is also about adding a simpler API (something like context.Database.HasPendingModelChanges()) to do this.

@ajcvickers
Copy link
Contributor

Note from triage. This issue is still tracking having a command-line experience for asking specifically if there are pending migrations A command that returns zero if there are no pending migrations and non-zero if there are would be useful in the C.I. where the return value is used to stop/continue the process.

This command would be something like dotnet ef migrations check-pending.

@ajcvickers ajcvickers added this to the Backlog milestone May 17, 2023
@BenMakesGames
Copy link

BenMakesGames commented Jun 29, 2023

just wanted to pop in to give a warning: after updating to the latest minor version of EF 7 (we went from 7.0.2 to 7.0.8), @aboryczko's solution stopped working for me! happily, @bricelam's code works!

I only needed some small tweaks to create an options builder (we're using DateOnly in some of our entities, which requires the ErikEJ.EntityFrameworkCore.SqlServer.DateOnlyTimeOnly package until EF 8, but I assume that's not a super-common use-case).

@justindbaur
Copy link
Contributor

I've created a PR adding the check-pending command if people subscribed here have any notes! #31164

@bricelam bricelam self-assigned this Jul 7, 2023
@bricelam
Copy link
Contributor

bricelam commented Jul 8, 2023

Looks like we started talking about two separate issues here.

React to this comment with 🚀 if you're interested in a command that checks whether there are any migrations that haven't been applied to a database (i.e. if you need to run dotnet ef database update)

React with 👀 if you're interested in a command that checks there are any model changes that haven't yet been added to a migration (i.e. if you need to run dotnet ef migrations add)

React with both if you want both.

@bricelam
Copy link
Contributor

FYI, you can test for unapplied migrations today using grep:

dotnet ef migrations list | grep "(Pending)"

@phatt-millips
Copy link

phatt-millips commented Jul 12, 2023

My GitHub Action to ensure that no new migrations are needed

  migration-test:
    name: Migration Test
    runs-on: ubuntu-latest

    steps:
      - uses: actions/checkout@v2

      - name: Setup .NET
        uses: actions/setup-dotnet@v1
        with:
          dotnet-version: '6.0.x'

      - name: Install EF Core CLI
        run: dotnet tool install --global dotnet-ef

      - name: Generate Migrations
        run: dotnet ef migrations add ${{ github.sha }} <your-params>

      - name: Check Created Migration File Against Regex
        run: |
            if [[ ! $(find <your-migration-folder> -name "*${{ github.sha }}.cs" | xargs grep -E -zo 'protected\s+override\s+void\s+Up\(.*\)\s*\{\s*\}\s*protected\s+override\s+void\s+Down\(.*\)\s*\{\s*\}') ]]; then
                echo "Unapplied Migration Found. Make sure you have run 'dotnet ef migration add ... and committed the generated migration file. See https://learn.microsoft.com/en-us/ef/core/managing-schemas/migrations/?tabs=dotnet-core-cli for more information."
                exit 1
            fi

@phatt-millips
Copy link

Our team doesn't necessarily need something like dotnet ef migrations list | grep "(Pending)" because we always apply pending migrations in the deployment process. However, we have left the add migration step up to the developer so they have to commit the migration, designer, and updated snapshot files

@bricelam bricelam added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Jul 18, 2023
@bricelam bricelam modified the milestones: Backlog, 8.0.0-preview7 Jul 18, 2023
@ajcvickers ajcvickers modified the milestones: 8.0.0-preview7, 8.0.0 Nov 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-migrations area-tools closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. community-contribution customer-reported type-enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants