Remove data-plane VIP/BGP/OSPF remnants#891
Merged
Merged
Conversation
…, and frontend Complete cleanup of data-plane VIP management code that is now handled by the CNI (NovaNet/Cilium/MetalLB) via Service type=LoadBalancer. This removes: - VIP CRUD from Backend interface and both Kubernetes/Standalone implementations - VIP models (VIP, VIPStatus, BGPConfig, OSPFConfig, VIPRef) from webui models - VIP routes, handlers, and dispatch functions from webui server - VIP metrics (VIPStatus, BGPSessionStatus, OSPFNeighborStatus) from agent metrics - VIP error types (VIPError, ErrVIPOperationFailed) from agent errors - VIPConfig, ClusterBGPConfig, ClusterBGPPeer from CRD types and deepcopy - VIP configuration injection from operator controller - VIP config types from standalone config - VIP page, API client, hooks, normalizer, types from React frontend - VIP sidebar navigation and dashboard counter Control-plane VIP (cpvip) for controller HA is intentionally preserved as it is a separate feature from data-plane VIP management.
📊 Benchmark Results
|
There was a problem hiding this comment.
Pull request overview
This PR removes the legacy data-plane VIP/BGP/OSPF configuration surface across the Go control-plane/webui and the React frontend, aligning the product with the new model where external CNIs / Service type=LoadBalancer own VIP allocation/advertisement (while preserving control-plane HA VIP separately).
Changes:
- Removed VIP-related frontend pages, API client/hooks/normalizers, and dashboard/sidebar references.
- Removed VIP configuration and related CRUD/handlers from the WebUI backend (novactl) and standalone mode config.
- Removed VIP config wiring from the operator agent DaemonSet args, plus VIP metrics and error types from the agent.
Reviewed changes
Copilot reviewed 24 out of 24 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| web/src/pages/Dashboard.tsx | Removes VIP counts/cards from the dashboard resource summary. |
| web/src/components/layout/Sidebar.tsx | Removes VIP navigation entry from sidebar. |
| web/src/components/config/ImportDialog.tsx | Stops invalidating VIP query cache after import. |
| web/src/components/config/HistoryDialog.tsx | Stops invalidating VIP query cache after history actions. |
| web/src/api/types.ts | Removes VIP types and vip-related fields from shared frontend API typings. |
| web/src/api/normalizer.ts | Removes VIP normalizer helpers and gateway vipRef normalization. |
| web/src/api/hooks.ts | Removes VIP React Query hooks and narrows bulk-delete resource types. |
| web/src/api/client.ts | Removes VIP API client endpoints and normalization wiring. |
| web/src/App.tsx | Removes VIP route/lazy import so the page is no longer reachable. |
| internal/standalone/watcher.go | Removes VIP count logging from standalone config watcher. |
| internal/standalone/config.go | Removes standalone YAML VIP/BGP/OSPF/BFD config types/fields. |
| internal/operator/controller/novaedgecluster_controller.go | Removes VIP-related agent args injection; simplifies args assembly. |
| internal/agent/metrics/cluster_metrics.go | Removes VIP/BGP/OSPF Prometheus metrics and setters. |
| internal/agent/errors_test.go | Removes VIP error tests and common error assertions. |
| internal/agent/errors.go | Removes VIP error types/constants and updates package doc comment. |
| cmd/novactl/pkg/webui/server.go | Removes VIP API routes/handlers and VIP failover metric from dashboard metrics. |
| cmd/novactl/pkg/webui/models/config.go | Removes VIP models and gateway vipRef; renames IP allocation field from vipRef to owner. |
| cmd/novactl/pkg/webui/mode/standalone.go | Removes standalone backend VIP CRUD implementation and import/export handling. |
| cmd/novactl/pkg/webui/mode/mode.go | Removes VIP methods from the WebUI backend interface. |
| cmd/novactl/pkg/webui/mode/kubernetes.go | Removes ProxyVIP dynamic-client wiring and VIP import/export support. |
| cmd/novactl/pkg/webui/handlers_write.go | Removes VIP write handler and updates history metadata comment. |
| api/v1alpha1/zz_generated.deepcopy.go | Removes deepcopy support for VIPConfig/BGP peer types removed from APIs. |
| api/v1alpha1/novaedgeremotecluster_types.go | Removes RemoteAgentSpec VIP config field. |
| api/v1alpha1/novaedgecluster_types.go | Removes AgentSpec VIP/BGP config types/fields. |
Comments suppressed due to low confidence (3)
web/src/api/hooks.ts:301
- TypeScript build will fail because VIP-related call sites still exist in the frontend even though the VIP hooks were removed here. In particular,
web/src/pages/VIPs.tsxstill importsuseVIPs/useCreateVIP/useUpdateVIP/useDeleteVIPand callsuseBulkDelete('vips'), but those exports and the'vips'resource type are no longer available. Sinceweb/tsconfig.jsonincludes all ofsrc/, this file must be deleted or updated/excluded to keeppnpm build/tscpassing.
// Bulk delete mutation
export function useBulkDelete(resourceType: 'gateways' | 'routes' | 'backends' | 'policies') {
const queryClient = useQueryClient()
const apiResource = api[resourceType]
return useMutation({
mutationFn: async (items: { namespace: string; name: string }[]) => {
const results = await Promise.allSettled(
api/v1alpha1/novaedgecluster_types.go:203
- The Go types remove
.spec.agent.vip, but the generated CRD manifests are still advertising the VIP schema (e.g.config/crd/novaedge.io_novaedgeclusters.yamlstill contains.spec.agent.vipwith BGP/OSPF properties). This will leave the published CRDs out of sync with the controller types and can cause validation/UX confusion during upgrades. Regenerate and commit the CRDs/Helm chart CRDs so the VIP fields are fully removed from the published schemas.
// HealthPort is the port for health probes
// +kubebuilder:default=8080
// +kubebuilder:validation:Minimum=1
// +kubebuilder:validation:Maximum=65535
// +optional
HealthPort *int32 `json:"healthPort,omitempty"`
// ServiceAccount defines the service account configuration
// +optional
ServiceAccount *ServiceAccountSpec `json:"serviceAccount,omitempty"`
// PodSecurityContext defines the security context for agent pods
// +optional
PodSecurityContext *corev1.PodSecurityContext `json:"podSecurityContext,omitempty"`
api/v1alpha1/novaedgeremotecluster_types.go:229
- Similar to NovaEdgeCluster: this spec no longer includes
.spec.agent.vip, but the generated remote-cluster CRD still includes avipfield (config/crd/novaedge.io_novaedgeremoteclusters.yamlhas.spec.agent.vip). Please regenerate/update the published CRD YAMLs (and any Helm chart CRDs) to remove this field so the schema matches the Go types.
// RemoteAgentSpec defines agent configuration specific to this remote cluster
type RemoteAgentSpec struct {
// Version overrides the agent version for this cluster
// If not set, uses the hub cluster's default version
// +optional
Version string `json:"version,omitempty"`
// NodeSelector defines which nodes should run agents in the remote cluster
// +optional
NodeSelector map[string]string `json:"nodeSelector,omitempty"`
// Tolerations for agent pods in the remote cluster
// +optional
Tolerations []corev1.Toleration `json:"tolerations,omitempty"`
// Resources defines resource requirements for agents in this cluster
// +optional
Resources corev1.ResourceRequirements `json:"resources,omitempty"`
// ExtraArgs are additional arguments passed to agents
// +optional
ExtraArgs []string `json:"extraArgs,omitempty"`
// ExtraEnv are additional environment variables for agents
// +optional
ExtraEnv []corev1.EnvVar `json:"extraEnv,omitempty"`
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const RoutesPage = lazy(() => import('./pages/Routes')) | ||
| const Backends = lazy(() => import('./pages/Backends')) | ||
| const VIPs = lazy(() => import('./pages/VIPs')) | ||
| const Policies = lazy(() => import('./pages/Policies')) |
There was a problem hiding this comment.
Removing the /vips route is good, but the page implementation still exists at web/src/pages/VIPs.tsx and still depends on the now-removed VIP types/hooks. Because web/tsconfig.json includes the entire src/ tree, that file will still be type-checked and will break npx tsc --noEmit / pnpm build unless it’s deleted (or otherwise excluded/updated).
Suggested change
| const Policies = lazy(() => import('./pages/Policies')) |
| listeners?: Listener[] | ||
| vipRef?: ObjectRef | ||
| tracing?: Tracing | ||
| accessLog?: AccessLog |
There was a problem hiding this comment.
The frontend types no longer include GatewaySpec.vipRef, but the operator Helm CRD still requires it: charts/novaedge-operator/crds/novaedge.io_proxygateways.yaml contains .spec.vipRef in additionalPrinterColumns and lists it as a required field. This mismatch will block CRD installation/validation for users deploying via the operator chart; the chart CRD should be regenerated/updated to remove vipRef (and any ProxyVIP references) in lockstep with this API change.
Suggested change
| accessLog?: AccessLog | |
| accessLog?: AccessLog | |
| vipRef?: ObjectRef |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Service type=LoadBalancerFiles Changed
VIPConfig,ClusterBGPConfig,ClusterBGPPeerfrom CRD types and deepcopyTest plan
go build ./...passesgolangci-lint run ./...passes (0 issues)go test ./internal/agent/... ./api/... ./cmd/novactl/... ./internal/standalone/... ./internal/operator/...passes (gossip test failure is pre-existing)