occm: convert bash based lbaas e2e tests into go tests - #3153
Conversation
kubernetes-prow
Bot
added
the
release-note-none
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
kubernetes-prow
Bot
added
the
cncf-cla: yes
kubernetes-prow
Bot
added
the
size/XXL
5216215 to
68d2acb
Compare
68d2acb to
e235e91
Compare
| gomega.Expect(err).NotTo(gomega.HaveOccurred()) | ||
|
|
||
| err = createNamespace(ginkgo.GinkgoT().Context(), tstCtx) | ||
| gomega.Expect(err).NotTo(gomega.HaveOccurred()) | ||
|
|
||
| err = createDeployment(ginkgo.GinkgoT().Context(), tstCtx) | ||
| gomega.Expect(err).NotTo(gomega.HaveOccurred()) | ||
| }) | ||
|
|
||
| ginkgo.AfterEach(func() { | ||
| if autoCleanup { | ||
| cleanupResources(ginkgo.GinkgoT().Context(), tstCtx) |
There was a problem hiding this comment.
If setupTestContext or createDeployment fails in BeforeEach, tstCtx is nil here and this will panic. You can add a guard like: if autoCleanup && tstCtx != nil {
| framework.Logf("Initial member ports: %v", initialMemberPorts) | ||
|
|
||
| // Validate member ports match NodePorts | ||
| gomega.Expect(len(initialMemberPorts)).To(gomega.Equal(len(initialNodePorts)), |
There was a problem hiding this comment.
The original bash test explicitly failed if the NodePort did not change after update (if [[ ${member_port} == ${member_ports} ]]). This version only logs. If the point of this test is to verify Kubernetes assigns a new NodePort and the LB member is updated to match it, so please restore the assertion
| } | ||
|
|
||
| _, err = tstCtx.k8sClient.CoreV1().Namespaces().Create(ctx, ns, metav1.CreateOptions{}) | ||
| if err != nil && !strings.Contains(err.Error(), "already exists") { |
There was a problem hiding this comment.
Use k8serrors.IsAlreadyExists(err) / k8serrors.IsNotFound(err) from k8s.io/apimachinery/pkg/api/errors instead of strings.Contains(err.Error(), "already exists"). String matching on error messages is fragile across API versions and locales
|
|
||
| ginkgo.BeforeEach(func() { | ||
| var err error | ||
| tstCtx, err = setupTestContext(ginkgo.GinkgoT().Context()) |
There was a problem hiding this comment.
BeforeEach re-authenticates to OpenStack and creates all clients before every single test. Move the client setup to BeforeSuite/AfterSuite and only reset createdServices/createdFIPs/createdLBs slices in BeforeEach. This will significantly reduce test setup time
| framework.Logf("Load balancer %s is ACTIVE", lbID) | ||
| return true, nil | ||
| } | ||
| } else { |
There was a problem hiding this comment.
http.DefaultClient has no timeout. A connection that is established but never returns data will hang the test indefinitely. It could be better to use &http.Client{Timeout: 10 * time.Second} for connectivity checks
| } | ||
|
|
||
| fileName := time.Now().Format("20060102-150405") + "-api-debug.log" | ||
| logFile, err := os.Create(filepath.Join(dir, fileName)) |
There was a problem hiding this comment.
logFile is opened here but never stored or closed. If the test process is killed, buffered log data may be lost. Store the *os.File in testContext and close it in cleanup, or use defer in initAPILogger with a registry in BeforeSuite
2 participants
What this PR does / why we need it:
old bash based tests are slow and not modular. new go-based tests are modular, fast and provide more logs for debugging.
Special notes for reviewers:
The original bash tests had a bug where OVN provider failures were silently ignored. When creating an HTTP listener with OVN (which only supports TCP), the
openstack loadbalancer listener createcommand would fail with HTTP 501, but the bash script didn't check the exit code and continued executing, causing the tests to pass despite the failure.Full logs: https://storage.googleapis.com/kubernetes-ci-logs/pr-logs/pull/cloud-provider-openstack/3154/openstack-cloud-controller-manager-ovn-e2e-test-release-136/2082483820640079872/build-log.txt
Release note: