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.

Sign up for GitHub

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

Jump to bottom

[BUG] tags.GetCategory fails with unrelated error when finding category by name #2710

Closed
r4f4 opened this issue Jan 21, 2022 · 3 comments · Fixed by #2746
Closed

[BUG] tags.GetCategory fails with unrelated error when finding category by name #2710

r4f4 opened this issue Jan 21, 2022 · 3 comments · Fixed by #2746
Assignees
@embano1

Comments

@r4f4
Copy link

r4f4 commented Jan 21, 2022

Describe the bug
When trying to find a category by name, the current code will list all the category ids, loop over those ids fetching the respective category objects and then loop over those objects trying to find one with a matching name. However, if during the fetching of the objects any of them fails (e.g, it was deleted in the meantime), the whole fetching is "aborted" and no more ids are tried. GetCategory then fails even if the category exists and should've been fetched successfully.

To Reproduce
How it happens in Openshift:
Cluster A and B were created, and CI starts deleting Cluster A and then Cluster B in parallel.
Cluster A gets a list of category IDs that include [A,B]
Cluster A loops through all category IDs and gets a list of categories that includes [A,B]
Cluster B gets a list of category IDs that include [A,B]
Cluster A deletes category A.
Cluster B fails on the loop for all category IDs because category A is now 404.
Cluster B exits with an error, leaking category B.

Expected behavior
All the ids are tried before failing with an error

Affected version
Current master 6515e08

Screenshots/Debug Output
https://bugzilla.redhat.com/show_bug.cgi?id=2021041

Additional context
This will probably involve #1639 as well in case all ids were tried and no match was found
@github-actions
Copy link
Contributor

Howdy 🖐   r4f4 ! Thank you for your interest in this project. We value your feedback and will respond soon.

If you want to contribute to this project, please make yourself familiar with the CONTRIBUTION guidelines.

r4f4 added a commit to r4f4/installer that referenced this issue Jan 21, 2022
@r4f4
Bug 2021041: vsphere: Not found TagCategory when destroying ipi cluster
ed9ad2a 
While vmware/govmomi#2710 is not fixed
upstream, let's carry our own fixed version: try out all the category
ids before erroring out.

Signed-off-by: Rafael Fonseca <r4f4rfs@gmail.com>
@r4f4 r4f4 mentioned this issue Jan 21, 2022
Merged
r4f4 added a commit to r4f4/installer that referenced this issue Jan 21, 2022
@r4f4
Bug 2021041: vsphere: Not found TagCategory when destroying ipi cluster
6f74e5b 
While vmware/govmomi#2710 is not fixed
upstream, let's carry our own fixed version: try out all the category
ids before erroring out.

Signed-off-by: Rafael Fonseca <r4f4rfs@gmail.com>
@embano1
Copy link
Contributor

embano1 commented Jan 23, 2022 •
edited

Thx @r4f4 for reporting this. You're not the first one to hit this, so I'm glad you filed an issue.

Fix would be:

  • Check returned err in c.GetCategory if it's a 403 or 404
  • If 404 ignore and continue
  • Add tests
  • Document this semantical change in the signature

I might be able to work on this in the next days.

r4f4 added a commit to r4f4/installer that referenced this issue Feb 2, 2022
@r4f4
Bug 2021041: vsphere: Not found TagCategory when destroying ipi cluster
dd6f1b5 
While vmware/govmomi#2710 is not fixed
upstream, let's carry our own fixed version: try out all the category
ids before erroring out.

Signed-off-by: Rafael Fonseca <r4f4rfs@gmail.com>
@embano1 embano1 self-assigned this Feb 8, 2022
embano1 pushed a commit to embano1/govmomi that referenced this issue Feb 9, 2022
fix: Ignore concurrent deletes in GetCategories
3f9eff9 
GetCategories() would fail if a category was deleted between the initial
ListCategories() and GetCategory() details loop which is not atomic.

Closes: vmware#2710
Signed-off-by: Michael Gasch <mgasch@vmware.com>
@embano1 embano1 mentioned this issue Feb 9, 2022
Merged
11 tasks
@embano1
Copy link
Contributor

embano1 commented Feb 9, 2022

@r4f4 fixed in #2746

embano1 pushed a commit to embano1/govmomi that referenced this issue Feb 10, 2022
fix: Ignore concurrent deletes in GetCategories
7e2ce13 
GetCategories() would fail if a category was deleted between the initial
ListCategories() and GetCategory() details loop which is not atomic.

Closes: vmware#2710
Signed-off-by: Michael Gasch <mgasch@vmware.com>
dougm pushed a commit that referenced this issue Feb 10, 2022
@dougm
fix: Ignore concurrent deletes in GetCategories
dde5090 
GetCategories() would fail if a category was deleted between the initial
ListCategories() and GetCategory() details loop which is not atomic.

Closes: #2710
Signed-off-by: Michael Gasch <mgasch@vmware.com>
r4f4 added a commit to r4f4/installer that referenced this issue Feb 23, 2022
@r4f4
Bug 2021041: vsphere: Not found TagCategory when destroying ipi cluster
3fb9718 
While vmware/govmomi#2710 is not fixed
upstream, let's carry our own fixed version: try out all the category
ids before erroring out.

Signed-off-by: Rafael Fonseca <r4f4rfs@gmail.com>
openshift-cherrypick-robot pushed a commit to openshift-cherrypick-robot/installer that referenced this issue Mar 10, 2022
@r4f4
Bug 2021041: vsphere: Not found TagCategory when destroying ipi cluster
b338aab 
While vmware/govmomi#2710 is not fixed
upstream, let's carry our own fixed version: try out all the category
ids before erroring out.

Signed-off-by: Rafael Fonseca <r4f4rfs@gmail.com>
AnnaZivkovic pushed a commit to AnnaZivkovic/installer that referenced this issue Apr 1, 2022
@r4f4
Bug 2021041: vsphere: Not found TagCategory when destroying ipi cluster
a3082c0 
While vmware/govmomi#2710 is not fixed
upstream, let's carry our own fixed version: try out all the category
ids before erroring out.

Signed-off-by: Rafael Fonseca <r4f4rfs@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@embano1 embano1

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Ignore concurrent deletes in GetCategories() embano1/govmomi
2 participants
@r4f4 @embano1