Skip to content

Commit

Permalink
connectivity: cover echo-{same,other}-node services in ingress tests
Browse files Browse the repository at this point in the history
There's been some back and forth [1,2] on which service should be the
backend of the ingress, as echo-other-node allows to cover the cluster
mesh case as well, but is not available on single-node clusters. Given
that they provide a different coverage, and we just discovered a bug
which only happens when targeting a backend on the same node of the
client, let's start creating two ingresses to cover both cases.

[1]: f7a8822 ("connectivity: make ingress target echo-other-node deployment")
[2]: 7e7fa6a ("ingress: Update backend service for Ingress")

Signed-off-by: Marco Iorio <[email protected]>
  • Loading branch information
giorio94 authored and sayboras committed May 6, 2024
1 parent f4a8b64 commit f1a6316
Showing 1 changed file with 26 additions and 16 deletions.
42 changes: 26 additions & 16 deletions connectivity/check/deployment.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,6 @@ const (
KindTestConnDisrupt = "test-conn-disrupt"

EchoServerHostPort = 4000

IngressServiceName = "ingress-service"
)

type deploymentParameters struct {
Expand Down Expand Up @@ -339,10 +337,10 @@ func newLocalReadinessProbe(port int, path string) *corev1.Probe {
}
}

func newIngress() *networkingv1.Ingress {
func newIngress(name, backend string) *networkingv1.Ingress {
return &networkingv1.Ingress{
ObjectMeta: metav1.ObjectMeta{
Name: IngressServiceName,
Name: name,
Annotations: map[string]string{
"ingress.cilium.io/loadbalancer-mode": "dedicated",
"ingress.cilium.io/service-type": "NodePort",
Expand All @@ -365,7 +363,7 @@ func newIngress() *networkingv1.Ingress {
}(),
Backend: networkingv1.IngressBackend{
Service: &networkingv1.IngressServiceBackend{
Name: echoOtherNodeDeploymentName,
Name: backend,
Port: networkingv1.ServiceBackendPort{
Number: 8080,
},
Expand All @@ -381,6 +379,14 @@ func newIngress() *networkingv1.Ingress {
}
}

func (ct *ConnectivityTest) ingresses() map[string]string {
ingresses := map[string]string{"same-node": echoSameNodeDeploymentName}
if !ct.Params().SingleNode || ct.Params().MultiCluster != "" {
ingresses["other-node"] = echoOtherNodeDeploymentName
}
return ingresses
}

// deploy ensures the test Namespace, Services and Deployments are running on the cluster.
func (ct *ConnectivityTest) deploy(ctx context.Context) error {
var err error
Expand Down Expand Up @@ -908,12 +914,14 @@ func (ct *ConnectivityTest) deploy(ctx context.Context) error {

// Create one Ingress service for echo deployment
if ct.Features[features.IngressController].Enabled {
_, err = ct.clients.src.GetIngress(ctx, ct.params.TestNamespace, IngressServiceName, metav1.GetOptions{})
if err != nil {
ct.Logf("✨ [%s] Deploying Ingress resource...", ct.clients.src.ClusterName())
_, err = ct.clients.src.CreateIngress(ctx, ct.params.TestNamespace, newIngress(), metav1.CreateOptions{})
for name, backend := range ct.ingresses() {
_, err = ct.clients.src.GetIngress(ctx, ct.params.TestNamespace, name, metav1.GetOptions{})
if err != nil {
return err
ct.Logf("✨ [%s] Deploying Ingress resource...", ct.clients.src.ClusterName())
_, err = ct.clients.src.CreateIngress(ctx, ct.params.TestNamespace, newIngress(name, backend), metav1.CreateOptions{})
if err != nil {
return err
}
}
}
}
Expand Down Expand Up @@ -1313,13 +1321,15 @@ func (ct *ConnectivityTest) validateDeployment(ctx context.Context) error {
}

if ct.Features[features.IngressController].Enabled {
svcName := fmt.Sprintf("cilium-ingress-%s", IngressServiceName)
svc, err := WaitForServiceRetrieval(ctx, ct, ct.client, ct.params.TestNamespace, svcName)
if err != nil {
return err
}
for name := range ct.ingresses() {
svcName := fmt.Sprintf("cilium-ingress-%s", name)
svc, err := WaitForServiceRetrieval(ctx, ct, ct.client, ct.params.TestNamespace, svcName)
if err != nil {
return err
}

ct.ingressService[svcName] = svc
ct.ingressService[svcName] = svc
}
}

if ct.params.MultiCluster == "" {
Expand Down

0 comments on commit f1a6316

Please sign in to comment.