Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[EmbeddedAnsible] Better handle ConfigurationScriptSource status/last_updated_on/last_update_error #19061

Merged
merged 5 commits into from
Jul 29, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -24,16 +24,23 @@ def self.raw_create_in_provider(manager, params)
params.delete(:scm_type) if params[:scm_type].blank?
params.delete(:scm_branch) if params[:scm_branch].blank?

transaction { create!(params.merge(:manager => manager)).tap(&:sync) }
transaction { create!(params.merge(:manager => manager, :status => "new")) }
end

def self.create_in_provider(manager_id, params)
super.tap(&:sync_and_notify)
end

def raw_update_in_provider(params)
transaction do
update_attributes!(params.except(:task_id, :miq_task_id))
sync
end
end

def update_in_provider(params)
super.tap(&:sync_and_notify)
end

def raw_delete_in_provider
destroy!
end
Expand All @@ -48,6 +55,7 @@ def git_repository
end

def sync
update_attributes!(:status => "running")
transaction do
current = configuration_script_payloads.index_by(&:name)

Expand All @@ -60,7 +68,18 @@ def sync

configuration_script_payloads.reload
end
true
update_attributes!(:status => "successful",
:last_updated_on => Time.zone.now,
:last_update_error => nil)
rescue => error
update_attributes!(:status => "error",
:last_updated_on => Time.zone.now,
:last_update_error => format_sync_error(error))
raise error
end

def sync_and_notify
notify("syncing") { sync }
end

def sync_queue(auth_user = nil)
Expand All @@ -76,4 +95,12 @@ def checkout_git_repository(target_directory)
git_repository.update_repo
git_repository.checkout(scm_branch, target_directory)
end

ERROR_MAX_SIZE = 50.kilobytes
def format_sync_error(error)
result = error.message.dup
result << "\n\n"
result << error.backtrace.join("\n")
result.mb_chars.limit(ERROR_MAX_SIZE)
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -82,16 +82,38 @@ def files_in_repository(git_repo_dir)
context "with valid params" do
it "creates a record and initializes a git repo" do
expect(Notification).to receive(:create!).with(notify_creation_args)
expect(Notification).to receive(:create!).with(notification_args("syncing", {}))

result = described_class.create_in_provider(manager.id, params)

expect(result).to be_an(described_class)
expect(result.scm_type).to eq("git")
expect(result.scm_branch).to eq("master")
expect(result.status).to eq("successful")
expect(result.last_updated_on).to be_an(Time)
expect(result.last_update_error).to be_nil

git_repo_dir = repo_dir.join(result.git_repository.id.to_s)
expect(files_in_repository(git_repo_dir)).to eq ["hello_world.yaml"]
end

# NOTE: Second `.notify` stub below prevents `.sync` from getting fired
it "sets the status to 'new' on create" do
expect(Notification).to receive(:create!).with(notify_creation_args)
expect(described_class).to receive(:notify).with(any_args).and_call_original
expect(described_class).to receive(:notify).with("syncing", any_args).and_return(true)

result = described_class.create_in_provider(manager.id, params)

expect(result).to be_an(described_class)
expect(result.scm_type).to eq("git")
expect(result.scm_branch).to eq("master")
expect(result.status).to eq("new")
expect(result.last_updated_on).to be_nil
expect(result.last_update_error).to be_nil

expect(repos).to be_empty
end
end

context "with invalid params" do
Expand All @@ -109,6 +131,49 @@ def files_in_repository(git_repo_dir)
expect(repos).to be_empty
end
end

context "when there is a network error fetching the repo" do
before do
sync_notification_args = notification_args("syncing", {})
sync_notification_args[:type] = :tower_op_failure

expect(Notification).to receive(:create!).with(notify_creation_args)
expect(Notification).to receive(:create!).with(sync_notification_args)
expect(GitRepository).to receive(:create!).and_raise(::Rugged::NetworkError)

expect do
described_class.create_in_provider(manager.id, params)
end.to raise_error(::Rugged::NetworkError)
end

it "sets the status to 'error' if syncing has a network error" do
result = described_class.last

expect(result).to be_an(described_class)
expect(result.scm_type).to eq("git")
expect(result.scm_branch).to eq("master")
expect(result.status).to eq("error")
expect(result.last_updated_on).to be_an(Time)
expect(result.last_update_error).to start_with("Rugged::NetworkError")

expect(repos).to be_empty
end

it "clears last_update_error on re-sync" do
result = described_class.last

expect(result.status).to eq("error")
expect(result.last_updated_on).to be_an(Time)
expect(result.last_update_error).to start_with("Rugged::NetworkError")

expect(GitRepository).to receive(:create!).and_call_original

result.sync

expect(result.status).to eq("successful")
expect(result.last_update_error).to be_nil
end
end
end

describe ".create_in_provider_queue" do
Expand Down Expand Up @@ -136,6 +201,7 @@ def files_in_repository(git_repo_dir)
record = build_record

expect(Notification).to receive(:create!).with(notify_update_args)
expect(Notification).to receive(:create!).with(notification_args("syncing", {}))

result = record.update_in_provider update_params

Expand All @@ -161,6 +227,50 @@ def files_in_repository(git_repo_dir)
end.to raise_error(ActiveRecord::RecordInvalid)
end
end

context "when there is a network error fetching the repo" do
before do
record = build_record

sync_notification_args = notification_args("syncing", {})
sync_notification_args[:type] = :tower_op_failure

expect(Notification).to receive(:create!).with(notify_update_args)
expect(Notification).to receive(:create!).with(sync_notification_args)
expect(record.git_repository).to receive(:update_repo).and_raise(::Rugged::NetworkError)

expect do
# described_class.last.update_in_provider update_params
record.update_in_provider update_params
end.to raise_error(::Rugged::NetworkError)
end

it "sets the status to 'error' if syncing has a network error" do
result = described_class.last

expect(result).to be_an(described_class)
expect(result.scm_type).to eq("git")
expect(result.scm_branch).to eq("other_branch")
expect(result.status).to eq("error")
expect(result.last_updated_on).to be_an(Time)
expect(result.last_update_error).to start_with("Rugged::NetworkError")
end

it "clears last_update_error on re-sync" do
result = described_class.last

expect(result.status).to eq("error")
expect(result.last_updated_on).to be_an(Time)
expect(result.last_update_error).to start_with("Rugged::NetworkError")

expect(result.git_repository).to receive(:update_repo).and_call_original

result.sync

expect(result.status).to eq("successful")
expect(result.last_update_error).to be_nil
end
end
end

describe "#update_in_provider_queue" do
Expand Down Expand Up @@ -216,7 +326,7 @@ def files_in_repository(git_repo_dir)
end

def build_record
expect(Notification).to receive(:create!).with(any_args)
expect(Notification).to receive(:create!).with(any_args).twice
described_class.create_in_provider manager.id, params
end

Expand Down