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
Fix failing service e2e due to execPod unavailability #80805
Conversation
/area conformance |
/remove-area conformance |
test/e2e/network/service.go
Outdated
// Make sure acceptPod is running. There are certain chances that pod might be teminated due to unexpected reasons. | ||
acceptPod, err = cs.CoreV1().Pods(namespace).Get(acceptPod.Name, metav1.GetOptions{}) | ||
framework.ExpectNoError(err, "Unable to get pod %s", acceptPod.Name) | ||
gomega.Expect(acceptPod.Status.Phase).To(gomega.Equal(v1.PodRunning)) |
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.
We can replace here with framework.ExpectEqual(acceptPod.Status.Phase, v1.PodRunning)
.
After merging #80785 this line will be blocked.
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.
Thanks for pointing.
Done.
test/e2e/network/service.go
Outdated
acceptPod, err = cs.CoreV1().Pods(namespace).Get(acceptPod.Name, metav1.GetOptions{}) | ||
framework.ExpectNoError(err, "Unable to get pod %s", acceptPod.Name) | ||
gomega.Expect(acceptPod.Status.Phase).To(gomega.Equal(v1.PodRunning)) | ||
gomega.Expect(acceptPod.Status.PodIP).ToNot(gomega.Equal("")) |
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.
ditto: can replace here with framework.ExpectNotEqual(acceptPod.Status.PodIP, "")
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.
Done.
test/e2e/network/service.go
Outdated
// Make sure dropPod is running. There are certain chances that the pod might be teminated due to unexpected reasons. dropPod, err = cs.CoreV1().Pods(namespace).Get(dropPod.Name, metav1.GetOptions{}) | ||
dropPod, err = cs.CoreV1().Pods(namespace).Get(dropPod.Name, metav1.GetOptions{}) | ||
framework.ExpectNoError(err, "Unable to get pod %s", dropPod.Name) | ||
gomega.Expect(dropPod.Status.Phase).To(gomega.Equal(v1.PodRunning)) |
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.
ditto
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.
Done.
test/e2e/network/service.go
Outdated
dropPod, err = cs.CoreV1().Pods(namespace).Get(dropPod.Name, metav1.GetOptions{}) | ||
framework.ExpectNoError(err, "Unable to get pod %s", dropPod.Name) | ||
gomega.Expect(dropPod.Status.Phase).To(gomega.Equal(v1.PodRunning)) | ||
gomega.Expect(dropPod.Status.PodIP).ToNot(gomega.Equal("")) |
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.
ditto
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.
Done.
|
||
deployment, err = cs.AppsV1().Deployments(namespace).Get(deployment.Name, metav1.GetOptions{}) | ||
framework.ExpectNoError(err, "Error in retriving pause pod deployment") | ||
labelSelector, err := metav1.LabelSelectorAsSelector(deployment.Spec.Selector) |
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.
We might want to put framework.ExpectNoError(err)
for the previous call.
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.
I was not clear with this suggestion but added framework.ExpectNoError(err)
for metab1.LabelSelector statement along with other effective change in error checking statements.
/cc @oomichi |
2a8266d
to
bf1597f
Compare
/priority important-soon |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: mgdevstack, spiffxp The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/retest Review the full test history for this PR. Silence the bot with an |
/test pull-kubernetes-integration |
/retest Review the full test history for this PR. Silence the bot with an |
Hey @mgdevstack, may I ask you to modify the PR to rename the release notes entry from |
What type of PR is this?
/kind failing-test
What this PR does / why we need it:
Fixes following failing E2E (Release-master blocking)
https://testgrid.k8s.io/sig-release-master-blocking#gce-cos-master-slow&width=20
Issue Source: #80760 (comment)
Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
For this E2E,
ginkgo.Expect
checks are added to make sure Pods are in desired state. Incase, execPods are find to be terminated or not running with no PodIP, we will be introducing deployment to make sure execPods are in desired running state.E2E failing reasons are added into network service e2e tests failing #80760 (comment)
Does this PR introduce a user-facing change?:
Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.: