From 9e1366a772421b2a64fd2c928412639bf36140ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miha=20Ple=C5=A1ko?= Date: Mon, 12 Aug 2019 08:55:26 +0200 Subject: [PATCH] Utility function to update PxeMenu list MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With this commit we introduce a utility method on PxeServer model to handle PxeMenus list update. Until now, miq-api was always recreating all menus upon server update, but often we can actually just preserve exisitng menus. This method calculates what menus to add, what to preserve and what to remove, and then executes it. Fixes https://github.com/ManageIQ/manageiq/issues/18940 Signed-off-by: Miha Pleško --- app/models/pxe_server.rb | 10 +++++ spec/models/pxe_server_spec.rb | 77 ++++++++++++++++++++++++++++++++++ 2 files changed, 87 insertions(+) diff --git a/app/models/pxe_server.rb b/app/models/pxe_server.rb index 53fb6a28bf2..750f3a8085c 100644 --- a/app/models/pxe_server.rb +++ b/app/models/pxe_server.rb @@ -198,6 +198,16 @@ def delete_provisioning_files(pxe_image, mac_address, windows_image = nil, custo _log.info("#{log_message}...Complete") end + def ensure_menu_list(menu_list) + current_menus = pxe_menus.map(&:file_name) + to_destroy = current_menus - menu_list + to_create = menu_list - current_menus + transaction do + pxe_menus.where(:file_name => to_destroy).destroy_all + to_create.each { |menu| pxe_menus << PxeMenu.create!(:file_name => menu) } + end + end + def self.display_name(number = 1) n_('PXE Server', 'PXE Servers', number) end diff --git a/spec/models/pxe_server_spec.rb b/spec/models/pxe_server_spec.rb index 399f613b7d7..cd33857665a 100644 --- a/spec/models/pxe_server_spec.rb +++ b/spec/models/pxe_server_spec.rb @@ -4,6 +4,8 @@ @pxe_server = FactoryBot.create(:pxe_server) end + subject { FactoryBot.create(:pxe_server) } + context "#sync_images_queue" do it "should create a queue entry with the correct parameters" do msg = @pxe_server.sync_images_queue @@ -310,4 +312,79 @@ def file_write(file, contents) expect(@pxe_server.discovered_pxe_images).to eq([@discovered_image]) end end + + describe '#ensure_menu_list' do + context 'when creating server' do + subject { FactoryBot.build(:pxe_server) } + it 'existing menus remain' do + subject.ensure_menu_list(%w[new]) + subject.save! + expect(PxeMenu.count).to eq(1) + expect(subject.pxe_menus.count).to eq(1) + end + end + + context 'when updating server' do + let!(:pxe_menu_1) { FactoryBot.create(:pxe_menu, :pxe_server => subject, :file_name => 'existing1') } + let!(:pxe_image_1) { FactoryBot.create(:pxe_image, :pxe_server => subject, :pxe_menu => pxe_menu_1) } + let!(:pxe_menu_2) { FactoryBot.create(:pxe_menu, :pxe_server => subject, :file_name => 'existing2') } + let!(:pxe_image_2) { FactoryBot.create(:pxe_image, :pxe_server => subject, :pxe_menu => pxe_menu_2) } + + context 'when nothing changed' do + let(:menus) { %w[existing1 existing2] } + it 'all menus remain' do + subject.ensure_menu_list(menus) + expect(PxeMenu.count).to eq(2) + expect(PxeImage.count).to eq(2) + expect(subject.pxe_menus.find_by(:id => pxe_menu_1.id)).to eq(pxe_menu_1) + expect(subject.pxe_menus.find_by(:id => pxe_menu_2.id)).to eq(pxe_menu_2) + end + end + + context 'when new menus are added' do + let(:menus) { %w[existing1 existing2 new] } + it 'existing menus remain' do + subject.ensure_menu_list(menus) + expect(PxeMenu.count).to eq(3) + expect(PxeImage.count).to eq(2) + expect(subject.pxe_menus.find_by(:id => pxe_menu_1.id)).to eq(pxe_menu_1) + expect(subject.pxe_menus.find_by(:id => pxe_menu_2.id)).to eq(pxe_menu_2) + expect(subject.pxe_menus.find_by(:file_name => 'new')).not_to be_nil + end + end + + context 'when menus are deleted' do + let(:menus) { %w[existing1] } + it 'menus are destroyed' do + subject.ensure_menu_list(menus) + expect(PxeMenu.count).to eq(1) + expect(PxeImage.count).to eq(1) + expect(subject.pxe_menus.find_by(:id => pxe_menu_1.id)).to eq(pxe_menu_1) + expect(subject.pxe_menus.find_by(:id => pxe_menu_2.id)).to be_nil + end + end + + context 'when different menus are specified' do + let(:menus) { %w[new] } + it 'old menus are destroyed' do + subject.ensure_menu_list(menus) + expect(PxeMenu.count).to eq(1) + expect(PxeImage.count).to eq(0) + expect(subject.pxe_menus.find_by(:file_name => 'new')).not_to be_nil + end + end + end + + context 'when two servers have similar menu' do + let(:other_server) { FactoryBot.create(:pxe_server) } + it 'they are not mixed up' do + subject.ensure_menu_list(%w[new]) + other_server.ensure_menu_list(%w[new]) + expect(PxeMenu.count).to eq(2) + expect(subject.pxe_menus.count).to eq(1) + expect(other_server.pxe_menus.count).to eq(1) + expect(subject.pxe_menus.map(&:id)).not_to eq(other_server.pxe_menus.map(&:id)) + end + end + end end