Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(CI Failure Fix ) Put make_retire_request on queue in VmsController #662

Merged

Conversation

lpichler
Copy link
Contributor

this is also fixing CI failure https://travis-ci.org/ManageIQ/manageiq-api/jobs/576957657

api request:

curl -X POST http://localhost:3090/api/vms/1 -d '{ "action" : "request_retire"}' ..

please review @lfu

Links

It was on core rep here: ManageIQ/manageiq#19064

@miq-bot assign @abellotti

Record in MiqQueue looks like after API request:

#<MiqQueue:0x00007f95628251a0> {
                :id => 133284175,
         :target_id => nil,
          :priority => 100,
       :method_name => "make_retire_request",
             :state => "ready",
        :created_on => Mon, 26 Aug 2019 18:55:58 UTC +00:00,
        :updated_on => Mon, 26 Aug 2019 18:55:58 UTC +00:00,
      :lock_version => 0,
           :task_id => nil,
        :deliver_on => nil,
        :queue_name => "generic",
        :class_name => "ManageIQ::Providers::Vmware::InfraManager::Vm",
       :instance_id => 1,
              :args => [],
      :miq_callback => {
         :class_name => "MiqTask",
        :instance_id => 435627,
        :method_name => :queue_callback,
               :args => [
            [0] "Finished"
        ]
    },
          :msg_data => nil,
              :zone => nil,
              :role => "automate",
       :server_guid => nil,
       :msg_timeout => 600,
        :handler_id => nil,
      :handler_type => nil,
        :expires_on => nil,
    :tracking_label => nil,
           :user_id => 1,
          :group_id => 2,
         :tenant_id => 1,
       :miq_task_id => 435627
}

@lfu
Copy link
Member

lfu commented Aug 26, 2019

cc @d-m-u

@@ -273,6 +273,16 @@ def set_miq_server_resource(type, id, data)
action_result(false, "Failed to set miq_server - #{err}")
end

def request_retire_resource(type, id, _data = nil)
raise BadRequestError, "Must specify an id for retiring request for a #{type} resource" unless id
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, doesn't resource_search handle the case where the id isn't present?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes you are right 👍


api_action(type, id) do |klass|
vm = resource_search(id, type, klass)
api_log_info("Retiring request of vn #{vm_ident(vm)}")
Copy link
Contributor

@d-m-u d-m-u Aug 26, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And this is a stupid nit, sorry, but you have a typo here, vm not vn

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done 👍

@abellotti abellotti changed the title (CI Failiure Fix ) Put make_retire_request on queue in VmsController (CI Failure Fix ) Put make_retire_request on queue in VmsController Aug 26, 2019
@lpichler lpichler force-pushed the do_policy_check_before_retiring_request branch from 2eb9be4 to efdc5c8 Compare August 27, 2019 10:12
Copy link
Contributor

@d-m-u d-m-u left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is great, thank you!!

"message" => message_vm2,
"task_id" => task_id_vm2.to_s,
"task_href" => "http://www.example.com/api/tasks/#{task_id_vm2}",
"href" => "http://www.example.com/api/vms/#{vm2.id}"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in above expectations of api urls, would prefer usage of rails helper methods, i.e. a_string_matching(api_vm_url(nil, vm))

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@abellotti sure, I changed it, thanks!

@lpichler lpichler force-pushed the do_policy_check_before_retiring_request branch from efdc5c8 to b29fd91 Compare August 27, 2019 14:02
@miq-bot
Copy link
Member

miq-bot commented Aug 27, 2019

Checked commit lpichler@b29fd91 with ruby 2.4.6, rubocop 0.69.0, haml-lint 0.20.0, and yamllint 1.10.0
2 files checked, 0 offenses detected
Everything looks fine. 🍰

@abellotti
Copy link
Member

Thanks you @lpichler for the fix and update. LGTM!! 👍

will merge when green.

@abellotti abellotti merged commit d3b5c8e into ManageIQ:master Aug 27, 2019
@ghost
Copy link

ghost commented Aug 27, 2019

Shouldn't it be tagged as ivanchuk/yes ?

@lpichler lpichler deleted the do_policy_check_before_retiring_request branch August 27, 2019 14:39
@lpichler
Copy link
Contributor Author

lpichler commented Aug 27, 2019

Shouldn't it be tagger as ivanchuk/yes ?

Can you confirm it @lfu , please ?

I guess that ManageIQ/manageiq#19064 is in ivanchuk. so I think that you are right. thanks @fdupont-redhat

@miq-bot add_label ivanchuk/yes

@lfu
Copy link
Member

lfu commented Aug 27, 2019

@lpichler Thanks for the fix!
ManageIQ/manageiq#19064 is hammer/yes and ivanchuk/yes. The same should be applied to this commit.

@lpichler
Copy link
Contributor Author

@lfu thanks also for you help with this fix.

simaishi pushed a commit that referenced this pull request Dec 17, 2019
…_request

(CI Failure Fix ) Put make_retire_request on queue in VmsController

(cherry picked from commit d3b5c8e)
@simaishi
Copy link
Contributor

Ivanchuk backport details:

$ git log -1
commit 4ff6a9ccef4ced348912f31ebb6836f97fd51f62
Author: Alberto Bellotti <[email protected]>
Date:   Tue Aug 27 10:20:04 2019 -0400

    Merge pull request #662 from lpichler/do_policy_check_before_retiring_request

    (CI Failure Fix ) Put make_retire_request on queue in VmsController

    (cherry picked from commit d3b5c8edb74b606f8e57c9928ca7dfd90fa31118)

simaishi pushed a commit that referenced this pull request Dec 17, 2019
…_request

(CI Failure Fix ) Put make_retire_request on queue in VmsController

(cherry picked from commit d3b5c8e)
@simaishi
Copy link
Contributor

Hammer backport details:

$ git log -1
commit 0df1575a0d689330090c243f94bd34ee0f9ea821
Author: Alberto Bellotti <[email protected]>
Date:   Tue Aug 27 10:20:04 2019 -0400

    Merge pull request #662 from lpichler/do_policy_check_before_retiring_request

    (CI Failure Fix ) Put make_retire_request on queue in VmsController

    (cherry picked from commit d3b5c8edb74b606f8e57c9928ca7dfd90fa31118)

d-m-u added a commit to d-m-u/manageiq-api that referenced this pull request Feb 20, 2020
d-m-u added a commit to d-m-u/manageiq-api that referenced this pull request Feb 21, 2020
d-m-u added a commit to d-m-u/manageiq-api that referenced this pull request Feb 21, 2020
d-m-u added a commit to d-m-u/manageiq-api that referenced this pull request Feb 21, 2020
d-m-u added a commit to d-m-u/manageiq-api that referenced this pull request Feb 24, 2020
d-m-u added a commit to d-m-u/manageiq-api that referenced this pull request Feb 24, 2020
d-m-u added a commit to d-m-u/manageiq-api that referenced this pull request Feb 24, 2020
d-m-u added a commit to d-m-u/manageiq-api that referenced this pull request Feb 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants