Skip to content

Geo: Add verification for MR diffs using SSF [RUN ALL RSPEC] [RUN AS-IF-FOSS]

Aakriti Gupta requested to merge ag-backend-changes-mr-diffs-verification into master

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 is merge_request_diff_details and not merge_request_diffs which corresponds to the replicable model.

What's left to do?

  1. 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.

  1. 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:

Update issue template for implementing replication and verification with the SSF with instructions to override methods when verification state information is stored separately from the model that is being replicated.

Resolves: #323285 (closed)

Testing for locally stored diffs

UI

Checksum progress on primary Screenshot_2021-06-08_at_18.44.40

Sync and verification progress on secondary Screenshot_2021-06-08_at_19.12.06

  • 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

  1. 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,
  1. 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
  1. 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

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
  • 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
Edited by Aakriti Gupta

Merge request reports