diff --git a/pkg/utils/cluster.go b/pkg/utils/cluster.go index f9921e85..04a4cf15 100644 --- a/pkg/utils/cluster.go +++ b/pkg/utils/cluster.go @@ -19,10 +19,12 @@ package utils import ( "fmt" + "time" "github.com/go-logr/logr" networkingv1 "k8s.io/api/networking/v1" networkingv1beta1 "k8s.io/api/networking/v1beta1" + apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/client-go/discovery" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" @@ -31,9 +33,22 @@ import ( "github.com/apache/apisix-ingress-controller/internal/types" ) +const ( + // discoveryRetryInitialInterval is the first backoff interval used when + // retrying discovery after a transient error. + discoveryRetryInitialInterval = 1 * time.Second + // discoveryRetryMaxInterval caps the exponential backoff between retries. + discoveryRetryMaxInterval = 30 * time.Second +) + // HasAPIResource checks if a specific API resource is available in the current cluster. // It uses the Discovery API to query the cluster's available resources and returns true // if the resource is found, false otherwise. +// +// A transient failure to reach the API server (e.g. during a control-plane +// failover at startup) is NOT treated as "resource absent": discovery is +// retried with backoff until a definitive answer is obtained, so a temporary +// outage never causes a resource to be permanently skipped. func HasAPIResource(mgr ctrl.Manager, obj client.Object) bool { return HasAPIResourceWithLogger(mgr, obj, ctrl.Log.WithName("api-detection")) } @@ -56,29 +71,85 @@ func HasAPIResourceWithLogger(mgr ctrl.Manager, obj client.Object, logger logr.L "groupVersion", groupVersion, ) - // Create discovery client - discoveryClient, err := discovery.NewDiscoveryClientForConfig(mgr.GetConfig()) - if err != nil { - logger.Info("failed to create discovery client", "error", err) - return false - } + check := func() (bool, error) { + // Create discovery client + discoveryClient, err := discovery.NewDiscoveryClientForConfig(mgr.GetConfig()) + if err != nil { + return false, err + } - // Query server resources for the specific group/version - apiResources, err := discoveryClient.ServerResourcesForGroupVersion(groupVersion) - if err != nil { - logger.Info("group/version not available in cluster", "error", err) - return false + // Query server resources for the specific group/version + apiResources, err := discoveryClient.ServerResourcesForGroupVersion(groupVersion) + if err != nil { + return false, err + } + + // Check if the specific kind exists in the resource list + for _, res := range apiResources.APIResources { + if res.Kind == gvk.Kind { + return true, nil + } + } + return false, nil } - // Check if the specific kind exists in the resource list - for _, res := range apiResources.APIResources { - if res.Kind == gvk.Kind { - return true + return hasAPIResourceWithRetry(check, logger, time.Sleep) +} + +// hasAPIResourceWithRetry runs check until it returns a definitive answer. +// +// check returns (found, err). A nil error is definitive. A non-nil error is +// classified: a definitive error (the GroupVersion is genuinely absent, or +// discovery is forbidden) returns false immediately, while a transient error +// (the API server is temporarily unreachable) is retried with exponential +// backoff. This guarantees a transient API-server outage never collapses to a +// permanent "resource absent" result. sleep is injected for testability. +func hasAPIResourceWithRetry(check func() (bool, error), logger logr.Logger, sleep func(time.Duration)) bool { + interval := discoveryRetryInitialInterval + for { + found, err := check() + if err == nil { + if !found { + logger.Info("API resource kind not found in group/version") + } + return found + } + if !isTransientDiscoveryError(err) { + logger.Info("group/version not available in cluster", "error", err) + return false + } + logger.Info("transient error detecting API resource, retrying", + "error", err, "retryIn", interval.String()) + sleep(interval) + if interval < discoveryRetryMaxInterval { + if interval *= 2; interval > discoveryRetryMaxInterval { + interval = discoveryRetryMaxInterval + } } } +} - logger.Info("API resource kind not found in group/version") - return false +// isTransientDiscoveryError reports whether a discovery error is a transient +// transport/availability problem (the API server is temporarily unreachable) +// rather than a definitive signal that the GroupVersion is absent. +func isTransientDiscoveryError(err error) bool { + if err == nil { + return false + } + switch { + // A 404 on the group/version discovery endpoint definitively means the + // GroupVersion (and therefore the resource) is not installed. + case apierrors.IsNotFound(err): + return false + // RBAC forbids discovery: a misconfiguration that will not resolve by + // waiting, so treat it as definitive rather than blocking forever. + case apierrors.IsForbidden(err): + return false + default: + // Connection refused, timeouts, EOF, 5xx, throttling, or a not-yet-ready + // auth token: all may clear once the API server is reachable again. + return true + } } func FormatGVK(obj client.Object) string { diff --git a/pkg/utils/cluster_test.go b/pkg/utils/cluster_test.go new file mode 100644 index 00000000..250d8efe --- /dev/null +++ b/pkg/utils/cluster_test.go @@ -0,0 +1,111 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package utils + +import ( + "errors" + "net" + "testing" + "time" + + "github.com/go-logr/logr" + "github.com/stretchr/testify/assert" + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/runtime/schema" +) + +func TestIsTransientDiscoveryError(t *testing.T) { + gr := schema.GroupResource{Group: "gateway.networking.k8s.io", Resource: "tcproutes"} + tests := []struct { + name string + err error + transient bool + }{ + {"nil", nil, false}, + // Definitive: the GroupVersion is genuinely not installed. + {"not found", apierrors.NewNotFound(gr, "tcproutes"), false}, + // Definitive: RBAC will not self-heal by waiting. + {"forbidden", apierrors.NewForbidden(gr, "tcproutes", errors.New("nope")), false}, + // Transient: API server temporarily unreachable / overloaded. + {"connection refused", &net.OpError{Op: "dial", Err: errors.New("connection refused")}, true}, + {"service unavailable", apierrors.NewServiceUnavailable("apiserver is starting up"), true}, + {"server timeout", apierrors.NewServerTimeout(gr, "get", 1), true}, + {"too many requests", apierrors.NewTooManyRequestsError("slow down"), true}, + {"internal error", apierrors.NewInternalError(errors.New("boom")), true}, + {"generic transport error", errors.New("EOF"), true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.transient, isTransientDiscoveryError(tt.err)) + }) + } +} + +// TestHasAPIResourceWithRetry_TransientThenSuccess is the core regression test +// for #2734: a transient discovery failure at startup must NOT be treated as +// "resource absent". The check should be retried until it succeeds, so the +// field index (and its controller) is eventually registered. +func TestHasAPIResourceWithRetry_TransientThenSuccess(t *testing.T) { + var calls int + check := func() (bool, error) { + calls++ + if calls < 3 { + // Simulate the API server being unreachable during failover. + return false, apierrors.NewServiceUnavailable("apiserver not ready") + } + return true, nil + } + + var slept int + sleep := func(time.Duration) { slept++ } + + got := hasAPIResourceWithRetry(check, logr.Discard(), sleep) + assert.True(t, got, "resource must be reported present once the API server recovers") + assert.Equal(t, 3, calls, "transient errors must be retried, not skipped") + assert.Equal(t, 2, slept, "backoff must be applied between retries") +} + +// TestHasAPIResourceWithRetry_DefinitiveAbsent ensures a genuinely absent CRD +// returns false immediately without retrying, preserving optional-CRD behavior. +func TestHasAPIResourceWithRetry_DefinitiveAbsent(t *testing.T) { + gr := schema.GroupResource{Group: "gateway.networking.k8s.io", Resource: "tcproutes"} + var calls int + check := func() (bool, error) { + calls++ + return false, apierrors.NewNotFound(gr, "tcproutes") + } + + var slept int + got := hasAPIResourceWithRetry(check, logr.Discard(), func(time.Duration) { slept++ }) + assert.False(t, got) + assert.Equal(t, 1, calls, "a definitive NotFound must not be retried") + assert.Equal(t, 0, slept) +} + +// TestHasAPIResourceWithRetry_PresentNoError ensures a present resource on the +// first successful discovery returns true without retries. +func TestHasAPIResourceWithRetry_PresentNoError(t *testing.T) { + var calls int + check := func() (bool, error) { + calls++ + return true, nil + } + got := hasAPIResourceWithRetry(check, logr.Discard(), func(time.Duration) {}) + assert.True(t, got) + assert.Equal(t, 1, calls) +}