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

Add an event for health check failures. #4408

Merged
merged 2 commits into from
Feb 13, 2015

Conversation

brendandburns
Copy link
Contributor

No description provided.

@@ -1077,6 +1077,12 @@ func (kl *Kubelet) syncPod(pod *api.BoundPod, dockerContainers dockertools.Docke
containersToKeep[containerID] = empty{}
continue
}
ref, ok := kl.getRef(containerID)
if !ok {
glog.Warningf("No ref for pod '%v' - '%v'", ID, name)
Copy link
Member

Choose a reason for hiding this comment

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

This can't possibly even compile.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I blame copy and paste. fixed.

@brendandburns
Copy link
Contributor Author

comments addressed. ptal.

if !ok {
glog.Warningf("No ref for pod '%v' - '%v'", ID, name)
} else {
record.Eventf(ref, "unhealthy", "Health Check Failed %v - %v", ID, name)
Copy link
Member

Choose a reason for hiding this comment

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

Please don't use the term "Health Check". We now have both liveness and readiness probes in the API. You're reporting that the liveness probe failed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done.

@bgrant0607 bgrant0607 self-assigned this Feb 13, 2015
@bgrant0607
Copy link
Member

Please see my now-hidden comment: #4408 (comment)

@brendandburns
Copy link
Contributor Author

oops, comments crossed paths on the wire. addressed.

@bgrant0607
Copy link
Member

LGTM

@bgrant0607 bgrant0607 added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 13, 2015
saad-ali added a commit that referenced this pull request Feb 13, 2015
Add an event for health check failures.
@saad-ali saad-ali merged commit c112f8f into kubernetes:master Feb 13, 2015
@saad-ali
Copy link
Member

@brendandburns Out of an abundance of caution I reverted this change because it caused v1.3 to fail on both Shippable and Travis. Rerunning, Travis is green, but Shippable v1.3 is still failing. The error seems transient. Could you squash the commits and resubmit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm "Looks good to me", indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants