Revert migration squash that breaks 15.11
What does this MR do and why?
In April, I had !116021 (merged) merged, which squashed a migration that had a timestamp from January, but its corresponding commit was dated for October, which is after the squash date. This caused #409331 (closed), which prompted me to revert the squash in !118368 (merged). The revert was not present in 15.11.0 and 15.11.1, so any upgrades from 15.11.x to 16 will result in the un-squashed migrations being present for the upgrade, resulting in db:migrate
attempting to re-run the init_schema migration, which naturally results in a failure.
MR acceptance checklist
This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.
-
This MR is backporting a bug fix, documentation update, or spec fix, previously merged in the default branch. -
The original MR has been deployed to GitLab.com (not applicable for documentation or spec changes). -
This MR has a severity label assigned (if applicable). -
This MR has been approved by a maintainer (only one approval is required). -
Ensure the e2e:package-and-test-ee
job has either succeeded or been approved by a Software Engineer in Test.
Note to the merge request author and maintainer
The process of backporting bug fixes into stable branches is tracked as part of an internal pilot. If you have questions about this process, please:
- Refer to the internal pilot issue for feedback or questions.
- Refer to the patch release runbook for engineers and maintainers for guidance.
Merge request reports
Activity
changed milestone to %16.0
assigned to @jon_jenkins
To speed up your revert merge request, please consider using the revert merge request template. Adding the appropriate labels for resolving master:broken before the merge request is created will skip several CI/CD jobs.
For this merge request, if this is for resolving master:broken you can add the appropriate labels present in the merge request template, and trigger a new pipeline. It will be faster
.added pipeline:run-all-e2e label
3 Warnings This merge request is definitely too big (13522 lines changed), please split it into multiple merge requests. Most of the time, merge requests should target master
. Otherwise, please set the relevantPick into X.Y
label.The e2e:package-and-test-ee
job needs to succeed or have approval from a Software Engineer in Test.
Read the "QA e2e:package-and-test-ee" section for more details.Reviewer roulette
Changes that require review have been detected!
Please refer to the table below for assigning reviewers and maintainers suggested by Danger in the specified category:
Category Reviewer Maintainer backend Eduardo Bonet (
@eduardobonet
) (UTC+2, 7 hours ahead of@jon_jenkins
)Michael Kozono (
@mkozono
) (UTC-10, 5 hours behind@jon_jenkins
)~"group::integrations" (backend) No engineer is available for automated assignment, please reach out to the #g_manage_integrations
Slack channel or mention@gitlab-org/manage/integrations
for assistance.Maintainer review is optional for ~"group::integrations" (backend) ~"migration" No reviewer available No maintainer available To spread load more evenly across eligible reviewers, Danger has picked a candidate for each review slot, based on their timezone. Feel free to override these selections if you think someone else would be better-suited or use the GitLab Review Workload Dashboard to find other available reviewers.
To read more on how to use the reviewer roulette, please take a look at the Engineering workflow and code review guidelines. Please consider assigning a reviewer or maintainer who is a domain expert in the area of the merge request.
Once you've decided who will review this merge request, assign them as a reviewer! Danger does not automatically notify them for you.
QA
e2e:package-and-test-ee
@jon_jenkins, the
package-and-test
job must complete before merging this merge request.*If there are failures on the
package-and-test
pipeline, ping your team's associated Software Engineer in Test (SET) to confirm the failures are unrelated to the merge request. If there's no SET assigned, ask for assistance on the#quality
Slack channel.If needed, you can retry the
danger-review
job that generated this comment.Generated by
Danger- Resolved by 🤖 GitLab Bot 🤖
Proper labels assigned to this merge request. Please ignore me.
@jon_jenkins - please see the following guidance and update this merge request.2 Errors Please add typebug typefeature, or typemaintenance label to this merge request. typeignore is not a valid type label for merge requests and should be used only for issues. Please see this guidance for more details on typeignore. Edited by 🤖 GitLab Bot 🤖
added severity2 typebug labels and removed typeignore label
added Pick into 15.11 label
added bugavailability label
severity2 with bugavailability can only have priority1
Please see the Availability prioritization guidelines for more detail.
This message was generated automatically. You're welcome to improve it.
requested review from @samsam_kim
- Resolved by Dylan Griffith
@samsam_kim can you please take backend review and then @sabrams can you take maintainer review? I know you're somewhat familiar with the migration squash process, and this is a bugfix to get the migration revert into 15.11.
Allure report
allure-report-publisher
generated test report!e2e-package-and-test:
test report for 93520402expand test summary
+-----------------------------------------------------------------------+ | suites summary | +------------------+--------+--------+---------+-------+-------+--------+ | | passed | failed | skipped | flaky | total | result | +------------------+--------+--------+---------+-------+-------+--------+ | Data Stores | 185 | 0 | 3 | 10 | 188 | ❗ | | Configure | 1 | 0 | 15 | 0 | 16 | ✅ | | Create | 753 | 0 | 108 | 65 | 861 | ❗ | | Verify | 270 | 0 | 20 | 15 | 290 | ❗ | | Plan | 312 | 0 | 0 | 84 | 312 | ❗ | | Manage | 222 | 0 | 20 | 48 | 242 | ❗ | | Analytics | 11 | 0 | 0 | 10 | 11 | ❗ | | Monitor | 62 | 0 | 1 | 10 | 63 | ❗ | | Release | 30 | 0 | 0 | 5 | 30 | ❗ | | Govern | 235 | 0 | 0 | 40 | 235 | ❗ | | Growth | 0 | 0 | 10 | 0 | 10 | ➖ | | Package | 126 | 0 | 59 | 0 | 185 | ✅ | | Fulfillment | 12 | 0 | 110 | 0 | 122 | ✅ | | Framework sanity | 0 | 0 | 7 | 0 | 7 | ➖ | | Secure | 20 | 0 | 40 | 0 | 60 | ✅ | | ModelOps | 0 | 0 | 5 | 0 | 5 | ➖ | | Systems | 19 | 0 | 0 | 0 | 19 | ✅ | | GitLab Metrics | 2 | 0 | 1 | 0 | 3 | ✅ | +------------------+--------+--------+---------+-------+-------+--------+ | Total | 2260 | 0 | 399 | 287 | 2659 | ❗ | +------------------+--------+--------+---------+-------+-------+--------+
- Resolved by Mayra Cabrera
Starting with 15.10, there is a new engineering process for patch releases so the
Pick into 15.11
label will not have any effect. For this merge request to be included in the next patch release for 15.11, this MR needs to be merged into the15-11-stable-ee
branch . I'm removing the Pick into 15.11 label.
removed Pick into 15.11 label
requested review from @DylanGriffith
FYI @jon_jenkins - Since this has already been through review on merge to master, I hope it's alright I asked @DylanGriffith to take a look and speed this along (Thanks Dylan!).
- Resolved by Dylan Griffith
My intent is to merge this change to 15.11-stable and then backport it to 15.11.0 and 15.11.1.
@jon_jenkins I'm not totally up to date on backports but I didn't think this was possible. I thought we could only backport to "minor versions" (like
15.11
) which results in a new patch like15.11.3
. So I don't think it makes sense to backport into a patch like15.11.10
?
- Resolved by Dylan Griffith
- Resolved by Dylan Griffith
I see lots of "allowed" failures like https://gitlab.com/gitlab-org/gitlab/-/jobs/4220691885 :
Error: command failed: rake aborted! StandardError: An error has occurred, this and all later migrations canceled: PG::UndefinedColumn: ERROR: column "user_email_lookup_limit" of relation "application_settings" does not exist LINE 1: UPDATE "application_settings" SET user_email_lookup_limit=se...
Is this a known issue failing elsewhere? I just want to double check since this changes a lot of migrations.
Database migrations (on the main database)
Migrations included in this change have been executed on gitlab.com data for testing purposes. For details, please see the migration testing pipeline (limited access).
Migration Type Total runtime Result DB size change No histogram available for visualization
Other information
No other migrations pending on GitLab.com
Clone details
Clone ID Clone Created At Clone Data Timestamp Expected Removal Time database-testing-1913434-9961464-main
2023-05-04T19:08:31Z 2023-05-04T13:47:16Z 2023-05-05 07:12:32 +0000 database-testing-1913434-9961464-ci
2023-05-04T19:08:31Z 2023-05-04T16:45:44Z 2023-05-05 07:12:32 +0000 Database migrations (on the ci database)
Migrations included in this change have been executed on gitlab.com data for testing purposes. For details, please see the migration testing pipeline (limited access).
Migration Type Total runtime Result DB size change No histogram available for visualization
Other information
No other migrations pending on GitLab.com
Clone details
Clone ID Clone Created At Clone Data Timestamp Expected Removal Time database-testing-1913434-9961464-main
2023-05-04T19:08:31Z 2023-05-04T13:47:16Z 2023-05-05 07:12:32 +0000 database-testing-1913434-9961464-ci
2023-05-04T19:08:31Z 2023-05-04T16:45:44Z 2023-05-05 07:12:32 +0000
Brought to you by gitlab-org/database-team/gitlab-com-database-testing. Epic
- A deleted user
added database-testing-automation label
@jon_jenkins Thee2e:package-and-test-ee
job has failed.- Pipeline: #856542417
-
package-and-test
pipeline: https://gitlab.com/gitlab-org/gitlab/-/jobs/4222979271
Ping your team's associated Software Engineer in Test (SET) to confirm the failures are unrelated to the merge request. If there's no SET assigned, ask for assistance on the
#quality
Slack channel.@twk3 and/or @sliaquat I wanted to have you weigh in here. From what I can see, it looks like all of the important jobs succeeded in the pipeline:run-all-e2e run for this MR. The "allowed failures" have to do with the fact that we're adding some migrations back in out of chronological order.
What do you think?
Edit: We got this failure message because a single job had what appeared to me to be a transient failure - I reran the job, it succeeded.
Edited by Jon Jenkins@jon_jenkins Yup. They are all green: https://gitlab.com/gitlab-org/gitlab/-/pipelines/857810750
Thanks for working on this.
Edited by Sanad Liaquat
requested review from @sabrams and removed review request for @samsam_kim
- Resolved by Dylan Griffith
@samsam_kim
, thanks for approving this merge request.This is the first time the merge request is approved. Please ensure the
e2e:package-and-test-ee
job has succeeded. If there is a failure, a Software Engineer in Test (SET) needs to confirm the failures are unrelated to the merge request. If there's no SET assigned to this team, ask for assistance on the#quality
Slack channel.
removed review request for @sabrams
enabled an automatic merge when the pipeline for 53f90aa5 succeeds
mentioned in commit 9678ef77
mentioned in merge request omnibus-gitlab!6850 (merged)
mentioned in merge request gitlab-org/charts/gitlab!3143 (merged)
changed milestone to %15.11
mentioned in issue #408768 (closed)
mentioned in issue #409331 (closed)
mentioned in issue gitlab-org/quality/pipeline-triage#197 (closed)
mentioned in merge request omnibus-gitlab!6856 (merged)
mentioned in issue omnibus-gitlab#7809 (moved)
added groupdatabase frameworks label and removed groupdatabase [DEPRECATED] label
added pipeline:mr-approved label