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
kubelet/rkt: Add SyncPod() to rkt. #7611
Conversation
uid := pod.UID | ||
|
||
if len(runningPod.Containers) == 0 { | ||
glog.V(4).Infof("Pod is not running, start it") |
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.
does the running pod has an identity/description at this point? The log is very vague.
Need work around pod volumes. |
readinessManager *kubecontainer.ReadinessManager | ||
prober prober.Prober | ||
systemd *dbus.Conn | ||
absPath string |
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.
nit: I know this was here before, but can we document absPath?
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.
Sure, the name is confusing actually :(
Looks good overall, couple of things to node
What's missing from the volume stuff? |
@vmarmol Thanks! Volumes are required for launching a pod since now Im not using |
The ports yes, the network mode no since it is as the pod level. I'm fine doing the volumes later. You're already injecting the generator can you use that? |
@vmarmol That requires some modification to the RunPod(), I will figure out. |
@vmarmol, @yifan-gu, do we still want to keep
|
@yujuhong We can delete it I think! Thanks for the reminder |
Have kicked the volumes out from the rkt's syncPod()'s signature. Now volume gets unsupported, but I will use RunContainerOptions soon. There are pumps in rkt's syncPod now, which I will figure out today or tomorrow. |
195940a
to
1b77615
Compare
Thanks @vmarmol ! |
kubelet/rkt: Add SyncPod() to rkt.
@vmarmol @yujuhong @dchen1107 @jonboulle @bakins
Currently, if any containers in the pod need to be restarted, the whole pod will be restarted.
WIP, need to move
shouldContainerBeRestarted
to hooks so we can reuse the code.