Skip to content

Add Pod Status when pod is pending by going through pending container#2932

Merged
tejal29 merged 4 commits intoGoogleContainerTools:masterfrom
tejal29:add_pod_test
Oct 9, 2019
Merged

Add Pod Status when pod is pending by going through pending container#2932
tejal29 merged 4 commits intoGoogleContainerTools:masterfrom
tejal29:add_pod_test

Conversation

@tejal29
Copy link
Copy Markdown
Contributor

@tejal29 tejal29 commented Sep 24, 2019

Relates to #176

Add Pod Status when pod is pending by going through pending container.

Description

User facing changes

n/a

Next PRs.

Use this #2928

Submitter Checklist

These are the criteria that every PR should meet, please check them off as you
review them:

  • Includes unit tests
  • n/a Mentions any output changes.
  • n/a Adds documentation as needed: user docs, YAML reference, CLI reference.
  • n/a Adds integration tests if needed.

Reviewer Notes

  • The code flow looks good.
  • Unit test added.
  • User facing changes look good.

Release Notes

@codecov
Copy link
Copy Markdown

codecov bot commented Sep 24, 2019

Codecov Report

Merging #2932 into master will increase coverage by 0.03%.
The diff coverage is 77.77%.

Impacted Files Coverage Δ
pkg/skaffold/kubernetes/status_check.go 77.77% <77.77%> (ø)

}

func GetPodDetails(client kubernetes.Interface, ns string, podName string) PodStatus {
pi := client.CoreV1().Pods(ns)
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.

Suggested change
pi := client.CoreV1().Pods(ns)
pods := client.CoreV1().Pods(ns)

Copy link
Copy Markdown
Contributor

@balopat balopat left a comment

Choose a reason for hiding this comment

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

This looks good to me.
I think we should add the same logic though for EphemeralContainerStatuses and InitContainerStatuses by appending them to the ContainerStatuses.

@balopat balopat changed the title Add Pod Stauts when pod is pending by going through pending container Add Pod Status when pod is pending by going through pending container Oct 4, 2019
@tejal29
Copy link
Copy Markdown
Contributor Author

tejal29 commented Oct 7, 2019

@balopat please take a look again!

default:
reason, detail := getWaitingContainerStatus(pod.Status.ContainerStatuses)
return newPendingStatus(reason, detail)

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.

nit: remove newline

}

func (e *PodErr) Error() string {
return fmt.Sprintf("pod in error due to reason: %s, message: %s", e.reason, e.message)
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.

this error message is a bit confusing to me. what is the reason vs the message? like shouldn't the message implicitly have the reason that the pod is failing?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Reason is the error code and message is Message explaining Reason.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

an example would be "Reasan: ErrImgPull", Message: "image:tag could not be fetched"
Just Reason is not enough.

@tejal29 tejal29 merged commit b747758 into GoogleContainerTools:master Oct 9, 2019
@tejal29 tejal29 deleted the add_pod_test branch April 15, 2021 07:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants