bzp2010 commented on code in PR #2869:
URL:
https://github.com/apache/apisix-ingress-controller/pull/2869#discussion_r3985422830
##########
internal/adc/client/executor.go:
##########
@@ -150,40 +154,66 @@ func (e *HTTPADCExecutor) Validate(ctx context.Context,
config adctypes.Config,
return e.runHTTPValidate(ctx, config, resources, labels, resourceTypes)
}
-// runHTTPSync performs HTTP sync to ADC Server for each server address
+// runHTTPSync sends config in one /sync request and returns the parsed
failure, if any.
+// A sync is one request whatever config.ServerAddrs holds: apisix-standalone
joins every
+// entry with commas because ADC addresses them together as one logical
destination
+// (buildHTTPRequest splits them back apart into the request body), every
other backend
+// type takes the first entry only, since a GatewayProxy is expected to
resolve to one
+// address there even though nothing enforces it yet. Deciding how many
addresses a
+// GatewayProxy has belongs to the caller that built config.ServerAddrs.
+//
+// A GatewayProxy with no resolved address is a sync failure for
apisix-standalone (the
+// data plane it configures is unreachable, e.g. scaled to zero), and a no-op
for every
+// other backend type, which pushes per address and so has nothing to push.
+//
+// This package never decides whether to retry the failure; callers interpret
it and ask
+// again if they choose to.
func (e *HTTPADCExecutor) runHTTPSync(ctx context.Context, config
adctypes.Config, resources *adctypes.Resources, labels map[string]string,
resourceTypes []string) error {
- var execErrs = types.ADCExecutionError{
- Name: config.Name,
+ standalone := config.BackendType == BackendAPISIXStandalone
+ if len(config.ServerAddrs) == 0 {
+ if standalone {
+ return types.ADCExecutionServerAddrError{Err: "no data
plane address to sync apisix-standalone config to"}
+ }
+ return nil
}
- serverAddrs := func() []string {
- if config.BackendType == backendAPISIXStandalone {
- return []string{strings.Join(config.ServerAddrs, ",")}
+ target := syncTargetAddr(config)
+ e.log.V(1).Info("running http sync", "server", target)
+
+ ctx, cancel := context.WithTimeout(ctx, e.httpClient.Timeout)
+ defer cancel()
+
+ req, err := e.buildHTTPRequest(ctx, target, config, labels,
resourceTypes, resources, pathSync)
+ if err != nil {
+ return types.ADCExecutionServerAddrError{ServerAddr: target,
Err: fmt.Sprintf("failed to build HTTP request: %s", err)}
+ }
+
+ resp, err := e.httpClient.Do(req)
+ if err != nil {
+ return types.ADCExecutionServerAddrError{ServerAddr: target,
Err: fmt.Sprintf("failed to send HTTP request: %s", err)}
+ }
+ defer func() {
+ if closeErr := resp.Body.Close(); closeErr != nil {
+ e.log.Error(closeErr, "failed to close response body")
}
- return config.ServerAddrs
}()
- e.log.V(1).Info("running http sync", "serverAddrs", serverAddrs)
- for _, addr := range serverAddrs {
- if err := e.runHTTPSyncForSingleServer(ctx, addr, config,
resources, labels, resourceTypes); err != nil {
- e.log.Error(err, "failed to run http sync for server",
"server", addr)
- var execErr types.ADCExecutionServerAddrError
- if errors.As(err, &execErr) {
- execErrs.FailedErrors =
append(execErrs.FailedErrors, execErr)
- } else {
- execErrs.FailedErrors =
append(execErrs.FailedErrors, types.ADCExecutionServerAddrError{
- ServerAddr: addr,
- Err: err.Error(),
- })
- }
- }
- }
- if len(execErrs.FailedErrors) > 0 {
- return execErrs
+ if err := e.handleHTTPResponse(resp, target); err != nil {
+ e.log.Error(err, "failed to run http sync", "server", target)
+ return err
}
return nil
}
+// syncTargetAddr resolves config.ServerAddrs into what one /sync request
targets. Callers
+// must have already handled an empty ServerAddrs (see runHTTPSync).
+func syncTargetAddr(config adctypes.Config) string {
+ if config.BackendType == BackendAPISIXStandalone {
+ return strings.Join(config.ServerAddrs, ",")
+ }
+ return config.ServerAddrs[0]
Review Comment:
This is a deliberate interim tradeoff, not an oversight: a GatewayProxy is
expected to resolve to a single address for non-standalone backends, and
multi-address fan-out for that case is left unenforced (and unhandled) for now
rather than guarded against. Preserving or rejecting it is tracked as
follow-up, not addressed in this PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
