diff --git a/app/models/host/managed.rb b/app/models/host/managed.rb index 133fa92d4a6..a1504936263 100644 --- a/app/models/host/managed.rb +++ b/app/models/host/managed.rb @@ -333,6 +333,7 @@ class Jail < ::Safemode::Jail before_validation :set_compute_attributes, :on => :create, :if => proc { compute_attributes_empty? } validate :check_if_provision_method_changed, :on => :update, :if => proc { |host| host.managed } validates :uuid, uniqueness: { :allow_blank => true } + validate :check_if_rebuild_allowed, :if => proc { |host| host.build_changed?(from: false, to: true) } before_validation :set_hostgroup_defaults, :set_ip_address after_validation :ensure_associations @@ -500,6 +501,23 @@ def self.registered_provision_methods Foreman::Plugin.all.map(&:provision_methods).inject(:merge) || {} end + def check_if_rebuild_allowed + return true unless rebuild_requires_poweroff && power.ready? + + errors.add(:build, N_('The host must be powered off to build.')) + false + rescue Foreman::Exception => _e + logger.info "Could not read power state of #{name}. Allowing to build the host." + true + end + + def rebuild_requires_poweroff + method_name = "#{provision_method}_rebuild_requires_poweroff" + return false unless respond_to?(method_name) + + public_send(method_name) + end + def self.valid_rebuild_only_values if Host::Managed.respond_to?(:rebuild_methods) Nic::Managed.rebuild_methods.values + Host::Managed.rebuild_methods.values diff --git a/app/views/api/v2/hosts/main.json.rabl b/app/views/api/v2/hosts/main.json.rabl index c55efeb9649..305c9854b0b 100644 --- a/app/views/api/v2/hosts/main.json.rabl +++ b/app/views/api/v2/hosts/main.json.rabl @@ -13,7 +13,7 @@ attributes :ip, :ip6, :last_report, :mac, :realm_id, :realm_name, :sp_mac, :sp_ip, :sp_name, :domain_id, :domain_name, :architecture_id, :architecture_name, :operatingsystem_id, :operatingsystem_name, :subnet_id, :subnet_name, :subnet6_id, :subnet6_name, :sp_subnet_id, :ptable_id, :ptable_name, :medium_id, :medium_name, :pxe_loader, :build, :comment, :disk, :initiated_at, :installed_at, :model_id, :hostgroup_id, :owner_id, :owner_name, :owner_type, :creator_id, :creator, - :enabled, :managed, :use_image, :image_file, :uuid, + :enabled, :managed, :use_image, :image_file, :uuid, :rebuild_requires_poweroff, :compute_resource_id, :compute_resource_name, :compute_profile_id, :compute_profile_name, :capabilities, :provision_method, :certname, :image_id, :image_name, :created_at, :updated_at, diff --git a/test/models/host_test.rb b/test/models/host_test.rb index 87d2ac86144..360dca5566a 100644 --- a/test/models/host_test.rb +++ b/test/models/host_test.rb @@ -549,6 +549,62 @@ class HostTest < ActiveSupport::TestCase refute_equal original_status, new_status end + test "rebuild_requires_poweroff is validated and prevents to re-build a running host" do + power = mock("power") + power.stubs(:ready?).returns(true) + + host = FactoryBot.create(:host, :managed) + host.stubs(:power).returns(power) + host.save! + + host.expects(:build_rebuild_requires_poweroff).returns(true) + host.build = true + refute host.valid? + assert host.errors[:build].include?("The host must be powered off to build.") + end + + test "rebuild_requires_poweroff is validated and allows to re-build the turned-off host" do + power = mock("power") + power.stubs(:ready?).returns(false) + + host = FactoryBot.create(:host, :managed) + host.stubs(:power).returns(power) + host.save! + + host.expects(:build_rebuild_requires_poweroff).returns(true) + host.build = true + assert host.valid? + refute host.errors[:build].any? + end + + test "rebuild_requires_poweroff is validated and allows to re-build the host because provision_method allows it" do + power = mock("power") + power.stubs(:ready?).returns(true) + + host = FactoryBot.create(:host, :managed) + host.stubs(:power).returns(power) + host.save! + + host.expects(:build_rebuild_requires_poweroff).returns(false) + host.build = true + assert host.valid? + refute host.errors[:build].any? + end + + test "rebuild_requires_poweroff allows rebuild when power state check fails" do + power = mock("power") + power.stubs(:ready?).raises(Foreman::Exception.new("boom")) + + host = FactoryBot.create(:host, :managed) + host.stubs(:power).returns(power) + host.save! + + host.expects(:build_rebuild_requires_poweroff).returns(true) + host.build = true + assert host.valid? + refute host.errors[:build].any? + end + context 'host assigned to location and organization' do setup do @host = FactoryBot.create(:host, :managed => false) diff --git a/webpack/assets/javascripts/react_app/components/HostDetails/ActionsBar/index.js b/webpack/assets/javascripts/react_app/components/HostDetails/ActionsBar/index.js index d0cea17cffc..4f1efa867d0 100644 --- a/webpack/assets/javascripts/react_app/components/HostDetails/ActionsBar/index.js +++ b/webpack/assets/javascripts/react_app/components/HostDetails/ActionsBar/index.js @@ -45,6 +45,7 @@ const ActionsBar = ({ hostName, computeId, isBuild, + buildRequiresPowerOff, permissions: { destroy_hosts: canDestroy, create_hosts: canCreate, @@ -67,6 +68,7 @@ const ActionsBar = ({ ); const isConsoleDisabled = !(computeId && isHostActive); + const isHostActiveButRequiresPowerOff = buildRequiresPowerOff && isHostActive; const determineTooltip = () => { if (isConsoleDisabled) { if (computeId) { @@ -76,6 +78,27 @@ const ActionsBar = ({ } return undefined; }; + const determineBuildDescription = () => { + if (!isBuild) { + if (!canBuild) { + return __('No permission to start build for this host.'); + } + if (isHostActiveButRequiresPowerOff) { + return __('The host must be powered off to build.'); + } + return __('Start build for this host.'); + } + + // isBuild=true => build is already in progress + if (!canBuild) { + return __('No permission to cancel the build.'); + } + return __('Cancel the current build.'); + }; + // Build action is disabled if user doesn't have permissions to build or + // if the host is active but requires power off before build + const isBuildDisabled = + !canBuild || (!isBuild && isHostActiveButRequiresPowerOff); const buildHandler = () => { if (isBuild) { dispatch(cancelBuild(hostId, hostName)); @@ -90,7 +113,8 @@ const ActionsBar = ({ onClick={buildHandler} key="build" component="button" - isDisabled={!canBuild} + description={determineBuildDescription()} + isDisabled={isBuildDisabled} icon={ @@ -232,6 +256,7 @@ ActionsBar.propTypes = { computeId: PropTypes.number, permissions: PropTypes.object, isBuild: PropTypes.bool, + buildRequiresPowerOff: PropTypes.bool, }; ActionsBar.defaultProps = { hostId: undefined, @@ -245,6 +270,7 @@ ActionsBar.defaultProps = { build_hosts: false, }, isBuild: false, + buildRequiresPowerOff: false, }; export default ActionsBar; diff --git a/webpack/assets/javascripts/react_app/components/HostDetails/index.js b/webpack/assets/javascripts/react_app/components/HostDetails/index.js index 3188dd0b6e9..328029f2822 100644 --- a/webpack/assets/javascripts/react_app/components/HostDetails/index.js +++ b/webpack/assets/javascripts/react_app/components/HostDetails/index.js @@ -216,6 +216,7 @@ const HostDetails = ({ hostName={response.name} permissions={response.permissions} isBuild={response.build} + buildRequiresPowerOff={response.rebuild_requires_poweroff} />