-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
generate kube with volumes #3472
generate kube with volumes #3472
Conversation
Signed-off-by: Peter Hunt <[email protected]>
Signed-off-by: Peter Hunt <[email protected]>
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: haircommander, mheon 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 |
} | ||
kubeContainer.VolumeDevices = devices | ||
return kubeContainer, errors.Wrapf(define.ErrNotImplemented, "linux devices") | ||
return kubeContainer, kubeVolumes, errors.Wrapf(define.ErrNotImplemented, "linux devices") |
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.
Wait, what's going on here? We're getting devices... and then throwing an error that they're not supported?
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.
Should this just be a warning?
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.
TODO Enable when we can support devices and their names
I assume the translation isn't clear for libpod devices to kube devices?
6919823
to
8b23572
Compare
for containers that share volumes, so the pod section doesn't list copies Signed-off-by: Peter Hunt <[email protected]>
8b23572
to
ac1bdcb
Compare
/test images |
Signed-off-by: Peter Hunt <[email protected]>
Specifically, we were needlessly doing a double lookup to find which config mounts were user volumes. Improve this by refactoring a bit of code from inspect Signed-off-by: Peter Hunt <[email protected]>
ac1bdcb
to
aeabc45
Compare
Changes LGTM |
@baude @jwhonce @rhatdan @TomSweeneyRedHat @vrothberg Can we get this landed? I'm messing around in Inspect and I don't want to make merge conflicts... |
Changes LGTM, if you're happy with the one question you had @mheon, feel free to merge it. |
/lgtm |
Add volume functionality for generate kube, including adding the fields for containers, as well as the pod. Test added as well
Fixes: #2303