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

Revert "Run stack_check after DB migrations and seeds (#453)" #454

Merged
merged 1 commit into from
Aug 14, 2024

Conversation

xandroc
Copy link
Contributor

@xandroc xandroc commented Aug 8, 2024

This reverts commit f445c39.

We can now run the stack checker before migrations as per cloudfoundry/cloud_controller_ng#3925.

This is an improvement as we want to fail the stack checker before any migrations have run so that we are not in a state where migrations have run but the deployment has failed

  • I have viewed signed and have submitted the Contributor License Agreement

  • I have made this pull request to the develop branch

  • I have run CF Acceptance Tests on bosh lite

This reverts commit f445c39.

We can now run the stack checker before migrations as per
cloudfoundry/cloud_controller_ng#3925.

This is an improvement as we want to fail the stack checker before any
migrations have run so that we are not in a state where migrations
have run but the deployment has failed

Co-authored-by: Sam Gunaratne <sam.gunaratne@broadcom.com>
@philippthun
Copy link
Member

I'm still unsure if this works for a from-scratch setup (as it's done in the CAPI CI pipeline for three environments). I think at this time - i.e. the very first time this runs on a vm on a new foundation - the database is completely empty (no tables etc.). So loading the models should actually fail...

@xandroc
Copy link
Contributor Author

xandroc commented Aug 9, 2024

I'm still unsure if this works for a from-scratch setup (as it's done in the CAPI CI pipeline for three environments). I think at this time - i.e. the very first time this runs on a vm on a new foundation - the database is completely empty (no tables etc.). So loading the models should actually fail...

Thanks @philippthun, perhaps we can detect if any migrations have run at all do you have any recommendations on how we might do that?

@philippthun
Copy link
Member

@xandroc Hm, maybe connect to the database and check if the migrations table exists:

db = VCAP::CloudController::DB.connect(RakeConfig.config.get(:db), logger)
return unless db.table_exists?(:schema_migrations)

...

@xandroc
Copy link
Contributor Author

xandroc commented Aug 9, 2024

Thanks @philippthun we added a variation, schema_migrations table does exist prior to migrations running so we checked for stacks table
next unless db.table_exists?(:stacks)

cloudfoundry/cloud_controller_ng@d854dab

@xandroc
Copy link
Contributor Author

xandroc commented Aug 12, 2024

@philippthun can you give the changes in this pr a review, thanks

@Samze Samze requested a review from philippthun August 12, 2024 16:43
@Samze
Copy link
Contributor

Samze commented Aug 12, 2024

LGTM given cloudfoundry/cloud_controller_ng#3925

@Samze Samze requested a review from sethboyles August 14, 2024 17:09
@xandroc xandroc marked this pull request as ready for review August 14, 2024 17:14
@Samze Samze merged commit 8517ca8 into develop Aug 14, 2024
2 checks passed
@moleske moleske deleted the improve_stack_checker branch August 15, 2024 02:50
Sign up for free to 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