fix: deregister NEGs on removing annotation

Signed-off-by: Akshay Patidar <akshaypatidar1999@gmail.com>
GoogleCloudPlatform · Oct 19, 2024 · a168284 · a168284
1 parent 5e6da75
commit a168284
Show file tree

Hide file tree

Showing 3 changed files with 48 additions and 30 deletions.
diff --git 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/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
@@ -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)