diff --git a/connectivity/check/check.go b/connectivity/check/check.go index c5af9c33f3..78b9e569ad 100644 --- a/connectivity/check/check.go +++ b/connectivity/check/check.go @@ -64,6 +64,9 @@ type Parameters struct { DeleteCiliumOnNodes []string + Retry uint + RetryDelay time.Duration + ConnectTimeout time.Duration RequestTimeout time.Duration diff --git a/connectivity/suite.go b/connectivity/suite.go index 093ae3a190..8697d2a08f 100644 --- a/connectivity/suite.go +++ b/connectivity/suite.go @@ -216,10 +216,10 @@ func Run(ctx context.Context, ct *check.ConnectivityTest) error { tests.ClientToClient(), tests.PodToService(), tests.PodToHostPort(), - tests.PodToWorld(), + tests.PodToWorld(tests.WithRetryAll()), tests.PodToHost(), tests.PodToExternalWorkload(), - tests.PodToCIDR(), + tests.PodToCIDR(tests.WithRetryAll()), } if s, ok := ct.Feature(check.FeatureNodeWithoutCilium); ok && s.Enabled { noPoliciesScenarios = append(noPoliciesScenarios, tests.FromCIDRToPod()) @@ -286,7 +286,7 @@ func Run(ctx context.Context, ct *check.ConnectivityTest) error { // 2. Egress to world works because there is no egress policy (so egress traffic originating from a pod is allowed), // then when replies come back, they are considered as "replies" to the outbound connection. // so they are not subject to ingress policy. - allIngressDenyScenarios := []check.Scenario{tests.PodToPod(), tests.PodToCIDR()} + allIngressDenyScenarios := []check.Scenario{tests.PodToPod(), tests.PodToCIDR(tests.WithRetryAll())} if s, ok := ct.Feature(check.FeatureNodeWithoutCilium); ok && s.Enabled { allIngressDenyScenarios = append(allIngressDenyScenarios, tests.FromCIDRToPod()) } @@ -309,7 +309,7 @@ func Run(ctx context.Context, ct *check.ConnectivityTest) error { // Egress to world works because there is no egress policy (so egress traffic originating from a pod is allowed), // then when replies come back, they are considered as "replies" to the outbound connection. // so they are not subject to ingress policy. - tests.PodToCIDR(), + tests.PodToCIDR(tests.WithRetryAll()), ). WithExpectations(func(a *check.Action) (egress, ingress check.Result) { if a.Destination().Address(check.GetIPFamily(ct.Params().ExternalOtherIP)) == ct.Params().ExternalOtherIP || @@ -455,7 +455,7 @@ func Run(ctx context.Context, ct *check.ConnectivityTest) error { // This policy allows UDP to kube-dns and port 80 TCP to all 'world' endpoints. ct.NewTest("to-entities-world").WithCiliumPolicy(clientEgressToEntitiesWorldPolicyYAML). WithScenarios( - tests.PodToWorld(), + tests.PodToWorld(tests.WithRetryDestPort(80)), ). WithExpectations(func(a *check.Action) (egress, ingress check.Result) { if a.Destination().Port() == 80 { @@ -470,7 +470,7 @@ func Run(ctx context.Context, ct *check.ConnectivityTest) error { ct.NewTest("to-cidr-external"). WithCiliumPolicy(renderedTemplates["clientEgressToCIDRExternalPolicyYAML"]). WithScenarios( - tests.PodToCIDR(), + tests.PodToCIDR(tests.WithRetryDestIP(ct.Params().ExternalIP)), ). WithExpectations(func(a *check.Action) (egress, ingress check.Result) { if a.Destination().Address(check.IPFamilyV4) == ct.Params().ExternalOtherIP { @@ -485,7 +485,7 @@ func Run(ctx context.Context, ct *check.ConnectivityTest) error { ct.NewTest("to-cidr-external-knp"). WithK8SPolicy(renderedTemplates["clientEgressToCIDRExternalPolicyKNPYAML"]). WithScenarios( - tests.PodToCIDR(), + tests.PodToCIDR(tests.WithRetryDestIP(ct.Params().ExternalIP)), ). WithExpectations(func(a *check.Action) (egress, ingress check.Result) { if a.Destination().Address(check.IPFamilyV4) == ct.Params().ExternalOtherIP { @@ -616,7 +616,7 @@ func Run(ctx context.Context, ct *check.ConnectivityTest) error { WithCiliumPolicy(allowAllEgressPolicyYAML). // Allow all egress traffic WithCiliumPolicy(renderedTemplates["clientEgressToCIDRExternalDenyPolicyYAML"]). WithScenarios( - tests.PodToCIDR(), // Denies all traffic to ExternalOtherIP, but allow ExternalIP + tests.PodToCIDR(tests.WithRetryDestIP(ct.Params().ExternalIP)), // Denies all traffic to ExternalOtherIP, but allow ExternalIP ). WithExpectations(func(a *check.Action) (egress, ingress check.Result) { if a.Destination().Address(check.GetIPFamily(ct.Params().ExternalOtherIP)) == ct.Params().ExternalOtherIP { @@ -733,7 +733,7 @@ func Run(ctx context.Context, ct *check.ConnectivityTest) error { WithCiliumPolicy(renderedTemplates["clientEgressL7HTTPPolicyYAML"]). // L7 allow policy with HTTP introspection WithScenarios( tests.PodToPod(), - tests.PodToWorld(), + tests.PodToWorld(tests.WithRetryDestPort(80), tests.WithRetryPodLabel("other", "client")), ). WithExpectations(func(a *check.Action) (egress, ingress check.Result) { if a.Source().HasLabel("other", "client") && // Only client2 is allowed to make HTTP calls. @@ -761,7 +761,7 @@ func Run(ctx context.Context, ct *check.ConnectivityTest) error { WithCiliumPolicy(renderedTemplates["clientEgressL7HTTPNamedPortPolicyYAML"]). // L7 allow policy with HTTP introspection (named port) WithScenarios( tests.PodToPod(), - tests.PodToWorld(), + tests.PodToWorld(tests.WithRetryDestPort(80), tests.WithRetryPodLabel("other", "client")), ). WithExpectations(func(a *check.Action) (egress, ingress check.Result) { if a.Source().HasLabel("other", "client") && // Only client2 is allowed to make HTTP calls. @@ -897,7 +897,7 @@ func Run(ctx context.Context, ct *check.ConnectivityTest) error { ct.NewTest("to-fqdns").WithCiliumPolicy(renderedTemplates["clientEgressToFQDNsCiliumIOPolicyYAML"]). WithFeatureRequirements(check.RequireFeatureEnabled(check.FeatureL7Proxy)). WithScenarios( - tests.PodToWorld(), + tests.PodToWorld(tests.WithRetryDestPort(80)), tests.PodToWorld2(), // resolves cilium.io ). WithExpectations(func(a *check.Action) (egress, ingress check.Result) { diff --git a/connectivity/tests/common.go b/connectivity/tests/common.go index b3450fc5a8..30a3edf623 100644 --- a/connectivity/tests/common.go +++ b/connectivity/tests/common.go @@ -3,6 +3,12 @@ package tests +import ( + "strconv" + + "github.com/cilium/cilium-cli/connectivity/check" +) + type labelsContainer interface { HasLabel(key, value string) bool } @@ -48,3 +54,79 @@ func hasAllLabels(labelsContainer labelsContainer, filters map[string]string) bo } return true } + +type retryCondition struct { + podLabels map[string]string + all bool + destPort uint32 + destIP string +} + +// CurlOptions returns curl retry option or empty slice depending on retry conditions +func (rc *retryCondition) CurlOptions(peer check.TestPeer, ipFam check.IPFamily, pod check.Pod, params check.Parameters) []string { + if params.Retry == 0 { + return []string{} + } + if !rc.all && rc.destIP == "" && rc.destPort == 0 { + return []string{} + } + + opts := []string{ + "--retry", strconv.FormatInt(int64(params.Retry), 10), + "--retry-all-errors", // add --retry-all-errors to retry on all possible errors + } + + if retryDelay := params.RetryDelay.Seconds(); retryDelay > 0.0 { + opts = append(opts, "--retry-delay", strconv.FormatFloat(retryDelay, 'f', -1, 64)) + } + + if rc.all { + return opts + } + if rc.destIP != "" && peer.Address(ipFam) != rc.destIP { + return []string{} + } + if rc.destPort != 0 && peer.Port() != rc.destPort { + return []string{} + } + for n, v := range rc.podLabels { + if !pod.HasLabel(n, v) { + return []string{} + } + } + + return opts +} + +type RetryOption func(*retryCondition) + +// WithRetryAll sets all condition, returns retry options in every case +func WithRetryAll() RetryOption { + return func(rc *retryCondition) { + rc.all = true + } +} + +// WithRetryDestIP sets ip address condition +func WithRetryDestIP(ip string) RetryOption { + return func(rc *retryCondition) { + rc.destIP = ip + } +} + +// WithRetryDestPort sets port condition +func WithRetryDestPort(port uint32) RetryOption { + return func(rc *retryCondition) { + rc.destPort = port + } +} + +// WithRetryPodLabel sets pod label condition +func WithRetryPodLabel(name, val string) RetryOption { + return func(rc *retryCondition) { + if rc.podLabels == nil { + rc.podLabels = map[string]string{} + } + rc.podLabels[name] = val + } +} diff --git a/connectivity/tests/to-cidr.go b/connectivity/tests/to-cidr.go index 7ec1799db1..6ed90520ef 100644 --- a/connectivity/tests/to-cidr.go +++ b/connectivity/tests/to-cidr.go @@ -11,14 +11,20 @@ import ( "github.com/cilium/cilium-cli/connectivity/check" ) -// PodToCIDR sends an ICMP packet from each client Pod -// to ExternalIP and ExternalOtherIP. -func PodToCIDR() check.Scenario { - return &podToCIDR{} +// PodToCIDR sends an HTTPS request from each client Pod +// to ExternalIP and ExternalOtherIP +func PodToCIDR(opts ...RetryOption) check.Scenario { + cond := &retryCondition{} + for _, op := range opts { + op(cond) + } + return &podToCIDR{rc: cond} } // podToCIDR implements a Scenario. -type podToCIDR struct{} +type podToCIDR struct { + rc *retryCondition +} func (s *podToCIDR) Name() string { return "pod-to-cidr" @@ -35,7 +41,8 @@ func (s *podToCIDR) Run(ctx context.Context, t *check.Test) { src := src // copy to avoid memory aliasing when using reference t.NewAction(s, fmt.Sprintf("%s-%d", ep.Name(), i), &src, ep, check.IPFamilyAny).Run(func(a *check.Action) { - a.ExecInPod(ctx, ct.CurlCommand(ep, check.IPFamilyAny)) + opts := s.rc.CurlOptions(ep, check.IPFamilyAny, src, ct.Params()) + a.ExecInPod(ctx, ct.CurlCommand(ep, check.IPFamilyAny, opts...)) a.ValidateFlows(ctx, src, a.GetEgressRequirements(check.FlowParameters{ RSTAllowed: true, diff --git a/connectivity/tests/world.go b/connectivity/tests/world.go index 31d86fdc7a..7e30948978 100644 --- a/connectivity/tests/world.go +++ b/connectivity/tests/world.go @@ -10,14 +10,19 @@ import ( "github.com/cilium/cilium-cli/connectivity/check" ) -// PodToWorld sends multiple HTTP(S) requests to one.one.one.one -// from random client Pods. -func PodToWorld() check.Scenario { - return &podToWorld{} +// PodToWorld sends multiple HTTP(S) requests to ExternalTarget +// from each client Pods. +func PodToWorld(opts ...RetryOption) check.Scenario { + cond := &retryCondition{} + for _, op := range opts { + op(cond) + } + return &podToWorld{rc: cond} } // podToWorld implements a Scenario. type podToWorld struct { + rc *retryCondition } func (s *podToWorld) Name() string { @@ -42,20 +47,23 @@ func (s *podToWorld) Run(ctx context.Context, t *check.Test) { client := client // copy to avoid memory aliasing when using reference // With http, over port 80. + httpOpts := s.rc.CurlOptions(http, check.IPFamilyAny, client, ct.Params()) t.NewAction(s, fmt.Sprintf("http-to-%s-%d", extTarget, i), &client, http, check.IPFamilyAny).Run(func(a *check.Action) { - a.ExecInPod(ctx, ct.CurlCommand(http, check.IPFamilyAny)) + a.ExecInPod(ctx, ct.CurlCommand(http, check.IPFamilyAny, httpOpts...)) a.ValidateFlows(ctx, client, a.GetEgressRequirements(fp)) }) // With https, over port 443. + httpsOpts := s.rc.CurlOptions(https, check.IPFamilyAny, client, ct.Params()) t.NewAction(s, fmt.Sprintf("https-to-%s-%d", extTarget, i), &client, https, check.IPFamilyAny).Run(func(a *check.Action) { - a.ExecInPod(ctx, ct.CurlCommand(https, check.IPFamilyAny)) + a.ExecInPod(ctx, ct.CurlCommand(https, check.IPFamilyAny, httpsOpts...)) a.ValidateFlows(ctx, client, a.GetEgressRequirements(fp)) }) // With https, over port 443, index.html. + httpsindexOpts := s.rc.CurlOptions(httpsindex, check.IPFamilyAny, client, ct.Params()) t.NewAction(s, fmt.Sprintf("https-to-%s-index-%d", extTarget, i), &client, httpsindex, check.IPFamilyAny).Run(func(a *check.Action) { - a.ExecInPod(ctx, ct.CurlCommand(httpsindex, check.IPFamilyAny)) + a.ExecInPod(ctx, ct.CurlCommand(httpsindex, check.IPFamilyAny, httpsindexOpts...)) a.ValidateFlows(ctx, client, a.GetEgressRequirements(fp)) }) diff --git a/defaults/defaults.go b/defaults/defaults.go index 386eaa78c3..22fdccaa2e 100644 --- a/defaults/defaults.go +++ b/defaults/defaults.go @@ -85,6 +85,9 @@ const ( PolicyWaitTimeout = 15 * time.Second + ConnectRetry = 3 + ConnectRetryDelay = 3 * time.Second + ConnectTimeout = 2 * time.Second RequestTimeout = 10 * time.Second diff --git a/internal/cli/cmd/connectivity.go b/internal/cli/cmd/connectivity.go index 33bdb35ff8..dbc3dd8c77 100644 --- a/internal/cli/cmd/connectivity.go +++ b/internal/cli/cmd/connectivity.go @@ -156,6 +156,9 @@ func newCmdConnectivityTest() *cobra.Command { cmd.Flags().StringVar(¶ms.JSONMockImage, "json-mock-image", defaults.ConnectivityCheckJSONMockImage, "Image path to use for json mock") cmd.Flags().StringVar(¶ms.DNSTestServerImage, "dns-test-server-image", defaults.ConnectivityDNSTestServerImage, "Image path to use for CoreDNS") + cmd.Flags().UintVar(¶ms.Retry, "retry", defaults.ConnectRetry, "Number of retries on connection failure to external targets") + cmd.Flags().DurationVar(¶ms.RetryDelay, "retry-delay", defaults.ConnectRetryDelay, "Delay between retries for external targets") + cmd.Flags().DurationVar(¶ms.ConnectTimeout, "connect-timeout", defaults.ConnectTimeout, "Maximum time to allow initiation of the connection to take") cmd.Flags().DurationVar(¶ms.RequestTimeout, "request-timeout", defaults.RequestTimeout, "Maximum time to allow a request to take")