Geo: Add verification for MR diffs using SSF [RUN ALL RSPEC] [RUN AS-IF-FOSS]
What does this MR do?
This MR uses the self-service framework to add verification for external MR diffs.
The database changes were introduced in !60935 (merged) and this MR is dependent on it getting merged. Once that one is merged, I will re-target this one to master.
This MR specifically does the following:
-
Add MR diffs related metrics to docs -
Introduce FF geo_merge_request_diff_verification
-
Add tests based on the framework -
Use the framework's VerificationState
concern by overriding some methods so that the table used to update verification state ismerge_request_diff_details
and notmerge_request_diffs
which corresponds to the replicable model.
What's left to do?
- Merging of the related db changes.
The only failing test is on geo:db:rollback and I am discussing it with @alexives
in the MR that introduces it.
This current MR is dependent on it, hence the failing test.
- Lower priority: manual testing of behaviour of verification of remotely stored diffs
This MR is already tested for external diffs stored locally. Diffs stored remotely are not expected to be verified due to SSF's current limitation. But it needs to be manually verified if the status bars look correct with remote diffs.
That is a bit more evolved, having to setup this current branch on GCP. I have run into some problems with the installation.
But since this is behind a feature flag it should be ok to merge it with the current manual testing of locally stored diffs. It is much easier to deploy master (once this is merged in) to an instance.
Future work:
Resolves: #323285 (closed)
Testing for locally stored diffs
UI
Sync and verification progress on secondary
-
Already existing MR diffs are verified when feature flag is turned on -
New MR diff is replicated and verified when a new MR is created
Metrics
- API response
curl --header "PRIVATE-TOKEN: <>" "http://127.0.0.1:3000/api/v4/geo_nodes/status"
{
"geo_node_id": 1,
...
"merge_request_diffs_count": 0,
"merge_request_diffs_checksum_total_count": null,
"merge_request_diffs_checksummed_count": null,
"merge_request_diffs_checksum_failed_count": null,
"merge_request_diffs_synced_count": 0,
"merge_request_diffs_failed_count": 0,
"merge_request_diffs_registry_count": 0,
"merge_request_diffs_verification_total_count": 0,
"merge_request_diffs_verified_count": 0,
"merge_request_diffs_verification_failed_count": 0,
- Status rake task:
rake geo:status
Name: gdk-geo
URL: http://127.0.0.1:3001
-----------------------------------------------------
GitLab Version: 14.0.0-pre
Geo Role: Secondary
Health Status: Healthy
...
Merge Request Diffs: 31/31 (100%)
...
Merge Request Diffs Verified: 31/31 (100%)
...
Sync Settings: Full
Database replication lag: 0 seconds
Last event ID seen from primary: 354 (about 1 hour ago)
Last event ID processed by cursor: 354 (about 1 hour ago)
Last status report was: 1 minute ago
- Prometheus
"enablement": {
"geo_secondary_web_oauth_users": 1,
"geo_node_usage": [
{
...
“merge_request_diffs_count”: 0,
“merge_request_diffs_checksum_total_count”: null,
“merge_request_diffs_checksummed_count”: null,
“merge_request_diffs_checksum_failed_count”: null,
“merge_request_diffs_synced_count”: 0,
“merge_request_diffs_failed_count”: 0,
“merge_request_diffs_registry_count”: 0,
“merge_request_diffs_verification_total_count”: 0,
“merge_request_diffs_verified_count”: 0,
“merge_request_diffs_verification_failed_count”: 0,
}
]
}
Does this MR meet the acceptance criteria?
Conformity
-
I have included a changelog entry, or it's not needed. (Does this MR need a changelog?) -
I have added/updated documentation, or it's not needed. (Is documentation required?) -
I have properly separated EE content from FOSS, or this MR is FOSS only. (Where should EE code go?) - [-] I have added information for database reviewers in the MR description, or it's not needed. (Does this MR have database related changes?)
-
I have self-reviewed this MR per code review guidelines. -
This MR does not harm performance, or I have asked a reviewer to help assess the performance impact. (Merge request performance guidelines) -
I have followed the style guides.
Availability and Testing
Things to do:
-
Test metrics manually -
Test replication and verification of externally stored MRs -
locally stored -
remotely stored (verification is not expected to work)
-
Steps to setup local testing:
- Enable external MR diffs in config/gitlab.yml or check if MRs with external diffs already exist in the db
- e.g.
MergeRequestDiff.where(external_diff_store: 1).count
- The default gdk setup comes with about 70 MRs with external diffs
- e.g.
- Enable feature flag with
Feature.enable(:geo_merge_request_diff_verification)
- Wait a few minutes for replication and verification to catch-up and check geo nodes status page
- MR diffs checksum progress should be green
-
MergeRequestDiffDetail.count
should tell you how many MRs have verification states
-
I have added/updated tests following the Testing Guide, or it's not needed. (Consider all test levels. See the Test Planning Process.) - [-] I have tested this MR in all supported browsers, or it's not needed.
- [-] I have informed the Infrastructure department of a default or new setting change per definition of done, or it's not needed.