Skip to content

Commit 9052abc

Browse files
privateipprivateip
authored
Merge pull request #162 from datum-cloud/chore/remove-gobgp-server
chore: remove gobgp-grpc-server-enabled feature flag
2 parents 9e2e9d4 + 1ee02f9 commit 9052abc

7 files changed

Lines changed: 3 additions & 202 deletions

File tree

‎cmd/galactic-router/main.go‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,6 @@ const (
2828
resourceBGPVRFInstances = "bgpvrfinstances"
2929
resourceSecrets = "secrets"
3030
resourceNodes = "nodes"
31-
32-
defaultGrpcPort = 50051
3331
)
3432

3533
func main() {

‎cmd/galactic-router/root.go‎

Lines changed: 1 addition & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -47,8 +47,6 @@ const (
4747
func newViper() *viper.Viper {
4848
v := viper.New()
4949
v.SetDefault("galactic_router.bgp_listen_port", 179)
50-
v.SetDefault("galactic_router.gobgp_grpc_port", defaultGrpcPort)
51-
v.SetDefault("galactic_router.gobgp_grpc_server_enabled", false)
5250
v.SetDefault("galactic_router.metrics_port", 8080)
5351
v.SetDefault("galactic_router.grpc_health_port", 5000)
5452
v.SetDefault("galactic_router.gc_namespace", "default")
@@ -68,10 +66,6 @@ func newViper() *viper.Viper {
6866
//nolint:errcheck
6967
v.BindEnv("galactic_router.bgp_local_address", "GALACTIC_ROUTER_BGP_LOCAL_ADDRESS")
7068
//nolint:errcheck
71-
v.BindEnv("galactic_router.gobgp_grpc_server_enabled", "GALACTIC_ROUTER_GOBGP_GRPC_SERVER_ENABLED")
72-
//nolint:errcheck
73-
v.BindEnv("galactic_router.gobgp_grpc_port", "GALACTIC_ROUTER_GOBGP_GRPC_PORT")
74-
//nolint:errcheck
7569
v.BindEnv("galactic_router.metrics_port", "GALACTIC_ROUTER_METRICS_PORT")
7670
//nolint:errcheck
7771
v.BindEnv("galactic_router.grpc_health_port", "GALACTIC_ROUTER_GRPC_HEALTH_PORT")
@@ -99,12 +93,6 @@ func bindFlags(cmd *cobra.Command, v *viper.Viper) { //nolint:lll // cobra flag
9993
cmd.Flags().StringP("bgp-local-address", "",
10094
v.GetString("galactic_router.bgp_local_address"),
10195
"Source address for outgoing BGP TCP connections")
102-
cmd.Flags().BoolP("gobgp-grpc-server-enabled", "",
103-
v.GetBool("galactic_router.gobgp_grpc_server_enabled"),
104-
"Enable the embedded GoBGP gRPC API server")
105-
cmd.Flags().IntP("gobgp-grpc-port", "",
106-
v.GetInt("galactic_router.gobgp_grpc_port"),
107-
"Port for the GoBGP gRPC API server")
10896
cmd.Flags().IntP("metrics-port", "",
10997
v.GetInt("galactic_router.metrics_port"),
11098
"Port for the controller-runtime metrics server")
@@ -128,7 +116,6 @@ func validateConfig(v *viper.Viper) error {
128116
nodeName := v.GetString("galactic_router.node_name")
129117
routerRole := v.GetString("galactic_router.router_role")
130118
bgpListenPort := v.GetInt("galactic_router.bgp_listen_port")
131-
grpcPort := v.GetInt("galactic_router.gobgp_grpc_port")
132119
metricsPort := v.GetInt("galactic_router.metrics_port")
133120
grpcHealthPort := v.GetInt("galactic_router.grpc_health_port")
134121

@@ -144,9 +131,6 @@ func validateConfig(v *viper.Viper) error {
144131
if bgpListenPort < -1 || bgpListenPort > 65535 {
145132
return fmt.Errorf("GALACTIC_ROUTER_BGP_LISTEN_PORT must be -1 or a valid port number, got %d", bgpListenPort)
146133
}
147-
if grpcPort < 1 || grpcPort > 65535 {
148-
return fmt.Errorf("GALACTIC_ROUTER_GOBGP_GRPC_PORT must be a valid port number (1-65535), got %d", grpcPort)
149-
}
150134
if metricsPort < 1 || metricsPort > 65535 {
151135
return fmt.Errorf("GALACTIC_ROUTER_METRICS_PORT must be a valid port number (1-65535), got %d", metricsPort)
152136
}
@@ -167,21 +151,13 @@ func runCmd(v *viper.Viper) error {
167151
routerRole := v.GetString("galactic_router.router_role")
168152
bgpListenPort := v.GetInt("galactic_router.bgp_listen_port")
169153
bgpLocalAddr := v.GetString("galactic_router.bgp_local_address")
170-
grpcEnabled := v.GetBool("galactic_router.gobgp_grpc_server_enabled")
171-
grpcPort := v.GetInt("galactic_router.gobgp_grpc_port")
172154
metricsPort := v.GetInt("galactic_router.metrics_port")
173155
grpcHealthPort := v.GetInt("galactic_router.grpc_health_port")
174156

175-
// Construct gRPC listen address.
176-
var grpcListenAddress string
177-
if grpcEnabled {
178-
grpcListenAddress = fmt.Sprintf(":%d", grpcPort)
179-
}
180-
181157
var factory galacticruntime.RuntimeFactory
182158
switch routerRole {
183159
case "tenant":
184-
factory = gobgp.NewRuntimeFactory(int32(bgpListenPort), bgpLocalAddr, grpcListenAddress)
160+
factory = gobgp.NewRuntimeFactory(int32(bgpListenPort), bgpLocalAddr)
185161
case roleFabric:
186162
factory = frr.NewRuntimeFactory()
187163
}

‎cmd/galactic-router/root_test.go‎

Lines changed: 0 additions & 108 deletions
Original file line numberDiff line numberDiff line change
@@ -32,12 +32,6 @@ func TestFlagDefaults(t *testing.T) {
3232
if v.GetInt("galactic_router.bgp_listen_port") != 179 {
3333
t.Errorf("bgp_listen_port default = %d, want 179", v.GetInt("galactic_router.bgp_listen_port"))
3434
}
35-
if v.GetInt("galactic_router.gobgp_grpc_port") != defaultGrpcPort {
36-
t.Errorf("gobgp_grpc_port default = %d, want %d", v.GetInt("galactic_router.gobgp_grpc_port"), defaultGrpcPort)
37-
}
38-
if v.GetBool("galactic_router.gobgp_grpc_server_enabled") != false {
39-
t.Errorf("gobgp_grpc_server_enabled default = %v, want false", v.GetBool("galactic_router.gobgp_grpc_server_enabled"))
40-
}
4135
if v.GetInt("galactic_router.metrics_port") != 8080 {
4236
t.Errorf("metrics_port default = %d, want 8080", v.GetInt("galactic_router.metrics_port"))
4337
}
@@ -57,53 +51,13 @@ func TestRequiredFlags(t *testing.T) {
5751
func TestEnvVarDefaults(t *testing.T) {
5852
t.Setenv("GALACTIC_ROUTER_NODE_NAME", "test-node")
5953
t.Setenv("GALACTIC_ROUTER_ROUTER_ROLE", "tenant")
60-
t.Setenv("GALACTIC_ROUTER_GOBGP_GRPC_SERVER_ENABLED", "false")
6154

6255
v := cmdWithViper(t)
6356
if err := validateConfig(v); err != nil {
6457
t.Errorf("validateConfig with valid env vars: %v", err)
6558
}
6659
}
6760

68-
func TestGRPCPortEnvVar(t *testing.T) {
69-
t.Setenv("GALACTIC_ROUTER_NODE_NAME", "test-node")
70-
t.Setenv("GALACTIC_ROUTER_ROUTER_ROLE", "tenant")
71-
t.Setenv("GALACTIC_ROUTER_GOBGP_GRPC_SERVER_ENABLED", "true")
72-
t.Setenv("GALACTIC_ROUTER_GOBGP_GRPC_PORT", "9999")
73-
74-
v := cmdWithViper(t)
75-
if err := validateConfig(v); err != nil {
76-
t.Errorf("validateConfig with GALACTIC_ROUTER_GOBGP_GRPC_PORT=9999: %v", err)
77-
}
78-
}
79-
80-
func TestFlagOverridesEnv(t *testing.T) {
81-
t.Setenv("GALACTIC_ROUTER_GOBGP_GRPC_PORT", "9999")
82-
83-
v := cmdWithViper(t)
84-
// Simulate a flag override by setting the viper key directly (v.Set
85-
// has higher precedence than env vars in viper).
86-
v.Set("galactic_router.gobgp_grpc_port", 12345)
87-
if got := v.GetInt("galactic_router.gobgp_grpc_port"); got != 12345 {
88-
t.Errorf("flag override: got %d, want 12345", got)
89-
}
90-
}
91-
92-
func TestEnvVarOnly(t *testing.T) {
93-
t.Setenv("GALACTIC_ROUTER_NODE_NAME", "env-node")
94-
t.Setenv("GALACTIC_ROUTER_ROUTER_ROLE", "tenant")
95-
t.Setenv("GALACTIC_ROUTER_GOBGP_GRPC_SERVER_ENABLED", "true")
96-
t.Setenv("GALACTIC_ROUTER_GOBGP_GRPC_PORT", "7777")
97-
98-
v := cmdWithViper(t)
99-
if v.GetString("galactic_router.node_name") != "env-node" {
100-
t.Errorf("node_name from env = %q, want %q", v.GetString("galactic_router.node_name"), "env-node")
101-
}
102-
if v.GetInt("galactic_router.gobgp_grpc_port") != 7777 {
103-
t.Errorf("gobgp_grpc_port from env = %d, want 7777", v.GetInt("galactic_router.gobgp_grpc_port"))
104-
}
105-
}
106-
10761
func TestInvalidRouterRole(t *testing.T) {
10862
t.Setenv("GALACTIC_ROUTER_NODE_NAME", "test-node")
10963
t.Setenv("GALACTIC_ROUTER_ROUTER_ROLE", "invalid")
@@ -115,32 +69,6 @@ func TestInvalidRouterRole(t *testing.T) {
11569
}
11670
}
11771

118-
func TestGRPCPortOverflow(t *testing.T) {
119-
t.Setenv("GALACTIC_ROUTER_NODE_NAME", "test-node")
120-
t.Setenv("GALACTIC_ROUTER_ROUTER_ROLE", "tenant")
121-
t.Setenv("GALACTIC_ROUTER_GOBGP_GRPC_SERVER_ENABLED", "true")
122-
t.Setenv("GALACTIC_ROUTER_GOBGP_GRPC_PORT", "70000")
123-
124-
v := cmdWithViper(t)
125-
err := validateConfig(v)
126-
if err == nil {
127-
t.Error("validateConfig with port 70000 returned nil error")
128-
}
129-
}
130-
131-
func TestGRPCPortZero(t *testing.T) {
132-
t.Setenv("GALACTIC_ROUTER_NODE_NAME", "test-node")
133-
t.Setenv("GALACTIC_ROUTER_ROUTER_ROLE", "tenant")
134-
t.Setenv("GALACTIC_ROUTER_GOBGP_GRPC_SERVER_ENABLED", "true")
135-
t.Setenv("GALACTIC_ROUTER_GOBGP_GRPC_PORT", "0")
136-
137-
v := cmdWithViper(t)
138-
err := validateConfig(v)
139-
if err == nil {
140-
t.Error("validateConfig with port 0 returned nil error")
141-
}
142-
}
143-
14472
func TestBGPListenPortMinusOne(t *testing.T) {
14573
t.Setenv("GALACTIC_ROUTER_NODE_NAME", "test-node")
14674
t.Setenv("GALACTIC_ROUTER_ROUTER_ROLE", "tenant")
@@ -188,7 +116,6 @@ func TestValidRoles(t *testing.T) {
188116
t.Run(role, func(t *testing.T) {
189117
t.Setenv("GALACTIC_ROUTER_NODE_NAME", "test-node")
190118
t.Setenv("GALACTIC_ROUTER_ROUTER_ROLE", role)
191-
t.Setenv("GALACTIC_ROUTER_GOBGP_GRPC_SERVER_ENABLED", "false")
192119

193120
v := cmdWithViper(t)
194121
// Verify the role is read correctly.
@@ -225,33 +152,6 @@ func TestGRPCHealthPortOverride(t *testing.T) {
225152
}
226153
}
227154

228-
func TestGRPCServerEnabled(t *testing.T) {
229-
t.Setenv("GALACTIC_ROUTER_NODE_NAME", "test-node")
230-
t.Setenv("GALACTIC_ROUTER_ROUTER_ROLE", "tenant")
231-
t.Setenv("GALACTIC_ROUTER_GOBGP_GRPC_SERVER_ENABLED", "true")
232-
t.Setenv("GALACTIC_ROUTER_GOBGP_GRPC_PORT", "50051")
233-
234-
v := cmdWithViper(t)
235-
if !v.GetBool("galactic_router.gobgp_grpc_server_enabled") {
236-
t.Error("gobgp_grpc_server_enabled should be true")
237-
}
238-
if v.GetInt("galactic_router.gobgp_grpc_port") != 50051 {
239-
t.Errorf("gobgp_grpc_port = %d, want 50051", v.GetInt("galactic_router.gobgp_grpc_port"))
240-
}
241-
}
242-
243-
func TestGRPCServerDisabled(t *testing.T) {
244-
t.Setenv("GALACTIC_ROUTER_NODE_NAME", "test-node")
245-
t.Setenv("GALACTIC_ROUTER_ROUTER_ROLE", "tenant")
246-
t.Setenv("GALACTIC_ROUTER_GOBGP_GRPC_SERVER_ENABLED", "false")
247-
t.Setenv("GALACTIC_ROUTER_GOBGP_GRPC_PORT", "9999")
248-
249-
v := cmdWithViper(t)
250-
if v.GetBool("galactic_router.gobgp_grpc_server_enabled") {
251-
t.Error("gobgp_grpc_server_enabled should be false")
252-
}
253-
}
254-
255155
func TestVersionFlag(t *testing.T) {
256156
if metadata.Version == "" {
257157
t.Error("metadata.Version should not be empty")
@@ -264,8 +164,6 @@ func TestDefaults(t *testing.T) {
264164
_ = os.Unsetenv("GALACTIC_ROUTER_ROUTER_ROLE")
265165
_ = os.Unsetenv("GALACTIC_ROUTER_BGP_LISTEN_PORT")
266166
_ = os.Unsetenv("GALACTIC_ROUTER_BGP_LOCAL_ADDRESS")
267-
_ = os.Unsetenv("GALACTIC_ROUTER_GOBGP_GRPC_SERVER_ENABLED")
268-
_ = os.Unsetenv("GALACTIC_ROUTER_GOBGP_GRPC_PORT")
269167
_ = os.Unsetenv("GALACTIC_ROUTER_METRICS_PORT")
270168
_ = os.Unsetenv("GALACTIC_ROUTER_GRPC_HEALTH_PORT")
271169

@@ -274,12 +172,6 @@ func TestDefaults(t *testing.T) {
274172
if v.GetInt("galactic_router.bgp_listen_port") != 179 {
275173
t.Errorf("bgp_listen_port = %d, want 179", v.GetInt("galactic_router.bgp_listen_port"))
276174
}
277-
if v.GetInt("galactic_router.gobgp_grpc_port") != defaultGrpcPort {
278-
t.Errorf("gobgp_grpc_port = %d, want %d", v.GetInt("galactic_router.gobgp_grpc_port"), defaultGrpcPort)
279-
}
280-
if v.GetBool("galactic_router.gobgp_grpc_server_enabled") != false {
281-
t.Error("gobgp_grpc_server_enabled default should be false")
282-
}
283175
if v.GetInt("galactic_router.metrics_port") != 8080 {
284176
t.Errorf("metrics_port = %d, want 8080", v.GetInt("galactic_router.metrics_port"))
285177
}

‎docs/router/configuration.md‎

Lines changed: 0 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,6 @@ or a combination of both. CLI flags take precedence over environment variables.
1111
| Router role | `GALACTIC_ROUTER_ROUTER_ROLE` | `--router-role` | _(required)_ |
1212
| BGP listen port | `GALACTIC_ROUTER_BGP_LISTEN_PORT` | `--bgp-listen-port` | `179` |
1313
| BGP local address | `GALACTIC_ROUTER_BGP_LOCAL_ADDRESS` | `--bgp-local-address` | `""` |
14-
| GoBGP gRPC server | `GALACTIC_ROUTER_GOBGP_GRPC_SERVER_ENABLED` | `--gobgp-grpc-server-enabled` | `false` |
15-
| GoBGP gRPC port | `GALACTIC_ROUTER_GOBGP_GRPC_PORT` | `--gobgp-grpc-port` | `50051` |
1614
| Metrics port | `GALACTIC_ROUTER_METRICS_PORT` | `--metrics-port` | `8080` |
1715
| gRPC health port | `GALACTIC_ROUTER_GRPC_HEALTH_PORT` | `--grpc-health-port` | `5000` |
1816

@@ -62,23 +60,6 @@ selects the default source address.
6260
**Type:** string
6361
**Default:** `""` (empty)
6462

65-
### `--gobgp-grpc-server-enabled` / `GALACTIC_ROUTER_GOBGP_GRPC_SERVER_ENABLED`
66-
67-
Enable the embedded GoBGP gRPC API server. When enabled, the gRPC server
68-
listens on the port specified by `--gobgp-grpc-port`.
69-
70-
**Type:** boolean
71-
**Default:** `false`
72-
73-
### `--gobgp-grpc-port` / `GALACTIC_ROUTER_GOBGP_GRPC_PORT`
74-
75-
TCP port for the GoBGP gRPC API server. Only used when
76-
`--gobgp-grpc-server-enabled` is `true`.
77-
78-
**Type:** integer
79-
**Default:** `50051`
80-
**Valid values:** `1`–`65535`
81-
8263
### `--metrics-port` / `GALACTIC_ROUTER_METRICS_PORT`
8364

8465
TCP port for the controller-runtime metrics HTTP server. Exposes Prometheus
@@ -130,8 +111,6 @@ args:
130111
- --node-name=$(GALACTIC_ROUTER_NODE_NAME)
131112
- --router-role=tenant
132113
- --metrics-port=9090
133-
- --gobgp-grpc-server-enabled=true
134-
- --gobgp-grpc-port=50051
135114
env:
136115
- name: GALACTIC_ROUTER_NODE_NAME
137116
valueFrom:

‎internal/runtime/gobgp/runtime.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,11 +53,11 @@ type GoBGPRuntime struct {
5353
// Pass -1 to disable inbound connections (outbound-only mode).
5454
// localAddress, if non-empty, is bound as the source address for outgoing BGP
5555
// TCP connections (sets Transport.LocalAddress on every peer).
56-
func NewRuntimeFactory(listenPort int32, localAddress string, grpcListenAddress string) runtime.RuntimeFactory {
56+
func NewRuntimeFactory(listenPort int32, localAddress string) runtime.RuntimeFactory {
5757
return func(key types.NamespacedName) (runtime.RouterRuntime, error) {
5858
return &GoBGPRuntime{
5959
key: key,
60-
server: newServer(Config{GRPCListenAddress: grpcListenAddress}),
60+
server: newServer(Config{}),
6161
listenPort: listenPort,
6262
localAddress: localAddress,
6363
establishedAt: make(map[string]time.Time),

‎internal/runtime/gobgp/server.go‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,6 @@ type Config struct {
2323
// LogLevel controls GoBGP's internal log verbosity.
2424
// Valid values: debug, info, warn, error, panic. Defaults to panic.
2525
LogLevel string
26-
// GRPCListenAddress is the TCP address the GoBGP gRPC API listens on.
27-
// An empty string (default) disables the gRPC server entirely.
28-
GRPCListenAddress string
2926
}
3027

3128
// Server wraps an embedded GoBGP BgpServer.
@@ -89,9 +86,6 @@ func (s *Server) buildServerOptions() []gobgpserver.ServerOption {
8986
levelVar.Set(level)
9087
logger := slog.New(slog.NewTextHandler(io.Discard, &slog.HandlerOptions{Level: levelVar}))
9188
opts := []gobgpserver.ServerOption{gobgpserver.LoggerOption(logger, levelVar)}
92-
if s.cfg.GRPCListenAddress != "" {
93-
opts = append(opts, gobgpserver.GrpcListenAddress(s.cfg.GRPCListenAddress))
94-
}
9589
return opts
9690
}
9791

‎internal/runtime/gobgp/server_test.go‎

Lines changed: 0 additions & 38 deletions
This file was deleted.

0 commit comments

Comments
 (0)