ananace / fog-hyperv

Hyper-V provider for fog
MIT License
8 stars 1 forks source link

State not recognized correctly #19

Closed ksagle77 closed 6 years ago

ksagle77 commented 6 years ago

Hey,

I'm seeing the following issue, likely related to https://github.com/ace13/fog-hyperv/commit/4574e20c073e79ce03b3069e0b5d4f4476de3c6c#diff-71f294f976a361bc4b9099a12f5c25ae.

State is returned as null from fog at command line:

image

In the foreman, VMs all show as powered off.

image

image

Clicking on one of the VMs throws the following:

 | NoMethodError: undefined method `start_with?' for :id:Symbol
 | /usr/share/foreman/.gem/ruby/gems/fog-hyperv-0.0.7/lib/fog/hyperv/compute.rb:240:in `block in run_shell'
 | /usr/share/foreman/.gem/ruby/gems/fog-hyperv-0.0.7/lib/fog/hyperv/compute.rb:240:in `delete_if'
 | /usr/share/foreman/.gem/ruby/gems/fog-hyperv-0.0.7/lib/fog/hyperv/compute.rb:240:in `run_shell'
 | /usr/share/foreman/.gem/ruby/gems/fog-hyperv-0.0.7/lib/fog/hyperv/requests/compute/get_vm.rb:6:in `get_vm'
 | /usr/share/foreman/.gem/ruby/gems/fog-hyperv-0.0.7/lib/fog/collection.rb:20:in `all'
 | /usr/share/foreman/.gem/ruby/gems/fog-hyperv-0.0.7/lib/fog/collection.rb:27:in `get'
 | /usr/share/foreman/.gem/ruby/gems/fog-hyperv-0.0.7/lib/fog/hyperv/models/compute/servers.rb:16:in `get'
 | /usr/share/foreman/app/models/compute_resource.rb:173:in `find_vm_by_uuid'
 | /usr/share/foreman/app/controllers/compute_resources_vms_controller.rb:117:in `find_vm'
 | /opt/rh/sclo-ror42/root/usr/share/gems/gems/activesupport-4.2.5.1/lib/active_support/callbacks.rb:432:in `block in make_lambda'

If I comment out the following line in lib/fog/hyperv/compute.rb:

options.delete_if { |o| o.start_with? '_' }

I am able to see the VM details with N/A as the state.

image

Trying to power on the VM throws the following: image

However, the VM does actually start:

image

Kevin

ksagle77 commented 6 years ago

changing

options.delete_if { |o| o.start_with? '_' }

to

options.delete_if { |o| o.to_s.start_with? '_' }

also allows me to click and see VM details (still with N/A state.)

ananace commented 6 years ago

Oh dear, another stupid mistake that should've never been committed in the first place. There should indeed be a to_s there.

I'll try and have a look at the enum issue tomorrow, I have an idea of exactly where the problem might lay, but I'll need to get to my test setup before I can verify it.

Thanks for testing and reporting the issues. :heart:

ananace commented 6 years ago

Enum handling should work now, so no more state being N/A.

Could you test if this also solves the capitalize issue for you? I think the error comes from Foreman's own compute resource controller, which tries to run notice _("%{vm} is now %{vm_state}") % {:vm => @vm, :vm_state => @vm.state.capitalize} for state changes.

ksagle77 commented 6 years ago

Hey,

Enum issue seems to be corrected on the Fog side. I am able to query and start VMs from fog now:

image

From foreman, the VM starts/stops correctly, but the following is thrown:

image

Thanks Ace! :)

ananace commented 6 years ago

Right, Ruby 2.4 has done some changes apparently.

Pushed another commit for that, hopefully that'll work even better for you with it :)

ksagle77 commented 6 years ago

Hey Ace,

It doesn't seem to like integer instead of fixnum.

[4] pry(main)> compute.servers[1].ready?
Fog::Hyperv::Errors::ServiceError: :"2" is not one of [:Unknown, :Running, :Off, :Stopping, :Saved, :Paused, :Starting, :Reset, :Saving, :Pausing, :Resuming, :FastSaved, :FastSaving, itical, :StoppingCritical, :SavedCritical, :PausedCritical, :StartingCritical, :ResetCritical, :SavingCritical, :PausingCritical, :ResumingCritical, :FastSavedCritical, :FastSavingCri
from /home/kevin/test_8/new/fog-hyperv/lib/fog/hyperv/fog_extensions/enum.rb:38:in `state='
[5] pry(main)>

I was able to get it working with Numeric:

image

However, that caused a problem getting the power state in foreman:

image

Issue is that state returns a symbol:

image

I was able to resolve this by modifying create_getter like this:

      def create_getter
        ensure_value_getter
        model.class_eval <<-EOS, __FILE__, __LINE__
            def #{name}_num
              _values = #{name}_values
              return nil if self.#{name}.nil?
              if self.#{name}.is_a?(Fixnum)
                self.#{name}
              elsif self.#{name}.is_a?(Symbol)
                 _values[self.#{name}]
              else
                if _values.is_a?(Hash)
                  _values.key(self.#{name})
                else
                  _values.index(self.#{name})
                end
              end
            end
        EOS

But I'm unsure if that would have other unintended consequences. image

image

Kevin

ananace commented 6 years ago

Big thanks for the debugging help, I've got an idea about how to properly solve the foreman power issue as well, without causing issues in other ways. I won't have time to push that code until later today though.

On 25 Jan 2018 15:47, "ksagle77" notifications@github.com wrote:

Hey Ace,

It doesn't seem to like integer instead of fixnum.

[4] pry(main)> compute.servers[1].ready? Fog::Hyperv::Errors::ServiceError: :"2" is not one of [:Unknown, :Running, :Off, :Stopping, :Saved, :Paused, :Starting, :Reset, :Saving, :Pausing, :Resuming, :FastSaved, :FastSaving, itical, :StoppingCritical, :SavedCritical, :PausedCritical, :StartingCritical, :ResetCritical, :SavingCritical, :PausingCritical, :ResumingCritical, :FastSavedCritical, :FastSavingCri from /home/kevin/test_8/new/fog-hyperv/lib/fog/hyperv/fog_extensions/enum.rb:38:in `state=' [5] pry(main)>

I was able to get it working with Numeric:

[image: image] https://user-images.githubusercontent.com/34070189/35391889-fce40e68-01ad-11e8-9eed-f50b96b07b3b.png

However, that caused a problem getting the power state in foreman:

[image: image] https://user-images.githubusercontent.com/34070189/35393213-518c3590-01b2-11e8-94f2-fd76fc7f7403.png

Issue is that state returns a symbol:

[image: image] https://user-images.githubusercontent.com/34070189/35393665-a01383ca-01b3-11e8-9d28-89a4910d2b63.png

I was able to resolve this by modifying create_getter like this:

  def create_getter
    ensure_value_getter
    model.class_eval <<-EOS, __FILE__, __LINE__
        def #{name}_num
          _values = #{name}_values
          return nil if self.#{name}.nil?
          if self.#{name}.is_a?(Fixnum)
            self.#{name}
          elsif self.#{name}.is_a?(Symbol)
             _values[self.#{name}]
          else
            if _values.is_a?(Hash)
              _values.key(self.#{name})
            else
              _values.index(self.#{name})
            end
          end
        end
    EOS

But I'm unsure if that would have other unintended consequences. [image: image] https://user-images.githubusercontent.com/34070189/35393826-1aca4b30-01b4-11e8-8ff4-dc5bd3ed11f0.png

[image: image] https://user-images.githubusercontent.com/34070189/35393843-25cbe0b6-01b4-11e8-91da-5cdecff9d748.png

Kevin

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHub https://github.com/ace13/fog-hyperv/issues/19#issuecomment-360487441, or mute the thread https://github.com/notifications/unsubscribe-auth/AAgnvKSC98BE4JxTuPPWI3WtvUdF8Rgyks5tOJPkgaJpZM4Rp2tb .

ananace commented 6 years ago

Pushed a new commit to both fog-hyperv and foreman_hyperv, hopefully that should take care of that last issue.

Thanks again for the debugging and reporting :heart:

zyronix commented 6 years ago

I can confirm that your patch resolves the issue. Small note: the gem version still contains the bugged code.

ananace commented 6 years ago

Tagged and pushed a new version with all these fixes