diff --git a/controllers/autoneg.go b/controllers/autoneg.go index d84afaa..f31d05e 100644 --- a/controllers/autoneg.go +++ b/controllers/autoneg.go @@ -507,16 +507,24 @@ func getStatuses(namespace string, name string, annotations map[string]string, r } s.newConfig = true + } - tmp, ok = annotations[autonegStatusAnnotation] - if ok { - // Found a status, decode - if err = json.Unmarshal([]byte(tmp), &s.status); err != nil { - return - } + // Does service has new auto neg status? + tmp, statusOk := annotations[autonegStatusAnnotation] + if statusOk { + // Status annotation found but auto neg annotation not found set empty auto neg config + if !newOk && r.DeregisterNEGsOnAnnotationRemoval { + s.config = AutonegConfig{} + valid = true + s.newConfig = true + } + // Found a status, decode + if err = json.Unmarshal([]byte(tmp), &s.status); err != nil { + return } } - if !newOk { + + if !newOk && !statusOk { // Is this service using autoneg in legacy mode? tmp, oldOk = annotations[oldAutonegAnnotation] if oldOk { @@ -554,13 +562,18 @@ func getStatuses(namespace string, name string, annotations map[string]string, r err = errors.New(fmt.Sprintf("more than one port in %s, but autoneg configuration is for one or no ports", negAnnotation)) return } + } - tmp, ok = annotations[oldAutonegStatusAnnotation] - if ok { - // Found a status, decode - if err = json.Unmarshal([]byte(tmp), &s.oldStatus); err != nil { - return - } + tmp, ok = annotations[oldAutonegStatusAnnotation] + if ok { + // Status annotation found but auto neg annotation not found set empty auto neg config + if !oldOk && r.DeregisterNEGsOnAnnotationRemoval { + s.oldConfig = OldAutonegConfig{} + valid = true + } + // Found a status, decode + if err = json.Unmarshal([]byte(tmp), &s.oldStatus); err != nil { + return } } } diff --git a/controllers/autoneg_test.go b/controllers/autoneg_test.go index 0a4b6cd..8a0b92b 100644 --- a/controllers/autoneg_test.go +++ b/controllers/autoneg_test.go @@ -166,6 +166,14 @@ var statusTests = []struct { true, true, }, + { + "valid autoneg status without autoneg", + map[string]string{ + autonegStatusAnnotation: validStatus, + }, + true, + false, + }, } var oldStatusTests = []struct { @@ -256,12 +264,21 @@ var oldStatusTests = []struct { true, false, }, + { + "(legacy) valid autoneg status without autoneg", + map[string]string{ + oldAutonegStatusAnnotation: validStatus, + }, + true, + false, + }, } func TestGetStatuses(t *testing.T) { var serviceReconciler = ServiceReconciler{ - ServiceNameTemplate: "{namespace}-{name}-{port}-{hash}", - AllowServiceName: true, + ServiceNameTemplate: "{namespace}-{name}-{port}-{hash}", + AllowServiceName: true, + DeregisterNEGsOnAnnotationRemoval: true, } for _, st := range statusTests { _, valid, err := getStatuses("ns", "test", st.annotations, &serviceReconciler) @@ -282,8 +299,9 @@ func TestGetStatuses(t *testing.T) { func TestGetOldStatuses(t *testing.T) { var serviceReconciler = ServiceReconciler{ - ServiceNameTemplate: "{namespace}-{name}-{port}-{hash}", - AllowServiceName: true, + ServiceNameTemplate: "{namespace}-{name}-{port}-{hash}", + AllowServiceName: true, + DeregisterNEGsOnAnnotationRemoval: true, } for _, st := range oldStatusTests { _, valid, err := getStatuses("ns", "test", st.annotations, &serviceReconciler) @@ -340,6 +358,52 @@ func TestGetStatusesServiceNameAllowed(t *testing.T) { } } +func TestGetStatusesOnlyAutonegStatusAnnotation(t *testing.T) { + var serviceReconciler = ServiceReconciler{ + ServiceNameTemplate: "{namespace}-{name}-{port}-{hash}", + AllowServiceName: true, + DeregisterNEGsOnAnnotationRemoval: true, + } + statuses, valid, err := getStatuses("ns", "test", map[string]string{autonegStatusAnnotation: validAutonegStatus}, &serviceReconciler) + if err != nil { + t.Errorf("Expected no error, got one: %v", err) + } + if !valid { + t.Errorf("Expected autoneg status config, got none") + } + + if !statuses.newConfig { + t.Errorf("Expected new autoneg config") + } + if statuses.config.BackendServices != nil { + t.Errorf("Expected nil backend services") + } + +} + +func TestGetStatusesOnlyOldAutonegStatusAnnotation(t *testing.T) { + var serviceReconciler = ServiceReconciler{ + ServiceNameTemplate: "{namespace}-{name}-{port}-{hash}", + AllowServiceName: true, + DeregisterNEGsOnAnnotationRemoval: true, + } + statuses, valid, err := getStatuses("ns", "test", map[string]string{oldAutonegStatusAnnotation: validAutonegStatus}, &serviceReconciler) + if err != nil { + t.Errorf("Expected no error, got one: %v", err) + } + if !valid { + t.Errorf("Expected old autoneg status config, got none") + } + + if statuses.newConfig { + t.Errorf("Expected old autoneg config") + } + if statuses.oldConfig.Name != "" { + t.Errorf("Expected empty old autoneg config") + } + +} + func TestDefaultMaxRatePerEndpointWhenOverrideIsSet(t *testing.T) { var serviceReconciler = ServiceReconciler{ ServiceNameTemplate: "{namespace}-{name}-{port}", diff --git a/controllers/service_controller.go b/controllers/service_controller.go index c760d56..e4b7f1e 100644 --- a/controllers/service_controller.go +++ b/controllers/service_controller.go @@ -42,13 +42,14 @@ type ServiceReconciler struct { client.Client Scheme *runtime.Scheme BackendController - Recorder record.EventRecorder - ServiceNameTemplate string - AllowServiceName bool - MaxRatePerEndpointDefault float64 - MaxConnectionsPerEndpointDefault float64 - AlwaysReconcile bool - ReconcileDuration *time.Duration + Recorder record.EventRecorder + ServiceNameTemplate string + AllowServiceName bool + MaxRatePerEndpointDefault float64 + MaxConnectionsPerEndpointDefault float64 + AlwaysReconcile bool + ReconcileDuration *time.Duration + DeregisterNEGsOnAnnotationRemoval bool } //+kubebuilder:rbac:groups=core,resources=services,verbs=get;list;watch;update;patch diff --git a/main.go b/main.go index 2c2cc83..5a0ba05 100644 --- a/main.go +++ b/main.go @@ -68,6 +68,7 @@ func main() { var alwaysReconcile bool var reconcilePeriod string var namespaces string + var deregisterNEGsOnAnnotationRemoval bool flag.StringVar(&metricsAddr, "metrics-bind-address", ":8080", "The address the metric endpoint binds to.") flag.StringVar(&probeAddr, "health-probe-bind-address", ":8081", "The address the probe endpoint binds to.") flag.Float64Var(&maxRatePerEndpointDefault, "max-rate-per-endpoint", 0, "Default max rate per endpoint. Can be overridden by user config.") @@ -82,6 +83,8 @@ func main() { flag.BoolVar(&allowServiceName, "enable-custom-service-names", true, "Enable using custom service names in autoneg annotation.") flag.BoolVar(&alwaysReconcile, "always-reconcile", false, "Periodically reconciles even if annotation statuses don't change.") flag.StringVar(&reconcilePeriod, "reconcile-period", "", "The minimum frequency at which watched resources are reconciled, e.g. 10m. Defaults to 10h if not set.") + flag.BoolVar(&deregisterNEGsOnAnnotationRemoval, "deregister-negs-on-annotation-removal", true, "Deregister NEGs from backend service when annotation removed.") + opts := zap.Options{ Development: true, } @@ -141,16 +144,17 @@ func main() { } if err = (&controllers.ServiceReconciler{ - Client: mgr.GetClient(), - Scheme: mgr.GetScheme(), - BackendController: controllers.NewBackendController(project, s), - Recorder: mgr.GetEventRecorderFor("autoneg-controller"), - ServiceNameTemplate: serviceNameTemplate, - AllowServiceName: allowServiceName, - MaxRatePerEndpointDefault: maxRatePerEndpointDefault, - MaxConnectionsPerEndpointDefault: maxConnectionsPerEndpointDefault, - AlwaysReconcile: alwaysReconcile, - ReconcileDuration: &reconcileDuration, + Client: mgr.GetClient(), + Scheme: mgr.GetScheme(), + BackendController: controllers.NewBackendController(project, s), + Recorder: mgr.GetEventRecorderFor("autoneg-controller"), + ServiceNameTemplate: serviceNameTemplate, + AllowServiceName: allowServiceName, + MaxRatePerEndpointDefault: maxRatePerEndpointDefault, + MaxConnectionsPerEndpointDefault: maxConnectionsPerEndpointDefault, + AlwaysReconcile: alwaysReconcile, + DeregisterNEGsOnAnnotationRemoval: deregisterNEGsOnAnnotationRemoval, + ReconcileDuration: &reconcileDuration, }).SetupWithManager(mgr); err != nil { setupLog.Error(err, "unable to create controller", "controller", "Service") os.Exit(1)