-
Notifications
You must be signed in to change notification settings - Fork 546
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
Verify CRD's condition to ensure it's registered with k8s API (rebased) #614
Verify CRD's condition to ensure it's registered with k8s API (rebased) #614
Conversation
OLM needs to verify CRD's condition to make sure it is registered with k8s API during CSV control loop before installing operator. This verification will help to avoid errors related to the unavailability of an API when installing an operator via OLM. Jira: https://jira.coreos.com/browse/ALM-775 Signed-off-by: Vu Dinh <vdinh@redhat.com>
This allows log level changes in the unit tests to be respected. Also, change operator groups default level to debug.
Copied from old PR: OLM needs to verify CRD's condition to make sure it is registered |
namespace, | ||
"", | ||
installStrategy("csv1-dep", nil, nil), | ||
[]*v1beta1.CustomResourceDefinition{crdWithoutStatus("c1", "v2")}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I'm missing what makes this CRD be unavailable? What's the difference between this test and the previous?
9f13db7
to
7a4aaf0
Compare
Change the default `crd` function to return appropriate status as to pass the requirement checks. Also, make some minor logging improvements.
/retest |
/retest |
1 similar comment
/retest |
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jpeeler The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
), | ||
existingObjs: nil, | ||
existingExtObjs: []runtime.Object{ | ||
func() *v1beta1.CustomResourceDefinition { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool!
/lgtm |
/retest |
2 similar comments
/retest |
/retest |
Verify CRD's condition to ensure it's registered with k8s API (rebased)
This is Vu's #553, but rebased and with tests passing (should be at least!). It has been modified from his original code slightly -
mainly instead of creating a crdWithStatus, instead uses crdWithoutStatus which required far less code modification.