Skip to content

Move getter/setter methods to Ci::Metadatable module and apply feature flags

Problem

The code relies strictly on data to be written and read on specific tables. For example exit_code= always delegates to Ci::BuildMetadata. Since some data is moved to new destinations (e.g. intrinsic data moved to ci_builds and immutable processing data to ci_job_prototypes) we need to be able to use feature flags to read/write data to specific destination.

Proposal

Move getter/setter methods from Ci::Build and Ci::BuildMetadata to Metadatable module so we can switch the source based on the feature flag / state (options, yaml_variables, timeout, id_tokens, exit_code, etc.).

By leveraging Metadatable we would depend upon the abstraction rather than the specific table/model Ci::Build or Ci::BuildMetadata.

Checkout the PoC Draft: PoC: split build metadata into `job_prot... (!193948 - closed) to see what methods need to be moved and how. Methods like timeout_human_readable, timeout_value, exit_code, exit_code=, enable_debug_trace!, etc. needs to be moved.

The idea is to have:

  • getters will use a feature flag read_from_new_ci_destinations to determine whether to serve the value from the new ci_job_prototypes table or from the legacy ci_builds_metadata.
  • setters will write to 2 destinations:
    • either ci_builds for intrinsic data or ci_job_prototypes for immutable processing data.

    • ci_builds_metadata to maintain behavior backwards compatible and allowing us to make 2-way-door decisions. We will use a feature flag stop_writing_builds_metadata

      to control when to stop writing to this table.

Implementation

1. Introduce the feature flag read_from_new_ci_destinations.

  • Apply to options and yaml_variables.
  • Update all places that preload :metadata to also preload :job_definition, to avoid N+1 queries.

-> MR: Introduce FF `read_from_new_ci_destinations` an... (!199971 - merged)

2. Prepare for introduction of FF stop_writing_builds_metadata for job processing data.

  • Update job factories to write to both ci_builds_metadata and ci_job_definitions. Stop mutating options and yaml_variables in spec tests.
  • We'll need to update the degenerate! method since all factory-created job objects will have job_definition present, so we need to destroy it.
  • Update all factories/specs affected per !199971 (comment 2675309188):

@fabiopitino: @lma-git - in a discussion with @mbobin I recall we discussed this problem. We cannot mutate the config in ci_job_definitions because it would impact other jobs sharing the same job definition. E.g. 2 jobs A and B are initially created with the same options/definition. They will share the same definition ID. If later we mutate job A's options and update the relative job definition, it would mutate also the definition of job B.

IMO we should gradually remove all options= from the specs and replace them with factories or stubs like expect(job).to receive(:options).and_return(...).

-> MR: Update Rspec job factories to write to ci_job_d... (!202317 - merged)

3. Introduce the feature flag stop_writing_builds_metadata.

  • Apply the feature flag to an intrinsic data field to start.

-> MR: Introduce FF `stop_writing_builds_metadata` and... (!202462 - merged)

4. Address each field that needs to have its getter/setter added/moved/updated to Ci::Metadatable. Apply feature flags as necessary.

Field(s) Status MR / Notes
options, yaml_variables !199971 (merged), !203321 (merged)
scoped_user_id !203085 (merged)
timeout, timeout_source #551827 (closed), !202970 (merged)
exit_code !202815 (merged)
debug_trace_enabled !202462 (merged)
downstream_errors Now saved to ci_job_messages table, under the feature flag ci_new_downstream_errors_location. See !202194 (merged).
interruptible !203212 (merged)
secrets !202968 (merged)
id_tokens !202681 (merged)
Edited by Fabio Pitino