Fixes #39067 - rebuild only if host powered off

Author: sbernhard

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

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,

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)

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,8 @@ const ActionsBar = ({
     );
 
   const isConsoleDisabled = !(computeId && isHostActive);
+  const isHostActiveButRequiresPowerOff =
+    buildRequiresPowerOff && isHostActive;
   const determineTooltip = () => {
     if (isConsoleDisabled) {
       if (computeId) {
@@ -76,6 +79,26 @@ 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.');
+    } else {
+      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={
         <Icon>
           <BuildIcon />
@@ -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;

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}
                 />
               </FlexItem>
             </Flex>