Skip to content

Commit eb74c6c

Browse files
committed
Call after_update_* from edit_with_params
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.
1 parent bd821db commit eb74c6c

File tree

2 files changed

+164
-3
lines changed

2 files changed

+164
-3
lines changed

app/models/ext_management_system.rb

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -175,18 +175,30 @@ def self.create_from_params(params, endpoints, authentications)
175175

176176
def edit_with_params(params, endpoints, authentications)
177177
tap do |ems|
178+
endpoints_changed = false
179+
authentications_changed = false
180+
178181
transaction do
179182
# Remove endpoints/attributes that are not arriving in the arguments above
180-
ems.endpoints.where.not(:role => nil).where.not(:role => endpoints.map { |ep| ep['role'] }).delete_all
181-
ems.authentications.where.not(:authtype => nil).where.not(:authtype => authentications.map { |au| au['authtype'] }).delete_all
183+
endpoints_to_delete = ems.endpoints.where.not(:role => nil).where.not(:role => endpoints.map { |ep| ep['role'] })
184+
authentications_to_delete = ems.authentications.where.not(:authtype => nil).where.not(:authtype => authentications.map { |au| au['authtype'] })
185+
186+
endpoints_changed ||= endpoints_to_delete.delete_all > 0
187+
authentications_changed ||= authentications_to_delete.delete_all > 0
182188

183189
ems.assign_attributes(params)
184-
ems.endpoints = endpoints.map(&method(:assign_nested_endpoint))
190+
ems.endpoints = endpoints.map(&method(:assign_nested_endpoint))
185191
ems.authentications = authentications.map(&method(:assign_nested_authentication))
186192

193+
endpoints_changed ||= ems.endpoints.any?(&:changed?)
194+
authentications_changed ||= ems.authentications.any?(&:changed?)
195+
187196
ems.provider.save! if ems.provider.present? && ems.provider.changed?
188197
ems.save!
189198
end
199+
200+
after_update_endpoints if endpoints_changed
201+
after_update_authentication if authentications_changed
190202
end
191203
end
192204

@@ -867,6 +879,10 @@ def after_update_authentication
867879
stop_event_monitor_queue_on_credential_change
868880
end
869881

882+
def after_update_endpoints
883+
stop_event_monitor_queue_on_change
884+
end
885+
870886
###################################
871887
# Event Monitor
872888
###################################

spec/models/ext_management_system_spec.rb

Lines changed: 145 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -843,6 +843,151 @@ def deliver_queue_message(queue_message = MiqQueue.order(:id).first)
843843
end
844844
end
845845

846+
describe "#edit_with_params" do
847+
let(:zone) { EvmSpecHelper.local_miq_server.zone }
848+
let(:ems) do
849+
FactoryBot.create(:ext_management_system, :with_authentication, :zone => zone).tap do |ems|
850+
# assign_nested_authentication automatically sets the name to "#{self.class.name} #{name}"
851+
# set it here to prevent spurious authentication record changes
852+
ems.default_authentication.update!(:name => "#{ems.class.name} #{ems.name}")
853+
end
854+
end
855+
856+
let(:params) { {"name" => ems.name, "zone" => ems.zone} }
857+
let(:endpoints) { [default_endpoint] }
858+
let(:authentications) { [default_authentication] }
859+
860+
let(:default_endpoint) { {"role" => "default", "hostname" => ems.default_endpoint.hostname, "ipaddress" => ems.default_endpoint.ipaddress} }
861+
let(:default_authentication) { {"authtype" => "default", "userid" => ems.default_authentication.userid, "password" => ems.default_authentication.password} }
862+
863+
context "with no changes" do
864+
it "doesn't call endpoints or authentications changed callbaks" do
865+
expect(ems).not_to receive(:after_update_endpoints)
866+
expect(ems).not_to receive(:after_update_authentication)
867+
868+
ems.edit_with_params(params, endpoints, authentications)
869+
end
870+
end
871+
872+
context "changing an ext_management_system record attribute" do
873+
let(:params) { {:name => "new-name", :zone => ems.zone} }
874+
875+
it "changes the name" do
876+
ems.edit_with_params(params, endpoints, authentications)
877+
expect(ems.reload.name).to eq("new-name")
878+
end
879+
end
880+
881+
context "adding an endpoint" do
882+
let(:endpoints) { [default_endpoint, {"role" => "metrics", "hostname" => "metrics"}] }
883+
884+
it "creates the new endpoint" do
885+
ems.edit_with_params(params, endpoints, authentications)
886+
887+
ems.reload
888+
889+
expect(ems.endpoints.pluck(:role)).to match_array(["default", "metrics"])
890+
end
891+
892+
it "calls after_update_endpoints" do
893+
expect(ems).to receive(:after_update_endpoints)
894+
expect(ems).not_to receive(:after_update_authentication)
895+
896+
ems.edit_with_params(params, endpoints, authentications)
897+
end
898+
end
899+
900+
context "deleting an endpoint" do
901+
before { ems.endpoints.create!(:role => "metrics", :hostname => "metrics") }
902+
903+
it "deletes the unused endpoint" do
904+
ems.edit_with_params(params, endpoints, authentications)
905+
906+
ems.reload
907+
expect(ems.endpoints.pluck(:role)).to match_array(["default"])
908+
end
909+
910+
it "calls after_update_endpoints" do
911+
expect(ems).to receive(:after_update_endpoints)
912+
expect(ems).not_to receive(:after_update_authentication)
913+
914+
ems.edit_with_params(params, endpoints, authentications)
915+
end
916+
end
917+
918+
context "updating an endpoint" do
919+
let(:default_endpoint) { {"role" => "default", "hostname" => "new-hostname", "ipaddress" => ems.default_endpoint.ipaddress} }
920+
921+
it "updates the default hostname" do
922+
ems.edit_with_params(params, endpoints, authentications)
923+
924+
ems.reload
925+
expect(ems.default_endpoint.hostname).to eq("new-hostname")
926+
end
927+
928+
it "calls after_update_endpoints" do
929+
expect(ems).to receive(:after_update_endpoints)
930+
expect(ems).not_to receive(:after_update_authentication)
931+
932+
ems.edit_with_params(params, endpoints, authentications)
933+
end
934+
end
935+
936+
context "adding an authentication" do
937+
let(:authentications) { [default_authentication, {:authtype => "metrics", :auth_key => "secret"}] }
938+
939+
it "creates the authentication" do
940+
ems.edit_with_params(params, endpoints, authentications)
941+
942+
ems.reload
943+
expect(ems.authentications.pluck(:authtype)).to match_array(["default", "metrics"])
944+
end
945+
946+
it "calls after_update_authentication" do
947+
expect(ems).to receive(:after_update_authentication)
948+
expect(ems).not_to receive(:after_update_endpoints)
949+
950+
ems.edit_with_params(params, endpoints, authentications)
951+
end
952+
end
953+
954+
context "deleting an authentication" do
955+
before { ems.authentications.create!(:authtype => "metrics", :auth_key => "secret") }
956+
957+
it "deletes the authentication" do
958+
ems.edit_with_params(params, endpoints, authentications)
959+
960+
ems.reload
961+
expect(ems.authentications.pluck(:authtype)).to match_array(["default"])
962+
end
963+
964+
it "calls after_update_authentication" do
965+
expect(ems).to receive(:after_update_authentication)
966+
expect(ems).not_to receive(:after_update_endpoints)
967+
968+
ems.edit_with_params(params, endpoints, authentications)
969+
end
970+
end
971+
972+
context "updating an authentication" do
973+
let(:default_authentication) { {"authtype" => "default", "userid" => ems.default_authentication.userid, "password" => "more-secret"} }
974+
975+
it "updates the password" do
976+
ems.edit_with_params(params, endpoints, authentications)
977+
978+
ems.reload
979+
expect(ems.default_authentication.password).to eq("more-secret")
980+
end
981+
982+
it "calls after_update_authentication" do
983+
expect(ems).to receive(:after_update_authentication)
984+
expect(ems).not_to receive(:after_update_endpoints)
985+
986+
ems.edit_with_params(params, endpoints, authentications)
987+
end
988+
end
989+
end
990+
846991
context "virtual column :supports_block_storage (direct supports)" do
847992
it "returns false if block storage is not supported" do
848993
ems = FactoryBot.create(:ext_management_system)

0 commit comments

Comments
 (0)