Skip to content

Refactor Rollback validation

Robert Speicher requested to merge rs-rollback-refactor into master

What does this MR do?

This MR refactors each of the three major components of our rollback validation: Validator (now CompareService), Comparison, and Presenter.

This allows us to pull deployments from Omnibus, perform the comparison of those versions in gitlab-rails, and now show the package names in the incident issue body.

This MR depends on !1579 (merged).

8cab1b5c - Refactor Rollback::Validator into CompareService

This class no longer does the actual compare. It will ensure that the given current and target arguments are normalized into AutoDeploy::Version instances -- either from package strings or environment Deployments -- which then get passed to Compare, where the actual comparison happens.

bac6fc96 - Refactor Rollback::Comparison

  • Performs the comparison itself, rather than receiving an API response.
  • Receives the current and target versions so that it can be the sole representation used by Presenter.
  • Removes the new_ prefix from the migration-related attributes for brevity. It should be clear enough that these are migrations in the comparison, so the "new" is implied.
  • Adds convenience methods for delegating specific attributes to the current and target attributes.

3f157c9e - Refactor Rollback::Presenter

The Validator instance this class used to take was basically just delegating everything to the comparison attribute anyway. Its only advantage was that it dropped new_ prefixes from the migration-related methods for brevity.

71c7196a - Refactor IncidentRollback check to use new Compare

CompareService now returns a Compare object, which has all the information we need for the presenter. Namely, the packages (which we display instead of the Rails SHAs), the comparison link, and the health.

We now always use the Presenter, even when things are good, in order to show current/target and comparison links.

Author Check-list

  • [-] Has documentation been updated?
Edited by Alessio Caiazza

Merge request reports