Skip to content
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 podInfo. #7555

Merged
merged 1 commit into from
May 1, 2015
Merged

Conversation

yifan-gu
Copy link
Contributor

@vmarmol @dchen1107 @jonboulle @bakins

Another shot following #7465

This adds podInfo, which represents the rkt pod's state, and is used for reporting pod/container status to kubelet.

Think we have about three more PRs after this for implementing syncPod for rkt!

@vmarmol vmarmol self-assigned this Apr 30, 2015
status.PodIP = p.getIP()
// For now just make every container's state as same as the pod.
for _, container := range pod.Containers {
status.ContainerStatuses = append(status.ContainerStatuses, p.getContainerStatus(container))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this actually just using the pod status? Looks like we do get per-container status

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Containers are sharing fate at this moment, so they have the same status. This is something under hot discussion. Should be more clearer in the following weeks:
appc/spec#276

state string
// The ip of the pod. IPv4 for now.
ip string
networkInfo string
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is unused

@yifan-gu yifan-gu force-pushed the rkt_pod_info branch 3 times, most recently from 8c50615 to 74f1452 Compare April 30, 2015 20:19
@yifan-gu
Copy link
Contributor Author

@vmarmol Rebased and also moved getPodInfos to pod_info.go


// splitLineByTab breaks a line by tabs, and trims the leading and tailing spaces.
// Not using strings.Split because there could be multiple tabs.
func splitLineByTab(line string) []string {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alternatively: Split on tabs and remove all empty strings in the slice. May be simpler to implement and reason about.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 👍

podInfo reprensents the state of rkt pods.
It is used to for reporting the pod and container status to kubelet.
@yifan-gu
Copy link
Contributor Author

Thanks for the thorough review! @vmarmol !

@bakins
Copy link

bakins commented Apr 30, 2015

LGTM

@vmarmol
Copy link
Contributor

vmarmol commented Apr 30, 2015

LGTM, thanks for the quick changes @yifan-gu!

@yifan-gu yifan-gu changed the title Rkt pod info kubelet/rkt: Add podInfo. Apr 30, 2015
type containerID struct {
uuid string // uuid of the pod.
appName string // name of the app in that pod.
imageID string // id of the image. TODO(yifan): Depreciate this.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit, deprecate

@jonboulle
Copy link
Contributor

Good to go on this one?

@vmarmol
Copy link
Contributor

vmarmol commented Apr 30, 2015

@jonboulle yep, just waiting on the CI (which is taking so much longer than I'd like :( )

@vmarmol
Copy link
Contributor

vmarmol commented May 1, 2015

It's green!

vmarmol added a commit that referenced this pull request May 1, 2015
@vmarmol vmarmol merged commit df8490f into kubernetes:master May 1, 2015
@yifan-gu
Copy link
Contributor Author

yifan-gu commented May 1, 2015

awesome!

@yifan-gu yifan-gu deleted the rkt_pod_info branch May 7, 2015 17:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants