Skip to content
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 33 additions & 1 deletion test/e2e/cnp-domain/e2e_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
"github.com/onsi/ginkgo/v2"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/wait"
clientset "k8s.io/client-go/kubernetes"
"k8s.io/klog/v2"
"k8s.io/kubernetes/test/e2e"
Expand Down Expand Up @@ -111,7 +112,30 @@
}

testNetworkConnectivity := func(target string, shouldSucceed bool, description string) {
testNetworkConnectivityWithRetry(target, shouldSucceed, description, 20, 2*time.Second)
testNetworkConnectivityWithRetry(target, shouldSucceed, description, 30, 3*time.Second)
}

// waitForDNSResolversReady waits for all DNSNameResolver CRs associated with a CNP
// to be created and have at least one resolved address. This ensures the OVN ACL
// address sets are populated before connectivity checks.
waitForDNSResolversReady := func(name string, expectedCount int) {
ginkgo.By(fmt.Sprintf("Waiting for %d DNSNameResolver(s) to be ready for CNP %s", expectedCount, name))
dnsClient := f.DNSNameResolverClient()
labelSelector := fmt.Sprintf("anp=%s", name)

err := wait.PollUntilContextTimeout(context.TODO(), 2*time.Second, 60*time.Second, true, func(_ context.Context) (bool, error) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Using context.TODO() in wait.PollUntilContextTimeout is a common pattern, but it's generally better practice to use a cancellable context derived from a parent context (e.g., f.Context). This ensures proper context propagation and allows for earlier cancellation if the parent context is cancelled, leading to more robust test behavior.

Suggested change
err := wait.PollUntilContextTimeout(context.TODO(), 2*time.Second, 60*time.Second, true, func(_ context.Context) (bool, error) {
err := wait.PollUntilContextTimeout(f.Context, 2*time.Second, 60*time.Second, true, func(_ context.Context) (bool, error) {

resolverList := dnsClient.ListByLabel(labelSelector)
if len(resolverList.Items) < expectedCount {
return false, nil
}
for _, resolver := range resolverList.Items {
if len(resolver.Status.ResolvedNames) == 0 {
return false, nil
}
}
return true, nil
})
framework.ExpectNoError(err, "DNSNameResolvers for CNP %s failed to be ready within timeout", name)

Check failure on line 138 in test/e2e/cnp-domain/e2e_test.go

View workflow job for this annotation

GitHub Actions / Kube-OVN CNP Domain E2E (ipv4, overlay)

It 02/26/26 09:21:10.602

Check failure on line 138 in test/e2e/cnp-domain/e2e_test.go

View workflow job for this annotation

GitHub Actions / Kube-OVN CNP Domain E2E (ipv4, overlay)

It 02/26/26 09:19:59.537

Check failure on line 138 in test/e2e/cnp-domain/e2e_test.go

View workflow job for this annotation

GitHub Actions / Kube-OVN CNP Domain E2E (ipv4, overlay)

It 02/26/26 09:18:49.939

Check failure on line 138 in test/e2e/cnp-domain/e2e_test.go

View workflow job for this annotation

GitHub Actions / Kube-OVN CNP Domain E2E (ipv4, overlay)

It 02/26/26 09:17:40.041

Check failure on line 138 in test/e2e/cnp-domain/e2e_test.go

View workflow job for this annotation

GitHub Actions / Kube-OVN CNP Domain E2E (ipv4, overlay)

It 02/26/26 09:16:28.849
}

framework.ConformanceIt("should create CNP with domainName deny rule and verify connectivity behavior", func() {
Expand Down Expand Up @@ -158,6 +182,7 @@
framework.ExpectEqual(cnp.Spec.Priority, int32(55))
framework.ExpectEqual(cnp.Spec.Subject.Namespaces.MatchLabels["kubernetes.io/metadata.name"], namespaceName)

waitForDNSResolversReady(cnpName, 1)
testNetworkConnectivity("https://www.baidu.com", false, "Testing connectivity to baidu.com after applying CNP (should be blocked)")

ginkgo.By("Deleting ClusterNetworkPolicy " + cnpName)
Expand Down Expand Up @@ -229,6 +254,8 @@
framework.ExpectEqual(string(peer2.DomainNames[0]), "*.google.com.")
framework.ExpectEqual(cnp2.Spec.Priority, int32(45))

waitForDNSResolversReady(cnpName, 1)
waitForDNSResolversReady(cnpName2, 1)
testNetworkConnectivity("https://www.baidu.com", false, "Testing connectivity to baidu.com after applying both CNPs (should be blocked)")
testNetworkConnectivity("https://www.google.com", true, "Testing connectivity to google.com after applying both CNPs (should be allowed)")

Expand Down Expand Up @@ -288,6 +315,7 @@
updatedCNP, _ := cnpClient.Update(context.TODO(), createdCNP, metav1.UpdateOptions{})
framework.Logf("Successfully updated ClusterNetworkPolicy with baidu.com deny rule: %s", updatedCNP.Name)

waitForDNSResolversReady(cnpName, 1)
testNetworkConnectivity("https://www.baidu.com", false, "Testing connectivity to baidu.com after adding deny rule (should be blocked)")
testNetworkConnectivity("https://www.google.com", true, "Testing connectivity to google.com after adding baidu.com deny rule (should still succeed)")

Expand All @@ -299,6 +327,8 @@
updatedcnp2, _ := cnpClient.Update(context.TODO(), updatedCNP, metav1.UpdateOptions{})
framework.Logf("Successfully updated ClusterNetworkPolicy with both deny rules: %s", updatedcnp2.Name)

waitForDNSResolversReady(cnpName, 2)

testNetworkConnectivity("https://www.baidu.com", false, "Testing connectivity to baidu.com after adding both deny rules (should be blocked)")
testNetworkConnectivity("https://www.google.com", false, "Testing connectivity to google.com after adding both deny rules (should be blocked)")

Expand Down Expand Up @@ -373,6 +403,7 @@
framework.ExpectEqual(len(cnp.Spec.Egress), 2)
framework.ExpectEqual(cnp.Spec.Priority, int32(80))

waitForDNSResolversReady(cnpName, 1)
testNetworkConnectivity("https://www.baidu.com", false, "Testing connectivity to baidu.com after applying cnp (should be blocked)")
testNetworkConnectivity("https://www.google.com", true, "Testing connectivity to google.com after applying cnp (should be allowed)")
testNetworkConnectivity("https://8.8.8.8", false, "Testing connectivity to 8.8.8.8 after applying cnp (should be blocked by CIDR rule)")
Expand Down Expand Up @@ -421,6 +452,7 @@
framework.ExpectEqual(len(cnp.Spec.Egress), 1)
framework.ExpectEqual(cnp.Spec.Priority, int32(85))

waitForDNSResolversReady(cnpName, 1)
testNetworkConnectivity("https://www.baidu.com", false, "Testing connectivity to www.baidu.com after applying cnp (should be blocked by wildcard)")
testNetworkConnectivity("https://api.baidu.com", false, "Testing connectivity to api.baidu.com after applying cnp (should be blocked by wildcard)")
testNetworkConnectivity("https://news.baidu.com", false, "Testing connectivity to news.baidu.com after applying cnp (should be blocked by wildcard)")
Expand Down
Loading