-
Notifications
You must be signed in to change notification settings - Fork 211
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
[kwokctl] Support k8s format for snapshot #381
Conversation
✅ Deploy Preview for k8s-kwok canceled.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: wzshiming 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 |
ce9d959
to
ea6f378
Compare
91116f9
to
7ef2f37
Compare
e828375
to
08d503d
Compare
08d503d
to
9ab2a0e
Compare
9ab2a0e
to
2cbe85c
Compare
2cbe85c
to
5a9e9e4
Compare
|
||
// Save saves the snapshot of cluster | ||
func Save(ctx context.Context, rt Runtime, w io.Writer, filters []string) error { | ||
err := rt.KubectlInCluster(exec.WithWriteTo(ctx, w), "get", strings.Join(filters, ","), "-o=yaml", "--all-namespaces") |
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.
in practice, it's likely to fail due to hitting QPS limits, right?
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've tried the latest kubectl, which supports chunking without failing even with very large amounts of data.
However, the QPS limit causes it to be really slow, so I will try to remove the client/server QPS limit in the future to make this work better.
--chunk-size 500 Return large lists in chunks rather than all at once. Pass 0 to disable. This flag is beta and may change in the future.
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.
Let's put it to this week's meeting discussion. My understanding is that --chunk-size
is not the same thing as "client-go's paged query utility" (where it can define pageSize and retry logic programmatically), so IMO the latter impl. is what we need to pursue.
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.
That's great, I have documented the relevant code links in advance as additional information for the discussion.
kubectl get
seems to load all the resources into memory and then output them together, so too much memory is used and it fails.
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.
Yes, that is correct. In my local fork, I utilized client-go's paged query to take a snapshot of the API objects. I had planned to contribute it back for really a while... hopefully I can find some time to make it soon.
What type of PR is this?
/kind feature
What this PR does / why we need it:
Which issue(s) this PR fixes:
Fixes #32
Special notes for your reviewer:
Does this PR introduce a user-facing change?
Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.: