This is an automated email from the ASF dual-hosted git repository.
alinsran pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/apisix-ingress-controller.git
The following commit(s) were added to refs/heads/master by this push:
new d9ae07a1 fix: a failing endpoint shouldn't affect others (#2452)
d9ae07a1 is described below
commit d9ae07a10e45189b761ab958344be281d4fe1d68
Author: AlinsRan <[email protected]>
AuthorDate: Fri Jul 4 18:13:37 2025 +0800
fix: a failing endpoint shouldn't affect others (#2452)
---
internal/provider/adc/adc.go | 31 +++++---
internal/provider/adc/executor.go | 8 +-
test/e2e/gatewayapi/gatewayproxy.go | 146 +++++++++---------------------------
3 files changed, 66 insertions(+), 119 deletions(-)
diff --git a/internal/provider/adc/adc.go b/internal/provider/adc/adc.go
index ae0a4513..e8c44694 100644
--- a/internal/provider/adc/adc.go
+++ b/internal/provider/adc/adc.go
@@ -20,7 +20,9 @@ package adc
import (
"context"
"encoding/json"
+ "fmt"
"os"
+ "strings"
"sync"
"time"
@@ -277,6 +279,7 @@ func (d *adcClient) Start(ctx context.Context) error {
initalSyncDelay := d.InitSyncDelay
time.AfterFunc(initalSyncDelay, func() {
if err := d.Sync(ctx); err != nil {
+ log.Error(err)
return
}
})
@@ -290,7 +293,7 @@ func (d *adcClient) Start(ctx context.Context) error {
select {
case <-ticker.C:
if err := d.Sync(ctx); err != nil {
- log.Errorw("failed to sync resources",
zap.Error(err))
+ log.Error(err)
}
case <-ctx.Done():
return nil
@@ -315,25 +318,32 @@ func (d *adcClient) Sync(ctx context.Context) error {
log.Debugw("syncing resources with multiple configs",
zap.Any("configs", cfg))
+ var failedConfigs []string
for name, config := range cfg {
resources, err := d.store.GetResources(name)
if err != nil {
- return err
+ log.Errorw("failed to get resources from store",
zap.String("name", name), zap.Error(err))
+ failedConfigs = append(failedConfigs, name)
+ continue
}
if resources == nil {
continue
}
- err = d.sync(ctx, Task{
+ if err := d.sync(ctx, Task{
Name: name + "-sync",
configs: []adcConfig{config},
Resources: *resources,
- })
- if err != nil {
- return err
+ }); err != nil {
+ log.Errorw("failed to sync resources",
zap.String("name", name), zap.Error(err))
+ failedConfigs = append(failedConfigs, name)
}
}
-
+ if len(failedConfigs) > 0 {
+ return fmt.Errorf("failed to sync %d configs: %s",
+ len(failedConfigs),
+ strings.Join(failedConfigs, ", "))
+ }
return nil
}
@@ -353,13 +363,16 @@ func (d *adcClient) sync(ctx context.Context, task Task)
error {
args := BuildADCExecuteArgs(syncFilePath, task.Labels,
task.ResourceTypes)
- log.Debugw("syncing resources with multiple configs",
zap.Any("configs", task.configs))
+ var failedConfigs []string
for _, config := range task.configs {
if err := d.executor.Execute(ctx, d.BackendMode, config, args);
err != nil {
log.Errorw("failed to execute adc command",
zap.Error(err), zap.Any("config", config))
- return err
+ failedConfigs = append(failedConfigs, config.Name)
}
}
+ if len(failedConfigs) > 0 {
+ return fmt.Errorf("failed to execute adc command for configs:
%s", strings.Join(failedConfigs, ", "))
+ }
return nil
}
diff --git a/internal/provider/adc/executor.go
b/internal/provider/adc/executor.go
index 304739e2..bd7b6c7f 100644
--- a/internal/provider/adc/executor.go
+++ b/internal/provider/adc/executor.go
@@ -22,6 +22,7 @@ import (
"context"
"encoding/json"
"errors"
+ "fmt"
"os"
"os/exec"
"strings"
@@ -50,11 +51,16 @@ func (e *DefaultADCExecutor) Execute(ctx context.Context,
mode string, config ad
}
func (e *DefaultADCExecutor) runADC(ctx context.Context, mode string, config
adcConfig, args []string) error {
+ var failedAddrs []string
for _, addr := range config.ServerAddrs {
if err := e.runForSingleServerWithTimeout(ctx, addr, mode,
config, args); err != nil {
- return err
+ log.Errorw("failed to run adc for server",
zap.String("server", addr), zap.Error(err))
+ failedAddrs = append(failedAddrs, addr)
}
}
+ if len(failedAddrs) > 0 {
+ return fmt.Errorf("failed to run adc for servers: [%s]",
strings.Join(failedAddrs, ", "))
+ }
return nil
}
diff --git a/test/e2e/gatewayapi/gatewayproxy.go
b/test/e2e/gatewayapi/gatewayproxy.go
index 6bac0590..dfcbba9b 100644
--- a/test/e2e/gatewayapi/gatewayproxy.go
+++ b/test/e2e/gatewayapi/gatewayproxy.go
@@ -19,7 +19,6 @@ package gatewayapi
import (
"fmt"
- "net/http"
"time"
. "github.com/onsi/ginkgo/v2"
@@ -103,64 +102,6 @@ spec:
headers:
X-Proxy-Test: "disabled"
`
- var (
- gatewayProxyWithPluginMetadata0 = `
-apiVersion: apisix.apache.org/v1alpha1
-kind: GatewayProxy
-metadata:
- name: apisix-proxy-config
-spec:
- provider:
- type: ControlPlane
- controlPlane:
- endpoints:
- - %s
- auth:
- type: AdminKey
- adminKey:
- value: "%s"
- plugins:
- - name: error-page
- enabled: true
- config: {}
- pluginMetadata:
- error-page: {
- "enable": true,
- "error_404": {
- "body": "404 from plugin metadata",
- "content-type": "text/plain"
- }
- }
-`
- gatewayProxyWithPluginMetadata1 = `
-apiVersion: apisix.apache.org/v1alpha1
-kind: GatewayProxy
-metadata:
- name: apisix-proxy-config
-spec:
- provider:
- type: ControlPlane
- controlPlane:
- endpoints:
- - %s
- auth:
- type: AdminKey
- adminKey:
- value: "%s"
- plugins:
- - name: error-page
- enabled: true
- config: {}
- pluginMetadata:
- error-page: {
- "enable": false,
- "error_404": {
- "body": "404 from plugin metadata",
- "content-type": "text/plain"
- }
- }
-`
- )
var httpRouteForTest = `
apiVersion: gateway.networking.k8s.io/v1
@@ -275,59 +216,48 @@ spec:
})
})
- Context("Test Gateway with PluginMetadata", func() {
- var (
- err error
- )
-
- PIt("Should work OK with error-page", func() {
- By("Update GatewayProxy with PluginMetadata")
- err =
s.CreateResourceFromString(fmt.Sprintf(gatewayProxyWithPluginMetadata0,
s.Deployer.GetAdminEndpoint(), s.AdminKey()))
- Expect(err).ShouldNot(HaveOccurred())
+ Context("Test GatewayProxy with invalid endpoint", func() {
+ var gatewayProxyWithInvalidEndpoint = `
+apiVersion: apisix.apache.org/v1alpha1
+kind: GatewayProxy
+metadata:
+ name: apisix-proxy-config
+spec:
+ provider:
+ type: ControlPlane
+ controlPlane:
+ endpoints:
+ - "http://invalid-endpoint:9180"
+ - %s
+ auth:
+ type: AdminKey
+ adminKey:
+ value: "%s"
+`
+ It("Should fail to apply GatewayProxy with invalid endpoint",
func() {
+ By("Update GatewayProxy with invalid endpoint")
+ err :=
s.CreateResourceFromString(fmt.Sprintf(gatewayProxyWithInvalidEndpoint,
s.Deployer.GetAdminEndpoint(), s.AdminKey()))
+ Expect(err).NotTo(HaveOccurred(), "creating
GatewayProxy with enabled plugin")
time.Sleep(5 * time.Second)
- By("Create HTTPRoute for Gateway with GatewayProxy")
+ By("Create HTTPRoute")
resourceApplied("HTTPRoute", "test-route",
fmt.Sprintf(httpRouteForTest, "apisix"), 1)
- time.Sleep(5 * time.Second)
- By("Check PluginMetadata working")
- s.NewAPISIXClient().
- GET("/not-found").
- WithHost("example.com").
- Expect().
- Status(http.StatusNotFound).
- Body().Contains("404 from plugin metadata")
-
- By("Update GatewayProxy with PluginMetadata")
- err =
s.CreateResourceFromString(fmt.Sprintf(gatewayProxyWithPluginMetadata1,
s.Deployer.GetAdminEndpoint(), s.AdminKey()))
- Expect(err).ShouldNot(HaveOccurred())
- time.Sleep(5 * time.Second)
-
- By("Check PluginMetadata working")
- s.NewAPISIXClient().
- GET("/not-found").
- WithHost("example.com").
- Expect().
- Status(http.StatusNotFound).
- Body().Contains(`{"error_msg":"404 Route Not
Found"}`)
-
- By("Delete GatewayProxy")
- err =
s.DeleteResourceFromString(fmt.Sprintf(gatewayProxyWithPluginMetadata0,
s.Deployer.GetAdminEndpoint(), s.AdminKey()))
- Expect(err).ShouldNot(HaveOccurred())
- time.Sleep(5 * time.Second)
+ expectRequest := func() bool {
+ resp := s.NewAPISIXClient().
+ GET("/get").
+ WithHost("example.com").
+ Expect().Raw()
+ return resp.StatusCode == 200 &&
resp.Header.Get("X-Proxy-Test") == ""
+ }
- By("Check PluginMetadata is not working")
- s.NewAPISIXClient().
- GET("/not-found").
- WithHost("example.com").
- Expect().
- Status(http.StatusNotFound).
- Body().Contains(`{"error_msg":"404 Route Not
Found"}`)
+ Eventually(expectRequest).WithTimeout(8 *
time.Second).ProbeEvery(time.Second).Should(BeTrue())
})
})
- var (
- gatewayProxyWithInvalidProviderType = `
+ Context("Test GatewayProxy Provider Validation", func() {
+ var (
+ gatewayProxyWithInvalidProviderType = `
apiVersion: apisix.apache.org/v1alpha1
kind: GatewayProxy
metadata:
@@ -336,7 +266,7 @@ spec:
provider:
type: "InvalidType"
`
- gatewayProxyWithMissingControlPlane = `
+ gatewayProxyWithMissingControlPlane = `
apiVersion: apisix.apache.org/v1alpha1
kind: GatewayProxy
metadata:
@@ -345,7 +275,7 @@ spec:
provider:
type: "ControlPlane"
`
- gatewayProxyWithValidProvider = `
+ gatewayProxyWithValidProvider = `
apiVersion: apisix.apache.org/v1alpha1
kind: GatewayProxy
metadata:
@@ -361,9 +291,7 @@ spec:
adminKey:
value: "test-key"
`
- )
-
- Context("Test GatewayProxy Provider Validation", func() {
+ )
It("Should reject invalid provider type", func() {
By("Create GatewayProxy with invalid provider type")
err :=
s.CreateResourceFromString(gatewayProxyWithInvalidProviderType)