add e2e test for networkpolicy of openshift-config-operator#30910
add e2e test for networkpolicy of openshift-config-operator#30910zhouying7780 wants to merge 1 commit intoopenshift:mainfrom
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: automatic mode |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: zhouying7780 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 |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughAdds a new Ginkgo end-to-end test suite that validates NetworkPolicy behavior by creating ephemeral server and client pods to verify connectivity across IPv4/IPv6, exercising deny/allow ingress/egress rules, and checking OpenShift config-operator NetworkPolicy enforcement including DNS egress handling and namespace-level policy enumeration. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.11.3)Error: can't load config: unsupported version of the configuration: "" See https://golangci-lint.run/docs/product/migration-guide for migration instructions Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
test/extended/networking/config_operator_networkpolicy.go (2)
38-43: Remove misplaceddefer ginkgo.GinkgoRecover().
GinkgoRecover()is intended for use inside goroutines spawned within test specs to catch panics. At theDescribeblock level, it has no effect since the function returns immediately after registering callbacks.Suggested fix
var _ = ginkgo.Describe("[sig-network][Feature:NetworkPolicy] Config Operator NetworkPolicy", func() { - defer ginkgo.GinkgoRecover() - oc := exutil.NewCLI("config-operator-networkpolicy-e2e")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/extended/networking/config_operator_networkpolicy.go` around lines 38 - 43, Remove the misplaced defer ginkgo.GinkgoRecover() call from the top-level ginkgo.Describe block (the Describe closure where oc := exutil.NewCLI... and f := oc.KubeFramework() are defined); GinkgoRecover() should only be used inside goroutines spawned in specs, so simply delete the line "defer ginkgo.GinkgoRecover()" from that Describe function (no other changes required unless you add goroutines, in which case call ginkgo.GinkgoRecover() inside those goroutines).
423-435:wait.PollImmediateis deprecated—migrate towait.PollUntilContextTimeout.The deprecation is documented in k8s.io/apimachinery and explicitly recommends
PollUntilContextTimeoutas the replacement. This applies to all four usages in this file (lines 425, 457, 534, 552).Example migration for expectConnectivityForIP
- err := wait.PollImmediate(5*time.Second, 2*time.Minute, func() (bool, error) { + err := wait.PollUntilContextTimeout(context.Background(), 5*time.Second, 2*time.Minute, true, func(ctx context.Context) (bool, error) { succeeded, err := runConnectivityCheck(kubeClient, namespace, clientLabels, serverIP, port) if err != nil { return false, err } return succeeded == shouldSucceed, nil })The
trueparameter setsimmediatemode, matchingPollImmediatebehavior. No need for nested context timeouts.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/extended/networking/config_operator_networkpolicy.go` around lines 423 - 435, Replace wait.PollImmediate calls with wait.PollUntilContextTimeout using a background context and immediate=true to preserve behavior: for example in expectConnectivityForIP (and the three other occurrences in this file) create a ctx (e.g., context.Background()) and call wait.PollUntilContextTimeout(ctx, 5*time.Second, 2*time.Minute, true, func(ctx context.Context) (bool, error) { /* move existing body here, change closure signature to accept ctx and return (bool,error) */ }), leaving the existing connectivity check logic and gomega.Expect unchanged; ensure any inner closure returns the same success boolean and error as before.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@test/extended/networking/config_operator_networkpolicy.go`:
- Around line 525-530: The code dereferences
ContainerStatuses[0].State.Terminated without guarding for nil, which can panic;
in the function waiting for pod completion (e.g., waitForPodCompletion) check
that completed.Status.ContainerStatuses is non-empty and that
ContainerStatuses[0].State is non-nil and ContainerStatuses[0].State.Terminated
is non-nil before reading ExitCode, and if Terminated is nil return a
descriptive error (including namespace/name) instead of proceeding; update the
logic around exitCode/return to only compute exitCode when Terminated exists and
otherwise surface the error so framework.Logf and the boolean result aren't
based on a nil dereference.
---
Nitpick comments:
In `@test/extended/networking/config_operator_networkpolicy.go`:
- Around line 38-43: Remove the misplaced defer ginkgo.GinkgoRecover() call from
the top-level ginkgo.Describe block (the Describe closure where oc :=
exutil.NewCLI... and f := oc.KubeFramework() are defined); GinkgoRecover()
should only be used inside goroutines spawned in specs, so simply delete the
line "defer ginkgo.GinkgoRecover()" from that Describe function (no other
changes required unless you add goroutines, in which case call
ginkgo.GinkgoRecover() inside those goroutines).
- Around line 423-435: Replace wait.PollImmediate calls with
wait.PollUntilContextTimeout using a background context and immediate=true to
preserve behavior: for example in expectConnectivityForIP (and the three other
occurrences in this file) create a ctx (e.g., context.Background()) and call
wait.PollUntilContextTimeout(ctx, 5*time.Second, 2*time.Minute, true, func(ctx
context.Context) (bool, error) { /* move existing body here, change closure
signature to accept ctx and return (bool,error) */ }), leaving the existing
connectivity check logic and gomega.Expect unchanged; ensure any inner closure
returns the same success boolean and error as before.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7640fa04-74a9-4944-a68b-e94f16a687b9
📒 Files selected for processing (1)
test/extended/networking/config_operator_networkpolicy.go
|
Scheduling required tests: |
|
/retest-required |
|
Job Failure Risk Analysis for sha: 8cf1f04
Risk analysis has seen new tests most likely introduced by this PR. New Test Risks for sha: 8cf1f04
New tests seen in this PR at sha: 8cf1f04
|
zhouying7780
force-pushed
the
networkpolicy-config-e2e
branch
from
8cf1f04 to
50f9fb9
Compare
|
Scheduling required tests: |
|
/test e2e-vsphere-ovn |
|
/test e2e-gcp-ovn |
|
/test e2e-vsphere-ovn-upi |
|
/test e2e-metal-ipi-ovn-ipv6 |
|
/test e2e-aws-ovn-fips |
|
@zhouying7780: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
|
Job Failure Risk Analysis for sha: 50f9fb9
Risk analysis has seen new tests most likely introduced by this PR. New Test Risks for sha: 50f9fb9
New tests seen in this PR at sha: 50f9fb9
|
2 participants
No description provided.