-
Notifications
You must be signed in to change notification settings - Fork 542
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
adds paginating lister for evaluating CRs' upgrade fitness versus new CRDs. #3387
Conversation
Signed-off-by: Jordan Keister <[email protected]>
for _, cr := range crList.Items { | ||
err = validation.ValidateCustomResource(field.NewPath(""), cr.UnstructuredContent(), validator).ToAggregate() | ||
pager := pager.New(pager.SimplePageFunc(func(opts metav1.ListOptions) (runtime.Object, error) { | ||
return dynamicClient.Resource(gvr).List(context.TODO(), metav1.ListOptions{}) |
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.
return dynamicClient.Resource(gvr).List(context.TODO(), metav1.ListOptions{}) | |
return dynamicClient.Resource(gvr).List(context.TODO(), opts) |
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.
The ListOptions
argument opts
isn't being used...
Presumably that's how it's paging/restricting output?
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.
Resolved in ca452be
Signed-off-by: Jordan Keister <[email protected]>
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.
/lgtm
48a856a
LGTM, test pass: https://issues.redhat.com/browse/OCPBUGS-35358 |
Description of the change:
adds a paging lister for listing CRs deployed in the cluster, used for fitness testing new CRD versions during upgrade.
Motivation for the change:
scaling issues in clusters where listing CRs for an operator would exceed apiserver timeouts
Architectural changes:
since previously we attempted to load the list of all related CRs into memory, this should also significantly help with memory usage by OLM during upgrade evaluation.
Testing remarks:
Reviewer Checklist
/doc
[FLAKE]
are truly flaky and have an issue