Commit 4624a148 authored by Fabio Pitino's avatar Fabio Pitino Committed by Grzegorz Bizon

Add pipeline metadata for creation errors/warnings

Store errors or warnings occurring during pipeline
creation in Ci::PipelineMetadata.
parent 29b33778
......@@ -51,6 +51,8 @@ module Ci
has_many :latest_builds, -> { latest }, foreign_key: :commit_id, inverse_of: :pipeline, class_name: 'Ci::Build'
has_many :downloadable_artifacts, -> { not_expired.downloadable }, through: :latest_builds, source: :job_artifacts
has_many :messages, class_name: 'Ci::PipelineMessage', inverse_of: :pipeline
# Merge requests for which the current pipeline is running against
# the merge request's latest commit.
has_many :merge_requests_as_head_pipeline, foreign_key: "head_pipeline_id", class_name: 'MergeRequest'
......@@ -626,6 +628,12 @@ module Ci
yaml_errors.present?
end
def add_error_message(content)
return unless Gitlab::Ci::Features.store_pipeline_messages?(project)
messages.error.build(content: content)
end
# Manually set the notes for a Ci::Pipeline
# There is no ActiveRecord relation between Ci::Pipeline and notes
# as they are related to a commit sha. This method helps importing
......@@ -948,7 +956,7 @@ module Ci
stages.find_by!(name: name)
end
def error_messages
def full_error_messages
errors ? errors.full_messages.to_sentence : ""
end
......
# frozen_string_literal: true
module Ci
class PipelineMessage < ApplicationRecord
extend Gitlab::Ci::Model
MAX_CONTENT_LENGTH = 10_000
belongs_to :pipeline
validates :content, presence: true
before_save :truncate_long_content
enum severity: { error: 0, warning: 1 }
private
def truncate_long_content
return if content.length <= MAX_CONTENT_LENGTH
self.content = content.truncate(MAX_CONTENT_LENGTH)
end
end
end
......@@ -77,7 +77,7 @@ module Ci
def execute!(*args, &block)
execute(*args, &block).tap do |pipeline|
unless pipeline.persisted?
raise CreateError, pipeline.error_messages
raise CreateError, pipeline.full_error_messages
end
end
end
......
---
title: Store pipeline creation errors and warnings into Ci::PipelineMessage
merge_request: 33762
author:
type: other
# frozen_string_literal: true
class CreateCiPipelineMessagesTable < ActiveRecord::Migration[6.0]
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
ERROR_SEVERITY = 0
MAX_CONTENT_LENGTH = 10_000
disable_ddl_transaction!
def up
with_lock_retries do
create_table :ci_pipeline_messages do |t|
t.integer :severity, null: false, default: ERROR_SEVERITY, limit: 2
t.references :pipeline, index: true, null: false, foreign_key: { to_table: :ci_pipelines, on_delete: :cascade }, type: :integer
t.text :content, null: false
end
end
add_text_limit :ci_pipeline_messages, :content, MAX_CONTENT_LENGTH
end
def down
drop_table :ci_pipeline_messages
end
end
......@@ -1276,6 +1276,23 @@ CREATE SEQUENCE public.ci_pipeline_chat_data_id_seq
ALTER SEQUENCE public.ci_pipeline_chat_data_id_seq OWNED BY public.ci_pipeline_chat_data.id;
CREATE TABLE public.ci_pipeline_messages (
id bigint NOT NULL,
severity smallint DEFAULT 0 NOT NULL,
pipeline_id integer NOT NULL,
content text NOT NULL,
CONSTRAINT check_58ca2981b2 CHECK ((char_length(content) <= 10000))
);
CREATE SEQUENCE public.ci_pipeline_messages_id_seq
START WITH 1
INCREMENT BY 1
NO MINVALUE
NO MAXVALUE
CACHE 1;
ALTER SEQUENCE public.ci_pipeline_messages_id_seq OWNED BY public.ci_pipeline_messages.id;
CREATE TABLE public.ci_pipeline_schedule_variables (
id integer NOT NULL,
key character varying NOT NULL,
......@@ -7631,6 +7648,8 @@ ALTER TABLE ONLY public.ci_job_variables ALTER COLUMN id SET DEFAULT nextval('pu
ALTER TABLE ONLY public.ci_pipeline_chat_data ALTER COLUMN id SET DEFAULT nextval('public.ci_pipeline_chat_data_id_seq'::regclass);
ALTER TABLE ONLY public.ci_pipeline_messages ALTER COLUMN id SET DEFAULT nextval('public.ci_pipeline_messages_id_seq'::regclass);
ALTER TABLE ONLY public.ci_pipeline_schedule_variables ALTER COLUMN id SET DEFAULT nextval('public.ci_pipeline_schedule_variables_id_seq'::regclass);
ALTER TABLE ONLY public.ci_pipeline_schedules ALTER COLUMN id SET DEFAULT nextval('public.ci_pipeline_schedules_id_seq'::regclass);
......@@ -8341,6 +8360,9 @@ ALTER TABLE ONLY public.ci_job_variables
ALTER TABLE ONLY public.ci_pipeline_chat_data
ADD CONSTRAINT ci_pipeline_chat_data_pkey PRIMARY KEY (id);
ALTER TABLE ONLY public.ci_pipeline_messages
ADD CONSTRAINT ci_pipeline_messages_pkey PRIMARY KEY (id);
ALTER TABLE ONLY public.ci_pipeline_schedule_variables
ADD CONSTRAINT ci_pipeline_schedule_variables_pkey PRIMARY KEY (id);
......@@ -9609,6 +9631,8 @@ CREATE INDEX index_ci_pipeline_chat_data_on_chat_name_id ON public.ci_pipeline_c
CREATE UNIQUE INDEX index_ci_pipeline_chat_data_on_pipeline_id ON public.ci_pipeline_chat_data USING btree (pipeline_id);
CREATE INDEX index_ci_pipeline_messages_on_pipeline_id ON public.ci_pipeline_messages USING btree (pipeline_id);
CREATE UNIQUE INDEX index_ci_pipeline_schedule_variables_on_schedule_id_and_key ON public.ci_pipeline_schedule_variables USING btree (pipeline_schedule_id, key);
CREATE INDEX index_ci_pipeline_schedules_on_next_run_at_and_active ON public.ci_pipeline_schedules USING btree (next_run_at, active);
......@@ -12586,6 +12610,9 @@ ALTER TABLE ONLY public.packages_conan_metadata
ALTER TABLE ONLY public.vulnerability_feedback
ADD CONSTRAINT fk_rails_8c77e5891a FOREIGN KEY (issue_id) REFERENCES public.issues(id) ON DELETE SET NULL;
ALTER TABLE ONLY public.ci_pipeline_messages
ADD CONSTRAINT fk_rails_8d3b04e3e1 FOREIGN KEY (pipeline_id) REFERENCES public.ci_pipelines(id) ON DELETE CASCADE;
ALTER TABLE ONLY public.approval_merge_request_rules_approved_approvers
ADD CONSTRAINT fk_rails_8dc94cff4d FOREIGN KEY (user_id) REFERENCES public.users(id) ON DELETE CASCADE;
......@@ -14153,6 +14180,7 @@ COPY "schema_migrations" (version) FROM STDIN;
20200619154528
20200622095419
20200622103836
20200622104923
20200622235737
20200623000148
20200623000320
......
......@@ -46,7 +46,7 @@ module MergeTrains
source_sha: merge_status[:source_id])
.execute(:merge_request_event, merge_request: merge_request)
return error(pipeline.error_messages) unless pipeline.persisted?
return error(pipeline.full_error_messages) unless pipeline.persisted?
success(pipeline: pipeline)
end
......
......@@ -45,6 +45,11 @@ module Gitlab
def self.release_generation_enabled?
::Feature.enabled?(:ci_release_generation)
end
# Remove in https://gitlab.com/gitlab-org/gitlab/-/issues/224199
def self.store_pipeline_messages?(project)
::Feature.enabled?(:ci_store_pipeline_messages, project, default_enabled: true)
end
end
end
end
......
......@@ -11,7 +11,12 @@ module Gitlab
pipeline.yaml_errors = message
end
pipeline.add_error_message(message)
pipeline.drop!(drop_reason) if drop_reason
# TODO: consider not to rely on AR errors directly as they can be
# polluted with other unrelated errors (e.g. state machine)
# https://gitlab.com/gitlab-org/gitlab/-/issues/220823
pipeline.errors.add(:base, message)
end
end
......
......@@ -228,6 +228,7 @@ ci_pipelines:
- latest_builds
- daily_report_results
- latest_builds_report_results
- messages
ci_refs:
- project
- ci_pipelines
......
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe Ci::PipelineMessage do
describe 'validations' do
subject { described_class.new(pipeline: pipeline, content: content) }
let(:pipeline) { create(:ci_pipeline) }
context 'when message content is longer than the limit' do
let(:content) { 'x' * (described_class::MAX_CONTENT_LENGTH + 1) }
it 'is truncated with ellipsis' do
subject.save!
expect(subject.content).to end_with('x...')
expect(subject.content.length).to eq(described_class::MAX_CONTENT_LENGTH)
end
end
context 'when message is not present' do
let(:content) { '' }
it 'returns an error' do
expect(subject.save).to be_falsey
expect(subject.errors[:content]).to be_present
end
end
context 'when message content is valid' do
let(:content) { 'valid message content' }
it 'is saved with default error severity' do
subject.save!
expect(subject.content).to eq(content)
expect(subject.severity).to eq('error')
expect(subject).to be_error
end
it 'is persist the defined severity' do
subject.severity = :warning
subject.save!
expect(subject.content).to eq(content)
expect(subject.severity).to eq('warning')
expect(subject).to be_warning
end
end
end
end
......@@ -2617,6 +2617,28 @@ RSpec.describe Ci::Pipeline, :mailer do
end
end
describe '#add_error_message' do
let(:pipeline) { build_stubbed(:ci_pipeline) }
it 'adds a new pipeline error message' do
pipeline.add_error_message('The error message')
expect(pipeline.messages.map(&:content)).to contain_exactly('The error message')
end
context 'when feature flag ci_store_pipeline_messages is disabled' do
before do
stub_feature_flags(ci_store_pipeline_messages: false)
end
it ' does not add pipeline error message' do
pipeline.add_error_message('The error message')
expect(pipeline.messages).to be_empty
end
end
end
describe '#has_yaml_errors?' do
context 'when yaml_errors is set' do
before do
......@@ -3181,8 +3203,8 @@ RSpec.describe Ci::Pipeline, :mailer do
end
end
describe '#error_messages' do
subject { pipeline.error_messages }
describe '#full_error_messages' do
subject { pipeline.full_error_messages }
before do
pipeline.valid?
......
......@@ -194,6 +194,7 @@ RSpec.describe Ci::CreatePipelineService do
expect(head_pipeline).to be_persisted
expect(head_pipeline.yaml_errors).to be_present
expect(head_pipeline.messages).to be_present
expect(merge_request.reload.head_pipeline).to eq head_pipeline
end
end
......@@ -1695,6 +1696,7 @@ RSpec.describe Ci::CreatePipelineService do
expect(pipeline).to be_persisted
expect(pipeline.builds).to be_empty
expect(pipeline.yaml_errors).to eq("test_a: needs 'build_a'")
expect(pipeline.messages.pluck(:content)).to contain_exactly("test_a: needs 'build_a'")
expect(pipeline.errors[:base]).to contain_exactly("test_a: needs 'build_a'")
end
end
......@@ -1706,6 +1708,7 @@ RSpec.describe Ci::CreatePipelineService do
expect(pipeline).not_to be_persisted
expect(pipeline.builds).to be_empty
expect(pipeline.yaml_errors).to be_nil
expect(pipeline.messages).not_to be_empty
expect(pipeline.errors[:base]).to contain_exactly("test_a: needs 'build_a'")
end
end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment