Refactor instance state checks to use new methods

Author: neddp

src/bosh-director/lib/bosh/director/deployment_plan/instance.rb

@@ -314,6 +314,18 @@ def virtual_state
         @virtual_state.to_s.inquiry
       end
 
+      def started?
+        state == STATE_STARTED
+      end
+
+      def stopped?
+        state == STATE_STOPPED
+      end
+
+      def detached?
+        state == STATE_DETACHED
+      end
+
       ##
       # Checks if the target VM already has the same set of trusted SSL certificates
       # as the director currently wants to install on all managed VMs. This will

src/bosh-director/lib/bosh/director/deployment_plan/instance_group.rb

@@ -56,13 +56,6 @@ class InstanceGroup
       # @return [DeploymentPlan::UpdateConfig] Instance group update settings
       attr_reader :update
 
-      # @return [Array<DeploymentPlan::Instance>] All instances
-      attr_accessor :instances
-
-      # @return [Array<Models::Instance>] List of excess instance models that
-      #   are not needed for current deployment
-      attr_accessor :unneeded_instances
-
       # @return [String] Expected instance group state
       attr_reader :state
 
@@ -159,7 +152,7 @@ def unignored_instance_plans_needing_duplicate_vm
                                                      .select(&:needs_duplicate_vm?)
                                                      .reject(&:new?)
                                                      .reject(&:should_be_ignored?)
-                                                     .reject { |plan| plan.instance.state == 'detached' }
+                                                     .reject { |plan| plan.instance.detached? }
       end
 
       def add_job(job_to_add)
@@ -370,7 +363,7 @@ def errand?
 
       def instance_plans_with_missing_vms
         needed_instance_plans.reject do |instance_plan|
-          instance_plan.instance.vm_created? || instance_plan.instance.state == 'detached'
+          instance_plan.instance.vm_created? || instance_plan.instance.detached?
         end
       end
 

src/bosh-director/lib/bosh/director/deployment_plan/instance_plan.rb

@@ -186,15 +186,15 @@ def network_settings_changed?
         end
 
         def state_changed?
-          if instance.state == Instance::STATE_DETACHED && existing_instance.state != instance.state
+          if instance.detached? && existing_instance.state != instance.state
             @logger.debug("Instance '#{instance}' needs to be detached")
             return true
           end
 
           return true if unresponsive_agent?
 
-          if (instance.state == Instance::STATE_STOPPED && instance.current_job_state.running?) ||
-             (instance.state == Instance::STATE_STARTED && !instance.current_job_state.running?)
+          if (instance.stopped? && instance.current_job_state.running?) ||
+             (instance.started? && !instance.current_job_state.running?)
             @logger.debug("Instance state is '#{instance.state}' and agent reports '#{instance.current_job_state}'")
             return true
           end

src/bosh-director/lib/bosh/director/deployment_plan/planner.rb

@@ -260,7 +260,7 @@ def instance_groups_starting_on_deploy
           if instance_group.service?
             instance_groups << instance_group
           elsif instance_group.errand?
-            if instance_group.instances.any?(&:vm_created?) || instance_group.instances.any? { |i| i.model.state == 'detached' }
+            if instance_group.instances.any?(&:vm_created?) || instance_group.instances.any? { |i| i.model.detached? }
               instance_groups << instance_group
             end
           end

src/bosh-director/lib/bosh/director/instance_updater/state_applier.rb

@@ -17,7 +17,7 @@ def apply(update_config, wait_for_running = true)
       @instance.update_templates(@instance_plan.templates)
       @rendered_job_templates_cleaner.clean
 
-      start(update_config, wait_for_running) if @instance.state == 'started'
+      start(update_config, wait_for_running) if @instance.started?
     end
 
     private

src/bosh-director/lib/bosh/director/instance_updater/update_procedure.rb

@@ -78,7 +78,7 @@ def perform
           handle_not_detached_instance_plan
 
           # desired state
-          if instance.state == 'stopped'
+          if instance.stopped?
             # Command issued: `bosh stop`
             update_instance
             return
@@ -87,12 +87,12 @@ def perform
           handle_detached_instance_if_detached
         end
 
-        converge_vm if instance.state != 'detached'
+        converge_vm unless instance.detached?
         update_instance
         update_dns_if_changed
         update_vm_disk_metadata
 
-        return if instance.state == 'detached'
+        return if instance.detached?
 
         @rendered_templates_persister.persist(instance_plan)
         apply_state
@@ -119,19 +119,19 @@ def handle_not_detached_instance_plan
           @rendered_templates_persister.persist(instance_plan)
         end
 
-        unless instance_plan.needs_shutting_down? || instance.state == 'detached'
+        unless instance_plan.needs_shutting_down? || instance.detached?
           DeploymentPlan::Steps::PrepareInstanceStep.new(instance_plan).perform(instance_report)
         end
 
         # current state
-        return unless instance.model.state != 'stopped'
+        return if instance.model.stopped?
 
         stop
         take_snapshot
       end
 
       def handle_detached_instance_if_detached
-        return unless instance.state == 'detached'
+        return unless instance.detached?
 
         # Command issued: `bosh stop --hard`
         @logger.info("Detaching instance #{instance}")
@@ -149,7 +149,7 @@ def update_instance
       def update_vm_disk_metadata
         return unless instance_plan.changes.include?(:tags)
         return if instance_plan.new? || @needs_recreate
-        return if instance.state == 'detached' # disks will get a metadata update when attaching again
+        return if instance.detached? # disks will get a metadata update when attaching again
 
         @logger.debug("Updating instance #{instance} VM and disk metadata with tags")
         tags = instance_plan.tags
@@ -188,7 +188,7 @@ def stop
       end
 
       def deleting_vm?
-        @needs_recreate || instance_plan.needs_shutting_down? || instance.state == 'detached' ||
+        @needs_recreate || instance_plan.needs_shutting_down? || instance.detached? ||
           (instance_plan.should_create_swap_delete? && instance_plan.instance.model.vms.count > 1)
       end
 

src/bosh-director/lib/bosh/director/jobs/attach_disk.rb

@@ -53,7 +53,7 @@ def validate_instance(instance)
               'Attaching disks to ignored instances is not allowed.'
         end
 
-        if instance.state != 'detached' && instance.state != 'stopped'
+        unless instance.detached? || instance.stopped?
           raise AttachDiskInvalidInstanceState, "Instance '#{@job_name}/#{@instance_id}' in deployment '#{@deployment_name}' must be in 'bosh stopped' state"
         end
       end
@@ -66,7 +66,7 @@ def handle_previous_disk(instance)
           @cloud_properties = previous_persistent_disk.cloud_properties
         end
 
-        if instance.state == 'stopped'
+        if instance.stopped?
           @disk_manager.detach_disk(previous_persistent_disk)
         end
 
@@ -88,7 +88,7 @@ def handle_new_disk(instance)
           )
         end
 
-        if instance.state == 'stopped'
+        if instance.stopped?
           @disk_manager.attach_disk(disk, instance.deployment.tags)
         end
       end