Skip to content

Commit 5f03648

Browse files
rosmorosmo
authored
Merge pull request #103 from agh95/default-max-rate-per-endpoint
add config for default max_rate_per_endpoint and max_connections_per_…
2 parents 81640d3 + a00c4c9 commit 5f03648

4 files changed

Lines changed: 143 additions & 17 deletions

File tree

‎controllers/autoneg.go‎

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -408,7 +408,7 @@ func validateNewConfig(config AutonegConfig) error {
408408
return nil
409409
}
410410

411-
func getStatuses(namespace string, name string, annotations map[string]string, serviceNameTemplate string, allowServiceName bool) (s Statuses, valid bool, err error) {
411+
func getStatuses(namespace string, name string, annotations map[string]string, r *ServiceReconciler) (s Statuses, valid bool, err error) {
412412
// Read the current cloud.google.com/neg annotation
413413
tmp, ok := annotations[negAnnotation]
414414
if ok {
@@ -433,10 +433,20 @@ func getStatuses(namespace string, name string, annotations map[string]string, s
433433
for port, cfgs := range tempConfig.BackendServices {
434434
s.config.BackendServices[port] = make(map[string]AutonegNEGConfig, len(cfgs))
435435
for _, cfg := range cfgs {
436-
if cfg.Name == "" || !allowServiceName {
436+
if cfg.Name == "" || !r.AllowServiceName {
437437
// Default to name generated using serviceNameTemplate
438-
cfg.Name = generateServiceName(namespace, name, port, serviceNameTemplate)
438+
cfg.Name = generateServiceName(namespace, name, port, r.ServiceNameTemplate)
439439
}
440+
441+
//Use defaults if rate and connections have not been set
442+
if cfg.Rate == 0 && cfg.Connections == 0 {
443+
if r.MaxRatePerEndpointDefault > 0 {
444+
cfg.Rate = r.MaxRatePerEndpointDefault
445+
} else {
446+
cfg.Connections = r.MaxConnectionsPerEndpointDefault
447+
}
448+
}
449+
440450
s.config.BackendServices[port][cfg.Name] = cfg
441451
}
442452
}

‎controllers/autoneg_test.go‎

Lines changed: 112 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -237,8 +237,12 @@ var oldStatusTests = []struct {
237237
}
238238

239239
func TestGetStatuses(t *testing.T) {
240+
var serviceReconciler = ServiceReconciler{
241+
ServiceNameTemplate: "{namespace}-{name}-{port}-{hash}",
242+
AllowServiceName: true,
243+
}
240244
for _, st := range statusTests {
241-
_, valid, err := getStatuses("ns", "test", st.annotations, "{namespace}-{name}-{port}-{hash}", true)
245+
_, valid, err := getStatuses("ns", "test", st.annotations, &serviceReconciler)
242246
if err != nil && !st.err {
243247
t.Errorf("Set %q: expected no error, got one: %v", st.name, err)
244248
}
@@ -255,8 +259,12 @@ func TestGetStatuses(t *testing.T) {
255259
}
256260

257261
func TestGetOldStatuses(t *testing.T) {
262+
var serviceReconciler = ServiceReconciler{
263+
ServiceNameTemplate: "{namespace}-{name}-{port}-{hash}",
264+
AllowServiceName: true,
265+
}
258266
for _, st := range oldStatusTests {
259-
_, valid, err := getStatuses("ns", "test", st.annotations, "{namespace}-{name}-{port}-{hash}", true)
267+
_, valid, err := getStatuses("ns", "test", st.annotations, &serviceReconciler)
260268
if err != nil && !st.err {
261269
t.Errorf("Set %q: expected no error, got one: %v", st.name, err)
262270
}
@@ -273,8 +281,12 @@ func TestGetOldStatuses(t *testing.T) {
273281
}
274282

275283
func TestGetStatusesServiceNameNotAllowed(t *testing.T) {
284+
var serviceReconciler = ServiceReconciler{
285+
ServiceNameTemplate: "{namespace}-{name}-{port}",
286+
AllowServiceName: false,
287+
}
276288
validConf := `{"backend_services":{"80":[{"name":"http-be","max_rate_per_endpoint":100}]}}`
277-
statuses, valid, err := getStatuses("ns", "test", map[string]string{autonegAnnotation: validConf}, "{namespace}-{name}-{port}", false)
289+
statuses, valid, err := getStatuses("ns", "test", map[string]string{autonegAnnotation: validConf}, &serviceReconciler)
278290
if err != nil {
279291
t.Errorf("Expected no error, got one: %v", err)
280292
}
@@ -288,8 +300,12 @@ func TestGetStatusesServiceNameNotAllowed(t *testing.T) {
288300
}
289301

290302
func TestGetStatusesServiceNameAllowed(t *testing.T) {
303+
var serviceReconciler = ServiceReconciler{
304+
ServiceNameTemplate: "{namespace}-{name}-{port}",
305+
AllowServiceName: true,
306+
}
291307
validConf := `{"backend_services":{"80":[{"name":"http-be","max_rate_per_endpoint":100}]}}`
292-
statuses, valid, err := getStatuses("ns", "test", map[string]string{autonegAnnotation: validConf}, "{namespace}-{name}-{port}", true)
308+
statuses, valid, err := getStatuses("ns", "test", map[string]string{autonegAnnotation: validConf}, &serviceReconciler)
293309
if err != nil {
294310
t.Errorf("Expected no error, got one: %v", err)
295311
}
@@ -302,6 +318,98 @@ func TestGetStatusesServiceNameAllowed(t *testing.T) {
302318
}
303319
}
304320

321+
func TestDefaultMaxRatePerEndpointWhenOverrideIsSet(t *testing.T) {
322+
var serviceReconciler = ServiceReconciler{
323+
ServiceNameTemplate: "{namespace}-{name}-{port}",
324+
AllowServiceName: true,
325+
MaxRatePerEndpointDefault: 1234,
326+
}
327+
validConf := `{"backend_services":{"80":[{"name":"http-be","max_rate_per_endpoint":100}]}}`
328+
statuses, valid, err := getStatuses("ns", "test", map[string]string{autonegAnnotation: validConf}, &serviceReconciler)
329+
if err != nil {
330+
t.Errorf("Expected no error, got one: %v", err)
331+
}
332+
if !valid {
333+
t.Errorf("Expected autoneg config, got none")
334+
}
335+
cfg, ok := statuses.config.BackendServices["80"]["http-be"]
336+
if !ok {
337+
t.Errorf("Expected service config for http-be but got none, service statuses: \n%v", statuses.config.BackendServices)
338+
}
339+
if cfg.Rate != 100 {
340+
t.Errorf("Expected max_rate_per_endpoint to be 100 but got: \n%v", cfg.Rate)
341+
}
342+
}
343+
344+
func TestDefaultMaxRatePerEndpointWhenOverrideIsNotSet(t *testing.T) {
345+
var serviceReconciler = ServiceReconciler{
346+
ServiceNameTemplate: "{namespace}-{name}-{port}",
347+
AllowServiceName: true,
348+
MaxRatePerEndpointDefault: 1234,
349+
}
350+
validConf := `{"backend_services":{"80":[{"name":"http-be"}]}}`
351+
statuses, valid, err := getStatuses("ns", "test", map[string]string{autonegAnnotation: validConf}, &serviceReconciler)
352+
if err != nil {
353+
t.Errorf("Expected no error, got one: %v", err)
354+
}
355+
if !valid {
356+
t.Errorf("Expected autoneg config, got none")
357+
}
358+
cfg, ok := statuses.config.BackendServices["80"]["http-be"]
359+
if !ok {
360+
t.Errorf("Expected service config for http-be but got none, service statuses: \n%v", statuses.config.BackendServices)
361+
}
362+
if cfg.Rate != 1234 {
363+
t.Errorf("Expected max_rate_per_endpoint to be 1234 but got: \n%v", cfg.Rate)
364+
}
365+
}
366+
367+
func TestDefaultConnectionPerEndpointWhenOverrideIsSet(t *testing.T) {
368+
var serviceReconciler = ServiceReconciler{
369+
ServiceNameTemplate: "{namespace}-{name}-{port}",
370+
AllowServiceName: true,
371+
MaxConnectionsPerEndpointDefault: 1234,
372+
}
373+
validConf := `{"backend_services":{"80":[{"name":"http-be","max_connections_per_endpoint":100}]}}`
374+
statuses, valid, err := getStatuses("ns", "test", map[string]string{autonegAnnotation: validConf}, &serviceReconciler)
375+
if err != nil {
376+
t.Errorf("Expected no error, got one: %v", err)
377+
}
378+
if !valid {
379+
t.Errorf("Expected autoneg config, got none")
380+
}
381+
cfg, ok := statuses.config.BackendServices["80"]["http-be"]
382+
if !ok {
383+
t.Errorf("Expected service config for http-be but got none, service statuses: \n%v", statuses.config.BackendServices)
384+
}
385+
if cfg.Connections != 100 {
386+
t.Errorf("Expected max_rate_per_endpoint to be 100 but got: \n%v", cfg.Rate)
387+
}
388+
}
389+
390+
func TestDefaultMaxConnectionsEndpointWhenOverrideIsNotSet(t *testing.T) {
391+
var serviceReconciler = ServiceReconciler{
392+
ServiceNameTemplate: "{namespace}-{name}-{port}",
393+
AllowServiceName: true,
394+
MaxConnectionsPerEndpointDefault: 1234,
395+
}
396+
validConf := `{"backend_services":{"80":[{"name":"http-be"}]}}`
397+
statuses, valid, err := getStatuses("ns", "test", map[string]string{autonegAnnotation: validConf}, &serviceReconciler)
398+
if err != nil {
399+
t.Errorf("Expected no error, got one: %v", err)
400+
}
401+
if !valid {
402+
t.Errorf("Expected autoneg config, got none")
403+
}
404+
cfg, ok := statuses.config.BackendServices["80"]["http-be"]
405+
if !ok {
406+
t.Errorf("Expected service config for http-be but got none, service statuses: \n%v", statuses.config.BackendServices)
407+
}
408+
if cfg.Connections != 1234 {
409+
t.Errorf("Expected max_connections_per_endpoint to be 1234 but got: \n%v", cfg.Rate)
410+
}
411+
}
412+
305413
func TestValidateOldConfig(t *testing.T) {
306414
tests := []struct {
307415
name string

‎controllers/service_controller.go‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -37,9 +37,11 @@ type ServiceReconciler struct {
3737
client.Client
3838
Scheme *runtime.Scheme
3939
*BackendController
40-
Recorder record.EventRecorder
41-
ServiceNameTemplate string
42-
AllowServiceName bool
40+
Recorder record.EventRecorder
41+
ServiceNameTemplate string
42+
AllowServiceName bool
43+
MaxRatePerEndpointDefault float64
44+
MaxConnectionsPerEndpointDefault float64
4345
}
4446

4547
//+kubebuilder:rbac:groups=core,resources=services,verbs=get;list;watch;update;patch
@@ -70,7 +72,7 @@ func (r *ServiceReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct
7072
return reconcile.Result{}, err
7173
}
7274

73-
status, ok, err := getStatuses(svc.Namespace, svc.Name, svc.ObjectMeta.Annotations, r.ServiceNameTemplate, r.AllowServiceName)
75+
status, ok, err := getStatuses(svc.Namespace, svc.Name, svc.ObjectMeta.Annotations, r)
7476
// Is this service using autoneg?
7577
if !ok {
7678
return reconcile.Result{}, nil

‎main.go‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -56,11 +56,15 @@ func init() {
5656
func main() {
5757
var metricsAddr string
5858
var probeAddr string
59+
var maxRatePerEndpointDefault float64
60+
var maxConnectionsPerEndpointDefault float64
5961
var enableLeaderElection bool
6062
var serviceNameTemplate string
6163
var allowServiceName bool
6264
flag.StringVar(&metricsAddr, "metrics-bind-address", ":8080", "The address the metric endpoint binds to.")
6365
flag.StringVar(&probeAddr, "health-probe-bind-address", ":8081", "The address the probe endpoint binds to.")
66+
flag.Float64Var(&maxRatePerEndpointDefault, "max-rate-per-endpoint", 0, "The address the probe endpoint binds to.")
67+
flag.Float64Var(&maxConnectionsPerEndpointDefault, "max-connections-per-endpoint", 0, "The address the probe endpoint binds to.")
6468
flag.BoolVar(&enableLeaderElection, "leader-elect", false,
6569
"Enable leader election for controller manager. "+
6670
"Enabling this will ensure there is only one active controller manager.")
@@ -111,12 +115,14 @@ func main() {
111115
}
112116

113117
if err = (&controllers.ServiceReconciler{
114-
Client: mgr.GetClient(),
115-
Scheme: mgr.GetScheme(),
116-
BackendController: controllers.NewBackendController(project, s),
117-
Recorder: mgr.GetEventRecorderFor("autoneg-controller"),
118-
ServiceNameTemplate: serviceNameTemplate,
119-
AllowServiceName: allowServiceName,
118+
Client: mgr.GetClient(),
119+
Scheme: mgr.GetScheme(),
120+
BackendController: controllers.NewBackendController(project, s),
121+
Recorder: mgr.GetEventRecorderFor("autoneg-controller"),
122+
ServiceNameTemplate: serviceNameTemplate,
123+
AllowServiceName: allowServiceName,
124+
MaxRatePerEndpointDefault: maxRatePerEndpointDefault,
125+
MaxConnectionsPerEndpointDefault: maxConnectionsPerEndpointDefault,
120126
}).SetupWithManager(mgr); err != nil {
121127
setupLog.Error(err, "unable to create controller", "controller", "Service")
122128
os.Exit(1)

0 commit comments

Comments
 (0)