-
Notifications
You must be signed in to change notification settings - Fork 94
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
WIP feat(collector): improve remote host collector speed #1673
Conversation
pkg/supportbundle/collect.go
Outdated
results[file] = []byte(data) | ||
} | ||
|
||
time.Sleep(1 * time.Second) |
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.
Why this sleep?
pkg/supportbundle/collect.go
Outdated
} | ||
|
||
// wait for log stream to catch up | ||
time.Sleep(1 * time.Second) |
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.
Sleep isn't great here. Better to wait for the object. This will be brittle
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.
Thank you! Agree, I am trying to use polling method. However, the previous PR will be refactored. This PR will be on hold.
3c8db66
to
3f7b532
Compare
pkg/supportbundle/collect.go
Outdated
for _, pod := range pods.Items { | ||
eg.Go(func() error { | ||
// TODO: set timeout waiting | ||
if err := waitForPodRunning(ctx, clientset, &pod); err != nil { | ||
return err | ||
} | ||
|
||
results := map[string][]byte{} | ||
for _, collectorSpec := range hostCollectors { | ||
collector, ok := collect.GetHostCollector(collectorSpec, bundlePath) |
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.
Is the reporting and the progress updates accurate if the flow is:
for pod in pods:
run each collector
or should it be:
for collector in collectors:
run in each pod
Description, Motivation and Context
Please include a summary of the change or what problem it solves. Please also include relevant motivation and context.
Checklist
Does this PR introduce a breaking change?