-
Notifications
You must be signed in to change notification settings - Fork 364
feat: support deployment of multiple data plane modes #2647
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
api/adc/types.go
Outdated
| ServerAddrs []string | ||
| Token string | ||
| TlsVerify bool | ||
| BakcnedType string |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds support for multi-mode APISIX deployments by introducing a mode field to the GatewayProxy CRD. This allows individual gateway proxies to specify whether they use apisix (with etcd) or apisix-standalone (with yaml configuration) mode, independent of the global provider type setting.
Key changes:
- Added
modefield toControlPlaneProviderin theGatewayProxyAPI and CRD - Refactored provider options to use "Default" prefix to distinguish between default and per-gateway settings
- Added
BakcnedTypefield toConfigstruct to carry mode information per gateway - Created test case for multi-mode deployment scenarios
Reviewed Changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 13 comments.
Show a summary per file
| File | Description |
|---|---|
| api/v1alpha1/gatewayproxy_types.go | Added Mode field to ControlPlaneProvider struct with immutability validation |
| api/adc/types.go | Added BakcnedType field to Config struct to store backend mode |
| config/crd/bases/apisix.apache.org_gatewayproxies.yaml | Updated CRD with mode field definition and immutability validation |
| internal/adc/translator/gatewayproxy.go | Updated to populate BakcnedType from gateway proxy mode and resolve endpoints based on mode |
| internal/adc/client/executor.go | Removed mode parameter from methods, now using config.BakcnedType |
| internal/adc/client/client.go | Refactored to use per-config backend type with default fallback |
| internal/provider/options.go | Renamed fields to use "Default" prefix (DefaultBackendMode, DefaultResolveEndpoints) |
| internal/provider/apisix/provider.go | Updated to use renamed provider options fields |
| internal/provider/init/init.go | Updated to use renamed provider option functions |
| internal/manager/run.go | Removed BackendMode assignment from provider options initialization |
| test/e2e/scaffold/deployer.go | Added CreateAdditionalGatewayWithOptions method with ProviderType option |
| test/e2e/scaffold/apisix_deployer.go | Implemented CreateAdditionalGatewayWithOptions and updated config provider logic |
| test/e2e/framework/apisix_consts.go | Added constants for config provider types (Yaml, Etcd) |
| test/e2e/apisix/mode.go | New test file for multi-mode deployment scenarios |
| docs/en/latest/reference/api-reference.md | Added documentation for the new mode field |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 15 out of 15 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 15 out of 15 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| - message: mode is immutable | ||
| rule: oldSelf == null || (!has(self.mode) && !has(oldSelf.mode)) | ||
| || self.mode == oldSelf.mode |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the aim here? What might occur if we do not prohibit this modification? 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-
The primary conservative consideration is to prevent unforeseen consequences by introducing this restriction.
-
Since the ADC maintains in-memory state and different modes share the same cache_key, switching modes may affect the stored results.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- I understand that position.
However, please confirm: if I delete the GatewayProxy resource and recreate it with the same name using a different mode, no resources will be deleted in cascade, and the Ingress Controller will begin pushing configurations in the new mode, correct?
Once you have confirmed the issue, I shall approve and merge it. We will keep this restriction in place for the time being.
- The cache will only exist on a single backend. It will not be shared across different backend instances. This is not a problem.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Yes, when deleting GatewayProxy, it will only delete the association relationship, not the routing resources.
| if err := r.Get(ctx, req.NamespacedName, &gp); err != nil { | |
| if client.IgnoreNotFound(err) == nil { | |
| gp.Namespace = req.Namespace | |
| gp.Name = req.Name | |
| err = r.Provider.Update(ctx, tctx, &gp) | |
| } | |
| return ctrl.Result{}, err | |
| } |
- So the cache of ADC is distinguished by cache_key+type?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Essentially, yes. Each backend establishes its own cache based on its requirements and maintains it internally without sharing.
Currently, only the apisix-standalone backend incorporates caching; the other backend types do not utilise caching.
The type will be determined by the backend value you input during the sync operation and does not require explicit specification in the cacheKey. Naturally, given that the cacheKey is a transparent string, you may also specify it explicitly if desired.
Type of change:
What this PR does / why we need it:
By default, static configuration will prevail, and the deployment mode of the data plane can be dynamically specified.
mode: apisixormode: apisix-standalonePre-submission checklist: