Skip to content

Skip validating domain list ability if operator has already read, created, or updated CRD #2772

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

Merged
merged 1 commit into from
Feb 14, 2022

Conversation

rjeberhard
Copy link
Member

We have functionality in Main that trails the behavior in CRDHelper and additionally does a list of domains to validate the the custom type is available. Oddly, I could consistently reproduce getting a 404 trying to list domains immediately after creating the CRD. This meant that most customers would see a SEVERE message in the operator's log when the operator was started pointing at a fresh cluster.

Looking at the requirement, we added the domain list as a CRD "presence check" in response to use cases where the operator did not have privilege to read the CRD. We don't actually need this extra check if the operator was able to read, create, or update the CRD.

This PR skips the presence check if the operator was able to read, create, or update the CRD.

@ankedia
Copy link
Member

ankedia commented Feb 11, 2022

Oddly, I could consistently reproduce getting a 404 trying to list domains immediately after creating the CRD. This meant that most customers would see a SEVERE message in the operator's log when the operator was started pointing at a fresh cluster.
[ANIL] My understanding is that listDomainAsync call will return an empty list if CRD is installed but no domains are created. If so, can we have an explict check for the 404 (HTTP_NOT_FOUND) response code and not log the SEVERE error in that case?

[Response from Ryan] The issue was that we were installing the CRD and then immediately calling listDomainAsync -- this was resulting in a 404, which is the result you get when the CRD is not installed. There appears to be a timing window after the install of the CRD where listDomainAsync will return a 404 even though the CRD is now available.

Copy link
Member

@ankedia ankedia left a comment

Choose a reason for hiding this comment

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

LGTM

@lennyphan lennyphan self-requested a review February 11, 2022 21:48
@robertpatrick robertpatrick merged commit 0ab5e6b into main Feb 14, 2022
@rjeberhard rjeberhard deleted the owls-96304 branch March 22, 2022 19:06
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.

4 participants