-
Notifications
You must be signed in to change notification settings - Fork 77
e2e: add logLevel tests for MetalLB core components #526
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
Conversation
9907ea4 to
fed73b1
Compare
oribon
left a comment
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! a few small comments
test/e2e/functional/tests/e2e.go
Outdated
| err = testclient.Client.Get(context.Background(), goclient.ObjectKey{Namespace: metallb.Namespace, Name: metallb.Name}, metallb) | ||
| if !errors.IsNotFound(err) { | ||
| Expect(err).ToNot(HaveOccurred()) | ||
| metallbutils.DeleteAndCheck(metallb) | ||
| // Get a fresh object after deletion to avoid ResourceVersion issues | ||
| metallb, err = metallbutils.Get(OperatorNameSpace, UseMetallbResourcesFromFile) | ||
| Expect(err).ToNot(HaveOccurred()) | ||
| } |
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.
can we have this logic (starting from a clean state) in a BeforeEach?
test/e2e/functional/tests/e2e.go
Outdated
| } | ||
|
|
||
| Expect(testclient.Client.Create(context.Background(), metallb)).Should(Succeed()) | ||
| defer metallbutils.DeleteAndCheck(metallb) |
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.
can we clean in AfterEach instead of this defer?
test/e2e/functional/tests/e2e.go
Outdated
| By("Fetch controller pods from metallb-system namespace") | ||
| controllerPods, err := testclient.Client.Pods(OperatorNameSpace).List(context.Background(), metav1.ListOptions{ | ||
| LabelSelector: "component=controller", | ||
| }) | ||
| Expect(err).ToNot(HaveOccurred(), "Failed to list controller pods") | ||
| Expect(len(controllerPods.Items)).Should(BeNumerically(">", 0), "Controller Pods List should not be empty") |
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.
can we have this (and the speaker one) in a func?
fed73b1 to
c3dc1e8
Compare
|
@oribon I have addressed your comments. Also i updated webhook error message for bgpBackend. Please check. |
can you put it in a different commit please? 😅 |
test/e2e/metallb/metallb.go
Outdated
|
|
||
| // GetMetalLBPods fetches pods for a given MetalLB component (controller or speaker) | ||
| // and validates that the list is not empty | ||
| func GetMetalLBPods(namespace string, component string) *v1.PodList { |
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.
split this to two functions (speaker, controller)
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. ptal
9c76614 to
d6f3a4a
Compare
Updated |
82f942b to
6e93720
Compare
- Add tests to verify logLevel default and debug Signed-off-by: Anvesh J <[email protected]>
Signed-off-by: Anvesh J <[email protected]>
5c1e08d to
90eea2d
Compare
oribon
left a comment
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.
lgtm
Is this a BUG FIX or a FEATURE ?: add new e2e test case
What this PR does / why we need it: Migrating two tests from https://github.com/rh-ecosystem-edge/eco-gotests/blob/074dcb73d119b91a6b01dff02e4998bfe50db0e7/tests/cnf/core/network/metallb/tests/bgp-tests.go#L144-L180
Special notes for your reviewer:
Release note: