Add e2e test which verifies traffic policies and firewall in services #10972
Conversation
manuelbuil
force-pushed
the
addTestAdvanceFeatureServices
branch
2 times, most recently
from
fbabd3b
to
bb73757
Compare
manuelbuil
changed the title
manuelbuil
changed the title
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #10972 +/- ##
==========================================
- Coverage 50.06% 44.10% -5.96%
==========================================
Files 178 178
Lines 14801 14814 +13
==========================================
- Hits 7410 6534 -876
- Misses 6044 7077 +1033
+ Partials 1347 1203 -144
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
manuelbuil
force-pushed
the
addTestAdvanceFeatureServices
branch
2 times, most recently
from
975da3f
to
bc30eee
Compare
manuelbuil
force-pushed
the
addTestAdvanceFeatureServices
branch
from
bc30eee
to
2031570
Compare
| var nodeOS = flag.String("nodeOS", "bento/ubuntu-24.04", "VM operating system") | ||
| var serverCount = flag.Int("serverCount", 1, "number of server nodes") | ||
| var agentCount = flag.Int("agentCount", 1, "number of agent nodes") | ||
| var hardened = flag.Bool("hardened", false, "true or false") |
There was a problem hiding this comment.
Not used.
| var hardened = flag.Bool("hardened", false, "true or false") |
| Eventually(func() (int, error) { | ||
| externalIPs, _ := e2e.FetchExternalIPs(kubeConfigFile, lbSvc) | ||
| return len(externalIPs), nil | ||
| }, "25s", "5s").Should(Equal(2), "external IP count not equal to 2") |
There was a problem hiding this comment.
Your ignoring the error state that could occur from FetchExternalIPs but Ginkgo can handle that.
| Eventually(func() (int, error) { | |
| externalIPs, _ := e2e.FetchExternalIPs(kubeConfigFile, lbSvc) | |
| return len(externalIPs), nil | |
| }, "25s", "5s").Should(Equal(2), "external IP count not equal to 2") | |
| Eventually(func() ([]string, error) { | |
| return e2e.FetchExternalIPs(kubeConfigFile, lbSvc) | |
| }, "25s", "5s").Should(HaveLen(2), "external IP count not equal to 2") |
There was a problem hiding this comment.
Thanks, I did not know about the HaveLen
| @@ -0,0 +1,363 @@ | |||
| // This test verifies: | |||
There was a problem hiding this comment.
Note: I hand wrote these corrections, so spacing might be bad, don't just commit them via GH 😄
| Eventually(func() (bool, error) { | ||
| externalIPs, _ := e2e.FetchExternalIPs(kubeConfigFile, lbSvcExt) | ||
| if len(externalIPs) != 1 { | ||
| return false, nil | ||
| } | ||
| return externalIPs[0] == serverNodeIP, nil | ||
| }, "25s", "5s").Should(BeTrue(), "external IP count not equal to 1 or external IP does not match serverNodeIP") |
There was a problem hiding this comment.
| Eventually(func() (bool, error) { | |
| externalIPs, _ := e2e.FetchExternalIPs(kubeConfigFile, lbSvcExt) | |
| if len(externalIPs) != 1 { | |
| return false, nil | |
| } | |
| return externalIPs[0] == serverNodeIP, nil | |
| }, "25s", "5s").Should(BeTrue(), "external IP count not equal to 1 or external IP does not match serverNodeIP") | |
| Eventually(func(g Gomega) (bool, error) { | |
| externalIPs, _ := e2e.FetchExternalIPs(kubeConfigFile, lbSvcExt) | |
| g.Expect(externalIPs).To(HaveLen(1), "more than 1 exernalIP found") | |
| g.Expect(externalIPs[0]).To(Equal(serverNodeIP),"external IP does not match servernodeIP") | |
| }, "25s", "5s").Should(Succeed()) |
There was a problem hiding this comment.
We can use the gomega Assertions In Eventually feature to standardize the checks
There was a problem hiding this comment.
thanks! Your suggestion looks better :)
| Eventually(func() (bool) { | ||
| cmd := "curl -s --max-time 5 " + externalIP + ":82/ip" | ||
| _, err := e2e.RunCommand(cmd) | ||
| if err != nil && strings.Contains(err.Error(), "exit status") { | ||
| // Treat exit status as a successful condition | ||
| return true | ||
| } | ||
| return false | ||
| }, "40s", "5s").Should(BeTrue()) |
There was a problem hiding this comment.
| Eventually(func() (bool) { | |
| cmd := "curl -s --max-time 5 " + externalIP + ":82/ip" | |
| _, err := e2e.RunCommand(cmd) | |
| if err != nil && strings.Contains(err.Error(), "exit status") { | |
| // Treat exit status as a successful condition | |
| return true | |
| } | |
| return false | |
| }, "40s", "5s").Should(BeTrue()) | |
| Eventually(func() error { | |
| cmd := "curl -s --max-time 5 " + externalIP + ":82/ip" | |
| _, err := e2e.RunCommand(cmd) | |
| return err | |
| }, "40s", "5s").Should(MatchError(ContainSubstring("exit status"))) |
| return serverNodeName != "" && clientPod1 != "" && clientPod2 != "" && clientPod1Node != "" && clientPod2Node != "", nil | ||
| }, "25s", "5s").Should(BeTrue(), "All pod names and nodes should be non-empty") |
There was a problem hiding this comment.
You should be able to rewrite this using the g Gomega feature like the above comment, don't embed a complex series of checks as a single return statement, have each one be check with some g.Expect(serverNodeName).NotTo(BeEmpty) etc.
There was a problem hiding this comment.
When passing func(g Gomega) and checking variables along the function, there isn't really a need to return anything, right?
| Eventually(func() (bool) { | ||
| cmd := "curl -s --max-time 5 " + node.InternalIP + ":82" | ||
| _, err := e2e.RunCmdOnNode(cmd, nodes[1].Name) | ||
| if err != nil && strings.Contains(err.Error(), "exit status") { | ||
| // Treat exit status as a successful condition | ||
| return true | ||
| } | ||
| return false | ||
| }, "40s", "5s").Should(BeTrue()) |
There was a problem hiding this comment.
Same as above, rewrite using the gomega error matcher checks
Signed-off-by: manuelbuil <[email protected]>
manuelbuil
force-pushed
the
addTestAdvanceFeatureServices
branch
from
2031570
to
3be3ae2
Compare
Proposed Changes
New e2e test to verify services externaltrafficpolicy, internaltrafficpolicy and loadBalancerSourceRanges
Types of Changes
New e2e test
Verification
Run go test
Testing
Linked Issues
User-Facing Change
Further Comments