kubelet/rkt: Add routines for converting kubelet pod to rkt pod. #7543
Conversation
|
This follows #7465 |
| CAP_AUDIT_READ | ||
| ) | ||
|
|
||
| var capalibityList = map[int]string{ |
|
Needs a rebase btw |
|
Rebased @vmarmol |
| func getCapabilities(caps []api.CapabilityType) string { | ||
| var capList []string | ||
| for _, cap := range caps { | ||
| capList = append(capList, fmt.Sprintf("%q", cap)) |
There was a problem hiding this comment.
Won't this just print out the integer value? I think we need to do the lookup in the map above
There was a problem hiding this comment.
This is the api.CabapilityType, which is a string.
Actually we should have the capability list defined in kubelet. Let me TODO it :)
There was a problem hiding this comment.
Ah I see, yes you're right.
| "github.com/GoogleCloudPlatform/kubernetes/pkg/api" | ||
| ) | ||
|
|
||
| // TODO(yifan): Export this to higher level package. |
There was a problem hiding this comment.
Do we use this anywhere? It seems like we always use the string representation no?
There was a problem hiding this comment.
@vmarmol I mean we should define those string somewhere. At least it can help us check the user input in pod spec?
|
@vmarmol Thanks for the review! Comments addressed. Probably need another look :) |
| for _, m := range c.VolumeMounts { | ||
| mountPointName, err := appctypes.NewACName(m.Name) | ||
| if err != nil { | ||
| glog.Errorf("rkt: Cannot use the volume mount's name %q as ACName: %v", m.Name, err) |
There was a problem hiding this comment.
I'd argue we shouldn't log here (and below) since we are returning the error. We can make a new error with more data and let higher layers log if they'd like
| } | ||
| hash, err := appctypes.NewHash(imageID) | ||
| if err != nil { | ||
| glog.Errorf("rkt: Cannot create new hash from %q", imageID) |
There was a problem hiding this comment.
Yeah we probably should not log and return an error everywhere
|
This looks good. I am unsure about some of the detail of the rkt config setup, but we can iterate on those with future PRs. Good to merge after the nits above. |
|
@vmarmol Thanks! You are right, I changed all the logs with returning new errors |
|
LGTM, thanks @yifan-gu! |
|
Will need a rebase @yifan-gu |
|
Roger @vmarmol |
yifan-gu
changed the title
|
Merging as this is green, thanks @yifan-gu! |
kubelet/rkt: Add routines for converting kubelet pod to rkt pod.
@vmarmol @dchen1107 @jonboulle
This creates the rkt pod manifest from k8s pod spec.
There are some places where
RunContainerOptionsshould be used instead of reading the container's spec directly. But that requires some refactor on theRunContainerOptions(such as add volume/port names), so I leave a TODO for now.