Skip to content

Commit

Permalink
Call after_update_* from edit_with_params
Browse files Browse the repository at this point in the history
When changes are made to the endpoints or authentications workers which
run with an open connection like event catchers and streaming refresh
workers have to be restarted.

This was done via `after_update_authentication` called from
`update_authentication`.  The issue is that this method is no longer
called when providers are updated via the API with DDF parameters.
  • Loading branch information
agrare committed Dec 6, 2022
1 parent bd821db commit 0733f89
Show file tree
Hide file tree
Showing 2 changed files with 164 additions and 3 deletions.
22 changes: 19 additions & 3 deletions app/models/ext_management_system.rb
Original file line number Diff line number Diff line change
Expand Up @@ -175,18 +175,30 @@ def self.create_from_params(params, endpoints, authentications)

def edit_with_params(params, endpoints, authentications)
tap do |ems|
endpoints_changed = false
authentications_changed = false

transaction do
# Remove endpoints/attributes that are not arriving in the arguments above
ems.endpoints.where.not(:role => nil).where.not(:role => endpoints.map { |ep| ep['role'] }).delete_all
ems.authentications.where.not(:authtype => nil).where.not(:authtype => authentications.map { |au| au['authtype'] }).delete_all
endpoints_to_delete = ems.endpoints.where.not(:role => nil).where.not(:role => endpoints.map { |ep| ep['role'] })
authentications_to_delete = ems.authentications.where.not(:authtype => nil).where.not(:authtype => authentications.map { |au| au['authtype'] })

endpoints_changed ||= endpoints_to_delete.delete_all > 0
authentications_changed ||= authentications_to_delete.delete_all > 0

ems.assign_attributes(params)
ems.endpoints = endpoints.map(&method(:assign_nested_endpoint))
ems.endpoints = endpoints.map(&method(:assign_nested_endpoint))
ems.authentications = authentications.map(&method(:assign_nested_authentication))

endpoints_changed ||= ems.endpoints.any?(&:changed?)
authentications_changed ||= ems.authentications.any?(&:changed?)

ems.provider.save! if ems.provider.present? && ems.provider.changed?
ems.save!
end

after_update_endpoints if endpoints_changed
after_update_authentication if authentications_changed
end
end

Expand Down Expand Up @@ -867,6 +879,10 @@ def after_update_authentication
stop_event_monitor_queue_on_credential_change
end

def after_update_endpoints
stop_event_monitor_queue_on_credential_change
end

###################################
# Event Monitor
###################################
Expand Down
145 changes: 145 additions & 0 deletions spec/models/ext_management_system_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -843,6 +843,151 @@ def deliver_queue_message(queue_message = MiqQueue.order(:id).first)
end
end

describe "#edit_with_params" do
let(:zone) { EvmSpecHelper.local_miq_server.zone }
let(:ems) do
FactoryBot.create(:ext_management_system, :with_authentication, :zone => zone).tap do |ems|
# assign_nested_authentication automatically sets the name to "#{self.class.name} #{name}"
# set it here to prevent spurious authentication record changes
ems.default_authentication.update!(:name => "#{ems.class.name} #{ems.name}")
end
end

let(:params) { {"name" => ems.name, "zone" => ems.zone} }
let(:endpoints) { [default_endpoint] }
let(:authentications) { [default_authentication] }

let(:default_endpoint) { {"role" => "default", "hostname" => ems.default_endpoint.hostname, "ipaddress" => ems.default_endpoint.ipaddress} }
let(:default_authentication) { {"authtype" => "default", "userid" => ems.default_authentication.userid, "password" => ems.default_authentication.password} }

context "with no changes" do
it "doesn't call endpoints or authentications changed callbaks" do
expect(ems).not_to receive(:after_update_endpoints)
expect(ems).not_to receive(:after_update_authentication)

ems.edit_with_params(params, endpoints, authentications)
end
end

context "changing an ext_management_system record attribute" do
let(:params) { {:name => "new-name", :zone => ems.zone} }

it "changes the name" do
ems.edit_with_params(params, endpoints, authentications)
expect(ems.reload.name).to eq("new-name")
end
end

context "adding an endpoint" do
let(:endpoints) { [default_endpoint, {"role" => "metrics", "hostname" => "metrics"}] }

it "creates the new endpoint" do
ems.edit_with_params(params, endpoints, authentications)

ems.reload

expect(ems.endpoints.pluck(:role)).to match_array(["default", "metrics"])
end

it "calls after_update_endpoints" do
expect(ems).to receive(:after_update_endpoints)
expect(ems).not_to receive(:after_update_authentication)

ems.edit_with_params(params, endpoints, authentications)
end
end

context "deleting an endpoint" do
before { ems.endpoints.create!(:role => "metrics", :hostname => "metrics") }

it "deletes the unused endpoint" do
ems.edit_with_params(params, endpoints, authentications)

ems.reload
expect(ems.endpoints.pluck(:role)).to match_array(["default"])
end

it "calls after_update_endpoints" do
expect(ems).to receive(:after_update_endpoints)
expect(ems).not_to receive(:after_update_authentication)

ems.edit_with_params(params, endpoints, authentications)
end
end

context "updating an endpoint" do
let(:default_endpoint) { {"role" => "default", "hostname" => "new-hostname", "ipaddress" => ems.default_endpoint.ipaddress} }

it "updates the default hostname" do
ems.edit_with_params(params, endpoints, authentications)

ems.reload
expect(ems.default_endpoint.hostname).to eq("new-hostname")
end

it "calls after_update_endpoints" do
expect(ems).to receive(:after_update_endpoints)
expect(ems).not_to receive(:after_update_authentication)

ems.edit_with_params(params, endpoints, authentications)
end
end

context "adding an authentication" do
let(:authentications) { [default_authentication, {:authtype => "metrics", :auth_key => "secret"}] }

it "creates the authentication" do
ems.edit_with_params(params, endpoints, authentications)

ems.reload
expect(ems.authentications.pluck(:authtype)).to match_array(["default", "metrics"])
end

it "calls after_update_authentication" do
expect(ems).to receive(:after_update_authentication)
expect(ems).not_to receive(:after_update_endpoints)

ems.edit_with_params(params, endpoints, authentications)
end
end

context "deleting an authentication" do
before { ems.authentications.create!(:authtype => "metrics", :auth_key => "secret") }

it "deletes the authentication" do
ems.edit_with_params(params, endpoints, authentications)

ems.reload
expect(ems.authentications.pluck(:authtype)).to match_array(["default"])
end

it "calls after_update_authentication" do
expect(ems).to receive(:after_update_authentication)
expect(ems).not_to receive(:after_update_endpoints)

ems.edit_with_params(params, endpoints, authentications)
end
end

context "updating an authentication" do
let(:default_authentication) { {"authtype" => "default", "userid" => ems.default_authentication.userid, "password" => "more-secret"} }

it "updates the password" do
ems.edit_with_params(params, endpoints, authentications)

ems.reload
expect(ems.default_authentication.password).to eq("more-secret")
end

it "calls after_update_authentication" do
expect(ems).to receive(:after_update_authentication)
expect(ems).not_to receive(:after_update_endpoints)

ems.edit_with_params(params, endpoints, authentications)
end
end
end

context "virtual column :supports_block_storage (direct supports)" do
it "returns false if block storage is not supported" do
ems = FactoryBot.create(:ext_management_system)
Expand Down

0 comments on commit 0733f89

Please sign in to comment.