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

Allow session affinity a period of time to setup for new services. #75073

Merged
merged 1 commit into from
Mar 9, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
22 changes: 8 additions & 14 deletions test/e2e/framework/service_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -1453,6 +1453,7 @@ type affinityTracker struct {
// Record the response going to a given host.
func (at *affinityTracker) recordHost(host string) {
at.hostTrace = append(at.hostTrace, host)
Logf("Received response from host: %s", host)
}

// Check that we got a constant count requests going to the same host.
Expand Down Expand Up @@ -1480,13 +1481,11 @@ func checkAffinityFailed(tracker affinityTracker, err string) {
}

// CheckAffinity function tests whether the service affinity works as expected.
// If affinity is expected and transitionState is true, the test will
// return true once affinityConfirmCount number of same response observed in a
// row. If affinity is not expected, the test will keep observe until different
// responses observed. The function will return false only when no expected
// responses observed before timeout. If transitionState is false, the test will
// fail once different host is given if shouldHold is true.
func CheckAffinity(jig *ServiceTestJig, execPod *v1.Pod, targetIp string, targetPort int, shouldHold, transitionState bool) bool {
// If affinity is expected, the test will return true once affinityConfirmCount
// number of same response observed in a row. If affinity is not expected, the
// test will keep observe until different responses observed. The function will
// return false only in case of unexpected errors.
func CheckAffinity(jig *ServiceTestJig, execPod *v1.Pod, targetIp string, targetPort int, shouldHold bool) bool {
targetIpPort := net.JoinHostPort(targetIp, strconv.Itoa(targetPort))
cmd := fmt.Sprintf(`wget -qO- http://%s/ -T 2`, targetIpPort)
timeout := ServiceTestTimeout
Expand All @@ -1510,13 +1509,8 @@ func CheckAffinity(jig *ServiceTestJig, execPod *v1.Pod, targetIp string, target
if !shouldHold && !affinityHolds {
return true, nil
}
if shouldHold {
if !transitionState && !affinityHolds {
Copy link
Contributor

Choose a reason for hiding this comment

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

(without knowing much of the codebase) -- does the removal of this case lead to lower coverage?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No. E2e test is behavior driven. And our expectation to the behavior doesn't match with the actual one. And this PR is the fix.

return true, fmt.Errorf("Affinity should hold but didn't.")
}
if trackerFulfilled && affinityHolds {
return true, nil
}
if shouldHold && trackerFulfilled && affinityHolds {
return true, nil
}
return false, nil
}); pollErr != nil {
Expand Down
12 changes: 6 additions & 6 deletions test/e2e/network/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -2200,17 +2200,17 @@ func execAffinityTestForNonLBService(f *framework.Framework, cs clientset.Interf
Expect(err).NotTo(HaveOccurred(), "failed to fetch pod: %s in namespace: %s", execPodName, ns)

if !isTransitionTest {
Expect(framework.CheckAffinity(jig, execPod, svcIp, servicePort, true, false)).To(BeTrue())
Expect(framework.CheckAffinity(jig, execPod, svcIp, servicePort, true)).To(BeTrue())
}
if isTransitionTest {
svc = jig.UpdateServiceOrFail(svc.Namespace, svc.Name, func(svc *v1.Service) {
svc.Spec.SessionAffinity = v1.ServiceAffinityNone
})
Expect(framework.CheckAffinity(jig, execPod, svcIp, servicePort, false, true)).To(BeTrue())
Expect(framework.CheckAffinity(jig, execPod, svcIp, servicePort, false)).To(BeTrue())
svc = jig.UpdateServiceOrFail(svc.Namespace, svc.Name, func(svc *v1.Service) {
svc.Spec.SessionAffinity = v1.ServiceAffinityClientIP
})
Expect(framework.CheckAffinity(jig, execPod, svcIp, servicePort, true, true)).To(BeTrue())
Expect(framework.CheckAffinity(jig, execPod, svcIp, servicePort, true)).To(BeTrue())
}
}

Expand Down Expand Up @@ -2240,16 +2240,16 @@ func execAffinityTestForLBService(f *framework.Framework, cs clientset.Interface
port := int(svc.Spec.Ports[0].Port)

if !isTransitionTest {
Expect(framework.CheckAffinity(jig, nil, ingressIP, port, true, false)).To(BeTrue())
Expect(framework.CheckAffinity(jig, nil, ingressIP, port, true)).To(BeTrue())
}
if isTransitionTest {
svc = jig.UpdateServiceOrFail(svc.Namespace, svc.Name, func(svc *v1.Service) {
svc.Spec.SessionAffinity = v1.ServiceAffinityNone
})
Expect(framework.CheckAffinity(jig, nil, ingressIP, port, false, true)).To(BeTrue())
Expect(framework.CheckAffinity(jig, nil, ingressIP, port, false)).To(BeTrue())
svc = jig.UpdateServiceOrFail(svc.Namespace, svc.Name, func(svc *v1.Service) {
svc.Spec.SessionAffinity = v1.ServiceAffinityClientIP
})
Expect(framework.CheckAffinity(jig, nil, ingressIP, port, true, true)).To(BeTrue())
Expect(framework.CheckAffinity(jig, nil, ingressIP, port, true)).To(BeTrue())
}
}