cilium-cli: connectivity test: support every kind of resource for tests #35314
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
1afd8c4 to
e6e7af8
Compare
|
/test |
The connectivity-test code allows for creating dependent resources as a part of tests (e.g. NetworkPolicies). However, the code as written requires new methods for every kind of resource. This change converts this to use the dynamic client, which is resource-agnostic. That allows for creating arbitrary sets of resources, as well as mixing resource kinds in a single YAML file. It removes a lot of boilerplate and churn as new resources are needed. Internally, it uses Server-Side Apply to reduce the number of k8s API calls necessary. Signed-off-by: Casey Callendrello <cdc@isovalent.com>
e6e7af8 to
8b17441
Compare
|
/test |
christarazi
approved these changes
Oct 9, 2024
jschwinger233
added a commit
that referenced
this pull request
Dec 17, 2024
It wasn't until #23445 was fixed in 1.14 when Cilium IPv6 had basic connectivity. Previously CI workflows didn't cover IPv6 connectivity in 1.13, until #35314 recently enabled it. Subsequently, We noticed a number of CI failures in ci-ipsec-upgrade, caused by testing IPv6 connectivity in 1.13. This patch skips IPv6 connectivity test for Cilium<1.14. Signed-off-by: gray <gray.liang@isovalent.com>
jschwinger233
added a commit
that referenced
this pull request
Dec 18, 2024
It wasn't until #23445 was fixed in 1.14 when Cilium had robust IPv6 connectivity for IPsec[1]. Previously CI workflows didn't run IPv6 connectivity for IPsec in 1.13, until #35314 recently enabled it. Subsequently, We noticed a number of CI failures in ci-ipsec-upgrade, caused by testing IPv6 connectivity in 1.13. This patch skips IPsec IPv6 connectivity test for Cilium<1.14. [1] #23461 Signed-off-by: gray <gray.liang@isovalent.com>
Comment on lines
-952
to
-961
| // The per-endpoint routes feature is broken with IPv6 on < v1.14 when there | ||
| // are any netpols installed (https://github.com/cilium/cilium/issues/23852 | ||
| // and https://github.com/cilium/cilium/issues/23910). | ||
| if f, ok := t.Context().Feature(features.EndpointRoutes); ok && | ||
| f.Enabled && (len(t.cnps) > 0 || len(t.knps) > 0) && | ||
| versioncheck.MustCompile("<1.14.0")(t.Context().CiliumVersion) { | ||
|
|
||
| ipFams = []features.IPFamily{features.IPFamilyV4} | ||
| } | ||
|
|
There was a problem hiding this comment.
This part (and the other skip if <v1.14 part below) should have really been a separate patch. And ideally even a separate PR, so that the impact on downgrade testing for v1.14 -> v1.13 would have been obvious.
jschwinger233
added a commit
that referenced
this pull request
Dec 26, 2024
It wasn't until #23445 was fixed in 1.14 when Cilium had robust IPv6 connectivity for IPsec[1]. Previously CI workflows didn't run IPv6 connectivity for IPsec in 1.13, until #35314 recently enabled it. Subsequently, We noticed a number of CI failures in ci-ipsec-upgrade, caused by testing IPv6 connectivity in 1.13. This patch reverts the deletion of `if <1.14 then skip` made by #35314. [1] #23461 Signed-off-by: gray <gray.liang@isovalent.com>
jschwinger233
added a commit
that referenced
this pull request
Dec 26, 2024
It wasn't until #23445 was fixed in 1.14 when Cilium had robust IPv6 connectivity for IPsec[1]. Previously CI workflows didn't run IPv6 connectivity for IPsec in 1.13, until #35314 recently enabled it. Subsequently, We noticed a number of CI failures in ci-ipsec-upgrade, caused by testing IPv6 connectivity in 1.13. This patch reverts the deletion of `if <1.14 then skip` made by #35314. [2] [1] #23461 [2] g diff aabda4b aabda4b~ | grep '<1.14.0' -C8 ```diff func (t *Test) ForEachIPFamily(do func(features.IPFamily)) { ipFams := []features.IPFamily{features.IPFamilyV4, features.IPFamilyV6} + // The per-endpoint routes feature is broken with IPv6 on < v1.14 when there + // are any netpols installed (#23852 + // and #23910). + if f, ok := t.Context().Feature(features.EndpointRoutes); ok && + f.Enabled && (len(t.cnps) > 0 || len(t.knps) > 0) && + versioncheck.MustCompile("<1.14.0")(t.Context().CiliumVersion) { + + ipFams = []features.IPFamily{features.IPFamilyV4} + } + for _, ipFam := range ipFams { switch ipFam { case features.IPFamilyV4: @@ -853,6 +979,18 @@ func (t *Test) CertificateCAs() map[string][]byte { -- + "github.com/cilium/cilium/pkg/versioncheck" ) // PodToService sends an HTTP request from all client Pods @@ -227,6 +228,13 @@ func curlNodePort(ctx context.Context, s check.Scenario, t *check.Test, } } + // Skip IPv6 requests when running on <1.14.0 Cilium with CNPs + if features.GetIPFamily(addr.Address) == features.IPFamilyV6 && + versioncheck.MustCompile("<1.14.0")(t.Context().CiliumVersion) && + (len(t.CiliumNetworkPolicies()) > 0 || len(t.KubernetesNetworkPolicies()) > 0) { + continue + } + // Manually construct an HTTP endpoint to override the destination IP // and port of the request. ep := check.HTTPEndpoint(name, fmt.Sprintf("%s://%s:%d%s", svc.Scheme(), addr.Address, np, svc.Path())) diff --git a/cilium-cli/k8s/client.go b/cilium-cli/k8s/client.go ``` Signed-off-by: gray <gray.liang@isovalent.com>
jschwinger233
added a commit
that referenced
this pull request
Dec 26, 2024
It wasn't until #23445 was fixed in 1.14 when Cilium had robust IPv6 connectivity for IPsec[1]. Previously CI workflows didn't run IPv6 connectivity for IPsec in 1.13, until #35314 recently enabled it. Subsequently, We noticed a number of CI failures in ci-ipsec-upgrade, caused by testing IPv6 connectivity in 1.13. This patch reverts the deletion of `if <1.14 then skip` made by #35314. [2] [1] #23461 [2] g diff aabda4b aabda4b~ | grep '<1.14.0' -C8 ``` func (t *Test) ForEachIPFamily(do func(features.IPFamily)) { ipFams := []features.IPFamily{features.IPFamilyV4, features.IPFamilyV6} + // The per-endpoint routes feature is broken with IPv6 on < v1.14 when there + // are any netpols installed (#23852 + // and #23910). + if f, ok := t.Context().Feature(features.EndpointRoutes); ok && + f.Enabled && (len(t.cnps) > 0 || len(t.knps) > 0) && + versioncheck.MustCompile("<1.14.0")(t.Context().CiliumVersion) { + + ipFams = []features.IPFamily{features.IPFamilyV4} + } + for _, ipFam := range ipFams { switch ipFam { case features.IPFamilyV4: @@ -853,6 +979,18 @@ func (t *Test) CertificateCAs() map[string][]byte { -- + "github.com/cilium/cilium/pkg/versioncheck" ) // PodToService sends an HTTP request from all client Pods @@ -227,6 +228,13 @@ func curlNodePort(ctx context.Context, s check.Scenario, t *check.Test, } } + // Skip IPv6 requests when running on <1.14.0 Cilium with CNPs + if features.GetIPFamily(addr.Address) == features.IPFamilyV6 && + versioncheck.MustCompile("<1.14.0")(t.Context().CiliumVersion) && + (len(t.CiliumNetworkPolicies()) > 0 || len(t.KubernetesNetworkPolicies()) > 0) { + continue + } + // Manually construct an HTTP endpoint to override the destination IP // and port of the request. ep := check.HTTPEndpoint(name, fmt.Sprintf("%s://%s:%d%s", svc.Scheme(), addr.Address, np, svc.Path())) diff --git a/cilium-cli/k8s/client.go b/cilium-cli/k8s/client.go ``` Signed-off-by: gray <gray.liang@isovalent.com>
jschwinger233
added a commit
that referenced
this pull request
Dec 27, 2024
It wasn't until #23445 was fixed in 1.14 when Cilium had robust IPv6 connectivity for IPsec[1]. Previously CI workflows didn't run IPv6 connectivity for IPsec in 1.13, until #35314 recently enabled it. Subsequently, We noticed a number of CI failures in ci-ipsec-upgrade, caused by testing IPv6 connectivity in 1.13. This patch reverts the deletion of `if <1.14 then skip` made by #35314. [2] [1] #23461 [2] git diff aabda4b aabda4b~ ``` func (t *Test) ForEachIPFamily(do func(features.IPFamily)) { + // The per-endpoint routes feature is broken with IPv6 on < v1.14 when there + // are any netpols installed (#23852 + // and #23910). + if f, ok := t.Context().Feature(features.EndpointRoutes); ok && + f.Enabled && (len(t.cnps) > 0 || len(t.knps) > 0) && + versioncheck.MustCompile("<1.14.0")(t.Context().CiliumVersion) { + + ipFams = []features.IPFamily{features.IPFamilyV4} + } + func curlNodePort(ctx context.Context, s check.Scenario, t *check.Test, + // Skip IPv6 requests when running on <1.14.0 Cilium with CNPs + if features.GetIPFamily(addr.Address) == features.IPFamilyV6 && + versioncheck.MustCompile("<1.14.0")(t.Context().CiliumVersion) && + (len(t.CiliumNetworkPolicies()) > 0 || len(t.KubernetesNetworkPolicies()) > 0) { + continue + } + ``` Signed-off-by: gray <gray.liang@isovalent.com>
aanm
pushed a commit
that referenced
this pull request
Jan 6, 2025
It wasn't until #23445 was fixed in 1.14 when Cilium had robust IPv6 connectivity for IPsec[1]. Previously CI workflows didn't run IPv6 connectivity for IPsec in 1.13, until #35314 recently enabled it. Subsequently, We noticed a number of CI failures in ci-ipsec-upgrade, caused by testing IPv6 connectivity in 1.13. This patch reverts the deletion of `if <1.14 then skip` made by #35314. [2] [1] #23461 [2] git diff aabda4b aabda4b~ ``` func (t *Test) ForEachIPFamily(do func(features.IPFamily)) { + // The per-endpoint routes feature is broken with IPv6 on < v1.14 when there + // are any netpols installed (#23852 + // and #23910). + if f, ok := t.Context().Feature(features.EndpointRoutes); ok && + f.Enabled && (len(t.cnps) > 0 || len(t.knps) > 0) && + versioncheck.MustCompile("<1.14.0")(t.Context().CiliumVersion) { + + ipFams = []features.IPFamily{features.IPFamilyV4} + } + func curlNodePort(ctx context.Context, s check.Scenario, t *check.Test, + // Skip IPv6 requests when running on <1.14.0 Cilium with CNPs + if features.GetIPFamily(addr.Address) == features.IPFamilyV6 && + versioncheck.MustCompile("<1.14.0")(t.Context().CiliumVersion) && + (len(t.CiliumNetworkPolicies()) > 0 || len(t.KubernetesNetworkPolicies()) > 0) { + continue + } + ``` Signed-off-by: gray <gray.liang@isovalent.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The connectivity-test code allows for creating dependent resources as a part of tests (e.g. NetworkPolicies). However, the code as written requires new methods for every kind of resource.
This change converts this to use the dynamic client, which is resource-agnostic. That allows for creating arbitrary sets of resources, as well as mixing resource kinds in a single YAML file. It removes a lot of boilerplate and churn as new resources are needed.
Internally, it uses Server-Side Apply to reduce the number of k8s API calls necessary.