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

Fix namespace when watching (rather than getting) resources. #4057

Merged
merged 1 commit into from
Jan 11, 2022

Conversation

absoludity
Copy link
Contributor

Signed-off-by: Michael Nelson minelson@vmware.com

I just took a brief look at this issue after you mentioned @antgamdia it in the standup:

but, that said, I wasn't able (yet) to test the streaming requests and as it turns out, it was using the installed package context namespace, rather than that of the resource.

This one-liner should fix it, though I'm keen to get the tests working for streaming requests to the fake k8s client too (need to add a watch reactor or something, which I think Greg may have done in some of his code already).

Signed-off-by: Michael Nelson <minelson@vmware.com>
Copy link
Contributor

@antgamdia antgamdia left a comment

Choose a reason for hiding this comment

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

+1, though I did perform this same change yesterday with no luck. I think something else is failing, but still don't know what it is.
Edit: perhaps has sth to do with resourceRefsEqual, but I tried removing the namespace from the function, but still no success.

@antgamdia
Copy link
Contributor

Good news, it seems it was a problem on my side (I wasn't fetching the k8s resources with the proper namespace).
Locally, with your changes and mine:

image

@absoludity
Copy link
Contributor Author

Good news, it seems it was a problem on my side (I wasn't fetching the k8s resources with the proper namespace).
Locally, with your changes and mine:

Sweet. Yeah, I was going to say, the only other possibility could be the namespace on the resource ref. Great.

@absoludity absoludity merged commit 39ffaba into master Jan 11, 2022
@absoludity absoludity deleted the fix-get-resource-other-namespace branch January 11, 2022 22:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants