Skip to content

Commit

Permalink
connectivity: fix service propagation check with mismatching IP families
Browse files Browse the repository at this point in the history
0dccab2 ("connectivity: wait for service propagation in agents")
introduced an additional check to wait until all test services have been
synchronized by cilium agents before starting the actual tests. Yet,
this check will never succeed if the cluster is configured in dual stack
mode, but only one IP family is enabled by Cilium. Indeed, the backends
for the disabled IP family will never be populated. Hence, let's skip
this check if the ClusterIP belongs to a disabled IP family.

Fixes: 0dccab2 ("connectivity: wait for service propagation in agents")
Reported-by: Donia Chaiehloudj <[email protected]>
Signed-off-by: Marco Iorio <[email protected]>
  • Loading branch information
giorio94 authored and michi-covalent committed Jul 7, 2023
1 parent 1b29d0a commit 76c400c
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 4 deletions.
2 changes: 1 addition & 1 deletion connectivity/check/deployment.go
Original file line number Diff line number Diff line change
Expand Up @@ -1269,7 +1269,7 @@ func (ct *ConnectivityTest) validateDeployment(ctx context.Context) error {

for _, agent := range ct.CiliumPods() {
if _, ok := nodes[agent.NodeName()]; ok {
if err := WaitForServiceEndpoints(ctx, ct, agent, s, 1); err != nil {
if err := WaitForServiceEndpoints(ctx, ct, agent, s, 1, ct.Features.IPFamilies()); err != nil {
return err
}
}
Expand Down
15 changes: 15 additions & 0 deletions connectivity/check/features.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,21 @@ func (fs FeatureSet) MatchRequirements(reqs ...FeatureRequirement) bool {
return true
}

// IPFamilies returns the list of enabled IP families.
func (fs FeatureSet) IPFamilies() []IPFamily {
var families []IPFamily

if fs.MatchRequirements(RequireFeatureEnabled(FeatureIPv4)) {
families = append(families, IPFamilyV4)
}

if fs.MatchRequirements(RequireFeatureEnabled(FeatureIPv6)) {
families = append(families, IPFamilyV6)
}

return families
}

// deriveFeatures derives additional features based on the status of other features
func (fs FeatureSet) deriveFeatures() error {
fs[FeatureHostPort] = FeatureStatus{
Expand Down
19 changes: 16 additions & 3 deletions connectivity/check/wait.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,13 @@ import (
"encoding/json"
"errors"
"fmt"
"net/netip"
"strconv"
"strings"
"time"

"github.com/cilium/cilium/api/v1/models"
"golang.org/x/exp/slices"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

"github.com/cilium/cilium-cli/defaults"
Expand Down Expand Up @@ -171,15 +173,15 @@ func WaitForService(ctx context.Context, log Logger, client Pod, service Service

// WaitForServiceEndpoints waits until the expected number of service backends
// are reported by the given agent.
func WaitForServiceEndpoints(ctx context.Context, log Logger, agent Pod, service Service, backends uint) error {
func WaitForServiceEndpoints(ctx context.Context, log Logger, agent Pod, service Service, backends uint, families []IPFamily) error {
log.Logf("⌛ [%s] Waiting for Service %s to be synchronized by Cilium pod %s",
agent.K8sClient.ClusterName(), service.Name(), agent.Name())

ctx, cancel := context.WithTimeout(ctx, ShortTimeout)
defer cancel()

for {
err := checkServiceEndpoints(ctx, agent, service, backends)
err := checkServiceEndpoints(ctx, agent, service, backends, families)
if err == nil {
return nil
}
Expand All @@ -196,7 +198,7 @@ func WaitForServiceEndpoints(ctx context.Context, log Logger, agent Pod, service
}
}

func checkServiceEndpoints(ctx context.Context, agent Pod, service Service, backends uint) error {
func checkServiceEndpoints(ctx context.Context, agent Pod, service Service, backends uint, families []IPFamily) error {
buffer, err := agent.K8sClient.ExecInPod(ctx, agent.Namespace(), agent.NameWithoutNamespace(),
defaults.AgentContainerName, []string{"cilium", "service", "list", "--output=json"})
if err != nil {
Expand All @@ -222,6 +224,17 @@ func checkServiceEndpoints(ctx context.Context, agent Pod, service Service, back
}

for _, ip := range service.Service.Spec.ClusterIPs {
addr, err := netip.ParseAddr(ip)
if err != nil {
return fmt.Errorf("failed to parse ClusterIP %q: %w", ip, err)
}

// Skip the check for a given address if the corresponding IP family is not
// enabled in Cilium, as the backends will never be populated.
if addr.Is4() && !slices.Contains(families, IPFamilyV4) || addr.Is6() && !slices.Contains(families, IPFamilyV6) {
continue
}

for _, port := range service.Service.Spec.Ports {
if found[l3n4{addr: ip, port: uint16(port.Port)}] < backends {
return errors.New("service not yet synchronized")
Expand Down

0 comments on commit 76c400c

Please sign in to comment.