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

Include the request_type when creating a retirement request #17779

Merged
merged 1 commit into from
Aug 3, 2018

Conversation

bdunne
Copy link
Member

@bdunne bdunne commented Jul 30, 2018

@bdunne bdunne force-pushed the central_admin_retirement branch from 3ce32d9 to fd81fa9 Compare July 30, 2018 20:18
@bdunne bdunne force-pushed the central_admin_retirement branch from fd81fa9 to 46dc853 Compare July 31, 2018 18:48
@miq-bot
Copy link
Member

miq-bot commented Jul 31, 2018

Checked commit bdunne@46dc853 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
3 files checked, 0 offenses detected
Everything looks fine. 🏆

@bdunne bdunne changed the title [DEPENDS ON https://github.com/ManageIQ/manageiq-api/pull/439] Include the request_type when creating a retirement request Include the request_type when creating a retirement request Aug 3, 2018
@carbonin
Copy link
Member

carbonin commented Aug 3, 2018

@d-m-u @tinaafitz If retirement is being done as a request, does Vm even respond to #retire anymore?

I'm asking because I'm wondering if we should remove https://github.com/ManageIQ/manageiq/blob/master/app/models/vm/operations/lifecycle.rb#L21-L25 or is it still valid to call retire directly on a Vm or Service object?

@carbonin
Copy link
Member

carbonin commented Aug 3, 2018

Oh I see, we added a method to the mixin rather than replacing the existing retire method.

So it sounds like this broke when retirement went from a direct method on the Vm object to a request.

Now instead of pushing the retire action over the API for central admin on the resource this PR is giving us the information we need for when we relay the create_request method.

@bdunne
Copy link
Member Author

bdunne commented Aug 3, 2018

Correct, now that it's hitting create_request which will be relayed to the remote region, we need to include the type of request for the remote region to parse.

Copy link
Member

@carbonin carbonin left a comment

Choose a reason for hiding this comment

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

Makes sense to me. 👍

@carbonin carbonin self-assigned this Aug 3, 2018
@carbonin carbonin added the core label Aug 3, 2018
@carbonin carbonin merged commit e90f712 into ManageIQ:master Aug 3, 2018
@carbonin carbonin added this to the Sprint 92 Ending Aug 13, 2018 milestone Aug 3, 2018
@carbonin
Copy link
Member

carbonin commented Aug 3, 2018

When was the retirement-as-a-request added? Does this need to be backported?

@bdunne bdunne deleted the central_admin_retirement branch August 3, 2018 19:54
@bdunne
Copy link
Member Author

bdunne commented Aug 3, 2018

I don't think it made it back to g-branch. @tinaafitz @d-m-u Can you confirm?

@tinaafitz
Copy link
Member

@bdunne That is correct. The new retirement as a request changes were not back ported to the g-branch.

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.

5 participants