Make Fixed Email Notification GA (V2)
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 manyCi::Pipeline
s instead of associating the latest pipeline only. This is useful to simplifyMergeReqeust#all_pipelines
logic. - In theory,
Ci::Ref
always exists when pipeline is created. -
Ci::Ref
usesstate_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.
ci_refs
table between the old/new code running during the deploy?
Question: Is it safe to recreate 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
- Remove unused classes on initial Ci::Ref implementation
- Remove the feature flags
ci_pipeline_fixed_notifications
Related Issues
Does this MR meet the acceptance criteria?
Conformity
-
Changelog entry - [-] Documentation (if required)
-
Code review guidelines -
Merge request performance guidelines -
Style guides -
Database guides -
Separation of EE specific content
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