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

Don't retire load balancers #18443

Merged
merged 1 commit into from
Feb 11, 2019

Conversation

d-m-u
Copy link
Contributor

@d-m-u d-m-u commented Feb 11, 2019

Presently system_context retirement only checks to see if the evm_owner_id is present on the object, it doesn't check that that id is that of a valid user. In order to make the check run on evm_owner we will need to remove load balancers from the list of things to retire in the retirement manager per #18437 (comment) since load balancers don't have owners.

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1673143. (it doesn't fix it but the related PR does and this is required for the related PR)

related to

#18437

@d-m-u
Copy link
Contributor Author

d-m-u commented Feb 11, 2019

@miq-bot add_label bug
@miq-bot add_reviewer @bdunne
@miq-bot assign @gmcculloug

@miq-bot miq-bot added the bug label Feb 11, 2019
@miq-bot miq-bot requested a review from bdunne February 11, 2019 14:08
@d-m-u d-m-u changed the title don't retire load balancers Don't retire load balancers Feb 11, 2019
@gmcculloug
Copy link
Member

We should also remove https://github.com/ManageIQ/manageiq/blob/master/app/models/load_balancer.rb#L5 and check for other retirement references on this object.

@d-m-u d-m-u force-pushed the remove_lbs_from_retirement_check branch from 38eb29b to 4940668 Compare February 11, 2019 14:16
@JPrause
Copy link
Member

JPrause commented Feb 11, 2019

@d-m-u if this can be backported, can you add the hammer/yes label.

@d-m-u
Copy link
Contributor Author

d-m-u commented Feb 11, 2019

@miq-bot add_label hammer/yes

@gmcculloug gmcculloug merged commit d8a6ba0 into ManageIQ:master Feb 11, 2019
@gmcculloug gmcculloug added this to the Sprint 105 Ending Feb 18, 2019 milestone Feb 11, 2019
simaishi pushed a commit that referenced this pull request Feb 18, 2019
@simaishi
Copy link
Contributor

Hammer backport details:

$ git log -1
commit 2aa3228428546cfd16f4d67319fb5aac01b7d25e
Author: Greg McCullough <gmccullo@redhat.com>
Date:   Mon Feb 11 12:04:57 2019 -0500

    Merge pull request #18443 from d-m-u/remove_lbs_from_retirement_check
    
    Don't retire load balancers
    
    (cherry picked from commit d8a6ba00be00d215a44af0f71e27f7ad364967ba)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1678476

@d-m-u
Copy link
Contributor Author

d-m-u commented Feb 19, 2019

sorry, i messed up, this should be hammer/no
@miq-bot add_label hammer/no

simaishi added a commit that referenced this pull request Feb 19, 2019
@simaishi
Copy link
Contributor

Reverted the backport:

commit 7a4a22b59ea8598b431488d0f68d70bad456cf59
Author: Satoe Imaishi <simaishi@redhat.com>
Date:   Tue Feb 19 10:10:07 2019 -0500

    Revert "Merge pull request #18443 from d-m-u/remove_lbs_from_retirement_check"
    
    This reverts commit 2aa3228428546cfd16f4d67319fb5aac01b7d25e.

@d-m-u d-m-u deleted the remove_lbs_from_retirement_check branch September 26, 2019 10:34
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