Make Fixed Email Notification GA (V2) (!28338) · Merge requests · GitLab.org / GitLab

What does this MR do?

The MR is to re-architect Ci::Ref related-logic in order to make #24309 (closed) GA in %13.0 .

The key changes are:

Ci::Ref supports Travis CI like notification statuses.
Ci::Ref has many Ci::Pipelines instead of associating the latest pipeline only. This is useful to simplify MergeReqeust#all_pipelines logic.
In theory, Ci::Ref always exists when pipeline is created.
Ci::Ref uses state_machine to handle the complex status transitions.
We persist source ref path to ci_refs table always. This makes this feature compatible with MR pipelines.
This resolves all of the concerns in !16951 (merged).

Closes #24309 (closed) #211331 (closed) #211336 (closed) #212278 (closed) #212282 (closed) #212281 (closed)

Feature Flags

This feature is guarded by the feature flag - ci_pipeline_fixed_notifications, which lets us easily rollback the change via chatops.

Question: Is it safe to recreate `ci_refs` table between the old/new code running during the deploy?

Tl;dr; Yes, it's safe. Because ci_refs is not currently used.

We introduced the table ci_refs in the previous release, however, we decided not to use this table because we spotted an architectual problem. After the initial MR was shipped and ran for a few days on production, we shipped ci_pipeline_fixed_notifications feature flag to make this feature disabled by default.

In fact, you see that a new record stopped being inserted into this table from a month ago.

gitlabhq_production=> select last_updated_by_pipeline_id from ci_refs ORDER BY id DESC LIMIT 1;
 last_updated_by_pipeline_id 
-----------------------------
                   131263172
(1 row)

gitlabhq_production=> select created_at from ci_pipelines where id = 131263172;                                   created_at         
----------------------------
 2020-03-31 02:29:05.075653
(1 row)

You can also see in https://log.gprd.gitlab.net/goto/e5784c66c23dc4840f0d4f723fc83d28, that PipelineUpdateCiRefStatusWorker is not being executed for a while, which is the only worker that reads/wrties the table. This worker is currently disabled by the feature flag ci_pipeline_fixed_notifications and its usage is removed in this MR.

Also, there are no needs to migrate existing data since the present data is going to be reproduced on the runtime (specifically pipeline.ci_ref&.update_status_by!(pipeline)).

Manual QA

- Case: When a pipeline fails on a branch
  Expectation: A `pipeline_failed_email` template is used for pipeline notification.
  Result: PASS

- Case: And When a pipeline succeeds on a branch
  Expectation: A `pipeline_fixed_email` template is used for pipeline notification.
  Result: PASS

- Case: And When a pipeline succeeds on a branch
  Expectation: A `pipeline_success_email` template is used for pipeline notification.
  Result: PASS

**The same test passed on Merge Request pipelines, which runs on a MR refs**

Follow-up Issues

Related Issues

!27977 (closed)

Does this MR meet the acceptance criteria?

Conformity

Availability and Testing

Review and add/update tests for this feature/bug. Consider all test levels. See the Test Planning Process.
[-] Tested in all supported browsers
[-] Informed Infrastructure department of a default or new setting change, if applicable per definition of done

Security

If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:

[-] Label as security and @ mention @gitlab-com/gl-security/appsec
[-] The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
[-] Security reports checked/validated by a reviewer from the AppSec team

Edited May 13, 2020 by 🙈 jacopo beschi 🙉

Make Fixed Email Notification GA (V2)