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

Make sure Containers Exist Before Processing #16922

Merged

Conversation

jerryk55
Copy link
Member

@jerryk55 jerryk55 commented Jan 31, 2018

The crosslink code for the Swift Manager refresh parser
attempts to loop through the set of containers for the
manager without checking if any exist first.
Fix by checking first.

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1538501

@roliveri @hsong-rh please review.

Links

@jerryk55
Copy link
Member Author

@miq-bot add_label fine/yes

@jerryk55
Copy link
Member Author

@miq-bot add_label gaprindashvili/yes

@jerryk55
Copy link
Member Author

@miq-bot add_label bug

@jerryk55 jerryk55 changed the title Make sure Containers Before Processing Make sure Containers Exist Before Processing Jan 31, 2018
The crosslink code for the Swift Manager refresh parser
attempts to loop through the set of containers for the
manager without checking if any exist first.
Fix by checking first.
The same check that was added to the crosslink code in the previous
commit has to be added to the refresh parser cleanup code.
@jerryk55 jerryk55 force-pushed the fix_swift_refresher_with_empty_containers branch from d9f75d2 to 0579193 Compare February 1, 2018 21:50
@jerryk55
Copy link
Member Author

jerryk55 commented Feb 1, 2018

Was able to test on a live system exhibiting this issue and found a second bit of code in the refresh parser with the same problem. Fixed and tested in the wild. Ready for review @roliveri @hsong-rh

@miq-bot
Copy link
Member

miq-bot commented Feb 1, 2018

Checked commits jerryk55/manageiq@8fa42b2~...0579193 with ruby 2.3.3, rubocop 0.52.0, haml-lint 0.20.0, and yamllint 1.10.0
2 files checked, 0 offenses detected
Everything looks fine. 👍

@blomquisg
Copy link
Member

@jerryk55 any tests you can add for this?

@JPrause
Copy link
Member

JPrause commented Feb 5, 2018

@blomquisg this is a blocker for 5.8.3
Any idea when this can be merged.

@roliveri roliveri modified the milestone: Sprint 79 Ending Feb 12, 2018 Feb 5, 2018
@jerryk55
Copy link
Member Author

jerryk55 commented Feb 5, 2018

@blomquisg this is a hard thing to test - the environment has to be kind of screwed up to get yourself into this situation (I really don't think it should be a blocker personally). You have to have access to openstack but not to the swift containers that probably exist.

@roliveri roliveri merged commit 530eb07 into ManageIQ:master Feb 6, 2018
@roliveri roliveri added this to the Sprint 79 Ending Feb 12, 2018 milestone Feb 6, 2018
simaishi pushed a commit that referenced this pull request Feb 6, 2018
…y_containers

Make sure Containers Exist Before Processing
(cherry picked from commit 530eb07)

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1542740
@simaishi
Copy link
Contributor

simaishi commented Feb 6, 2018

Gaprindashvili backport details:

$ git log -1
commit b8540c321ec226b2cdf59ee674e56dfc6da455e5
Author: Richard Oliveri <oliveri.richard.github@gmail.com>
Date:   Tue Feb 6 11:56:23 2018 -0500

    Merge pull request #16922 from jerryk55/fix_swift_refresher_with_empty_containers
    
    Make sure Containers Exist Before Processing
    (cherry picked from commit 530eb073efbedc759b510b5071bceace9a0d6677)
    
    Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1542740

simaishi pushed a commit that referenced this pull request Feb 6, 2018
…y_containers

Make sure Containers Exist Before Processing
(cherry picked from commit 530eb07)

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1542741
@simaishi
Copy link
Contributor

simaishi commented Feb 6, 2018

Fine backport details:

 $ git log -1
commit 478b32a7d2c47a8e79c0acd8665927116ecf2400
Author: Richard Oliveri <oliveri.richard.github@gmail.com>
Date:   Tue Feb 6 11:56:23 2018 -0500

    Merge pull request #16922 from jerryk55/fix_swift_refresher_with_empty_containers
    
    Make sure Containers Exist Before Processing
    (cherry picked from commit 530eb073efbedc759b510b5071bceace9a0d6677)
    
    Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1542741

d-m-u pushed a commit to d-m-u/manageiq that referenced this pull request Jun 6, 2018
…ith_empty_containers

Make sure Containers Exist Before Processing
(cherry picked from commit 530eb07)

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1542741
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