Skip to content

engine: internal representation of pod (store.pod) contains info on multiple containers [ch3006]#1908

Merged
maiamcc merged 6 commits into
masterfrom
maiamcc/all-containers-on-pod
Jul 25, 2019
Merged

engine: internal representation of pod (store.pod) contains info on multiple containers [ch3006]#1908
maiamcc merged 6 commits into
masterfrom
maiamcc/all-containers-on-pod

Conversation

@maiamcc

@maiamcc maiamcc commented Jul 24, 2019

Copy link
Copy Markdown
Contributor

Currently when we populate a *store.Pod, we bless a single container and collect info on that container (see code).

First step to enabling us to LiveUpdate multiple containers is to have info for all (relevant) containers on the store.Pod.

This PR only changes structs/data storage -- all logic continues to use info from a single blessed container, to minimize the size of this change.

@maiamcc
maiamcc force-pushed the maiamcc/all-containers-on-pod branch from 2bf215e to 51958c8 Compare July 24, 2019 20:29
@maiamcc
maiamcc marked this pull request as ready for review July 24, 2019 21:54
@maiamcc
maiamcc requested review from jazzdan and nicks July 24, 2019 21:54
Comment thread internal/engine/pod.go Outdated

func getBlessedContainerID(iTargets []model.ImageTarget, pod *v1.Pod) (container.ID, error) {
if len(iTargets) > 0 {
fmt.Println("~~~ GETTING BLESSED CONTAINER ID ~~~")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

👀

Comment thread internal/engine/pod.go Outdated
manifest.Name, podInfo.PodID)
}

fmt.Printf("Container %s (%s) is blessed? %t\n", cName, cID, cID == blessedCID)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

👀

Comment thread internal/engine/pod.go Outdated
cID, err := k8s.ContainerIDFromContainerStatus(cStatus)
if err != nil {
logger.Get(ctx).Debugf("Error parsing container ID: %v", err)
return

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

should this be a continue rather than a return?

alternatively, it might be worth moving this to a helper function so that it's harder to make this mistake

Comment thread internal/engine/pod.go Outdated
for _, iTarget := range iTargets {
cStatus, err := k8s.ContainerMatching(pod, container.NameSelector(iTarget.DeploymentRef))
if err != nil {
fmt.Println("~~~ Something went wrong!")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

👀

Comment thread internal/engine/pod.go Outdated
iTarget.DeploymentRef.String())
}
if cStatus.Name != "" {
fmt.Println("~~~ Got it!", cStatus.ContainerID)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@maiamcc
maiamcc merged commit 42319e8 into master Jul 25, 2019
@maiamcc
maiamcc deleted the maiamcc/all-containers-on-pod branch July 25, 2019 18:11
maiamcc pushed a commit that referenced this pull request Jul 26, 2019
…r ports' when trying to port fwd (tho port forwarding still worked)
maiamcc pushed a commit that referenced this pull request Jul 29, 2019
…r ports' when trying to port fwd (tho port forwarding still worked) (#1924)
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.

2 participants