remove unnecessary logs, change endpointslices flag name, clean up code

Author: harivall

controllers/elbv2/eventhandlers/endpointslices.go

@@ -2,6 +2,7 @@ package eventhandlers
 
 import (
 	"context"
+	"errors"
 
 	"github.com/go-logr/logr"
 	discv1 "k8s.io/api/discovery/v1beta1"
@@ -35,7 +36,7 @@ type enqueueRequestsForEndpointSlicesEvent struct {
 // Create is called in response to an create event - e.g. EndpointSlice Creation.
 func (h *enqueueRequestsForEndpointSlicesEvent) Create(e event.CreateEvent, queue workqueue.RateLimitingInterface) {
 	epNew := e.Object.(*discv1.EndpointSlice)
-	h.logger.Info("Create event for EndpointSlices", "name", epNew.Name)
+	h.logger.V(1).Info("Create event for EndpointSlices", "name", epNew.Name)
 	h.enqueueImpactedTargetGroupBindings(queue, epNew)
 }
 
@@ -45,15 +46,15 @@ func (h *enqueueRequestsForEndpointSlicesEvent) Update(e event.UpdateEvent, queu
 	epNew := e.ObjectNew.(*discv1.EndpointSlice)
 	h.logger.Info("Update event for EndpointSlices", "name", epNew.Name)
 	if !equality.Semantic.DeepEqual(epOld.Ports, epNew.Ports) || !equality.Semantic.DeepEqual(epOld.Endpoints, epNew.Endpoints) {
-		h.logger.Info("Enqueue EndpointSlice", "name", epNew.Name)
+		h.logger.V(1).Info("Enqueue EndpointSlice", "name", epNew.Name)
 		h.enqueueImpactedTargetGroupBindings(queue, epNew)
 	}
 }
 
 // Delete is called in response to a delete event - e.g. EndpointSlice Deleted.
 func (h *enqueueRequestsForEndpointSlicesEvent) Delete(e event.DeleteEvent, queue workqueue.RateLimitingInterface) {
 	epOld := e.Object.(*discv1.EndpointSlice)
-	h.logger.Info("Deletion event for EndpointSlices", "name", epOld.Name)
+	h.logger.V(1).Info("Deletion event for EndpointSlices", "name", epOld.Name)
 	h.enqueueImpactedTargetGroupBindings(queue, epOld)
 }
 
@@ -62,24 +63,30 @@ func (h *enqueueRequestsForEndpointSlicesEvent) Delete(e event.DeleteEvent, queu
 func (h *enqueueRequestsForEndpointSlicesEvent) Generic(event.GenericEvent, workqueue.RateLimitingInterface) {
 }
 
-func (h *enqueueRequestsForEndpointSlicesEvent) enqueueImpactedTargetGroupBindings(queue workqueue.RateLimitingInterface, epslice *discv1.EndpointSlice) {
+func (h *enqueueRequestsForEndpointSlicesEvent) enqueueImpactedTargetGroupBindings(queue workqueue.RateLimitingInterface, epSlice *discv1.EndpointSlice) {
 	tgbList := &elbv2api.TargetGroupBindingList{}
-	svcName := epslice.Labels["kubernetes.io/service-name"]
+	const svcNameLabel = "kubernetes.io/service-name"
+	svcName, present := epSlice.Labels[svcNameLabel]
+	if !present {
+		err := errors.New("EndpointSlice does not have a" + svcNameLabel + "label")
+		h.logger.Error(err, "unable to find service name for endpointslice")
+		return
+	}
 	if err := h.k8sClient.List(context.Background(), tgbList,
-		client.InNamespace(epslice.Namespace),
+		client.InNamespace(epSlice.Namespace),
 		client.MatchingFields{targetgroupbinding.IndexKeyServiceRefName: svcName}); err != nil {
 		h.logger.Error(err, "failed to fetch targetGroupBindings")
 		return
 	}
 
-	epsliceKey := k8s.NamespacedName(epslice)
+	epSliceKey := k8s.NamespacedName(epSlice)
 	for _, tgb := range tgbList.Items {
 		if tgb.Spec.TargetType == nil || (*tgb.Spec.TargetType) != elbv2api.TargetTypeIP {
 			continue
 		}
 
 		h.logger.V(1).Info("enqueue targetGroupBinding for endpointslices event",
-			"endpointslices", epsliceKey,
+			"endpointslices", epSliceKey,
 			"targetGroupBinding", k8s.NamespacedName(&tgb),
 		)
 		queue.Add(reconcile.Request{

controllers/elbv2/targetgroupbinding_controller.go

@@ -61,7 +61,7 @@ func NewTargetGroupBindingReconciler(k8sClient client.Client, eventRecorder reco
 
 		maxConcurrentReconciles:    config.TargetGroupBindingMaxConcurrentReconciles,
 		maxExponentialBackoffDelay: config.TargetGroupBindingMaxExponentialBackoffDelay,
-		useEndpointSlices:          config.UseEndpointSlices,
+		enableEndpointSlices:       config.EnableEndpointSlices,
 	}
 }
 
@@ -75,7 +75,7 @@ type targetGroupBindingReconciler struct {
 
 	maxConcurrentReconciles    int
 	maxExponentialBackoffDelay time.Duration
-	useEndpointSlices          bool
+	enableEndpointSlices       bool
 }
 
 // +kubebuilder:rbac:groups=elbv2.k8s.aws,resources=targetgroupbindings,verbs=get;list;watch;update;patch;create;delete
@@ -87,15 +87,16 @@ type targetGroupBindingReconciler struct {
 // +kubebuilder:rbac:groups="",resources=secrets,verbs=get;list;watch
 // +kubebuilder:rbac:groups="",resources=namespaces,verbs=get;list;watch
 // +kubebuilder:rbac:groups="",resources=events,verbs=create;patch
+// +kubebuilder:rbac:groups="discovery.k8s.io",resources=endpointslices,verbs=get;list;watch
 
 func (r *targetGroupBindingReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
-	r.logger.Info("Reconcile request", "name", req.Name)
+	r.logger.V(1).Info("Reconcile request", "name", req.Name)
 	return runtime.HandleReconcileError(r.reconcile(ctx, req), r.logger)
 }
 
 func (r *targetGroupBindingReconciler) reconcile(ctx context.Context, req ctrl.Request) error {
 	tgb := &elbv2api.TargetGroupBinding{}
-	r.logger.Info("reconcile request Get", "name", req.Name, "tgb name", tgb.ObjectMeta.Name)
+	r.logger.V(1).Info("reconcile request Get", "name", req.Name, "tgb name", tgb.ObjectMeta.Name)
 	if err := r.k8sClient.Get(ctx, req.NamespacedName, tgb); err != nil {
 		return client.IgnoreNotFound(err)
 	}
@@ -107,16 +108,15 @@ func (r *targetGroupBindingReconciler) reconcile(ctx context.Context, req ctrl.R
 }
 
 func (r *targetGroupBindingReconciler) reconcileTargetGroupBinding(ctx context.Context, tgb *elbv2api.TargetGroupBinding) error {
-	r.logger.Info("reconcileTargetGroupBinding AddFinalizers", "name", tgb.ObjectMeta.Name)
 	if err := r.finalizerManager.AddFinalizers(ctx, tgb, targetGroupBindingFinalizer); err != nil {
 		r.eventRecorder.Event(tgb, corev1.EventTypeWarning, k8s.TargetGroupBindingEventReasonFailedAddFinalizer, fmt.Sprintf("Failed add finalizer due to %v", err))
 		return err
 	}
-	r.logger.Info("reconcileTargetGroupBinding Reconcile", "name", tgb.ObjectMeta.Name)
+
 	if err := r.tgbResourceManager.Reconcile(ctx, tgb); err != nil {
 		return err
 	}
-	r.logger.Info("reconcileTargetGroupBinding updateTargetGroupBindingStatus", "name", tgb.ObjectMeta.Name)
+
 	if err := r.updateTargetGroupBindingStatus(ctx, tgb); err != nil {
 		r.eventRecorder.Event(tgb, corev1.EventTypeWarning, k8s.TargetGroupBindingEventReasonFailedUpdateStatus, fmt.Sprintf("Failed update status due to %v", err))
 		return err
@@ -163,7 +163,7 @@ func (r *targetGroupBindingReconciler) SetupWithManager(ctx context.Context, mgr
 		r.logger.WithName("eventHandlers").WithName("node"))
 
 	// Use the config flag to decide whether to use and watch an Endpoints event handler or an EndpointSlices event handler
-	if r.useEndpointSlices {
+	if r.enableEndpointSlices {
 		epSliceEventsHandler := eventhandlers.NewEnqueueRequestsForEndpointSlicesEvent(r.k8sClient,
 			r.logger.WithName("eventHandlers").WithName("endpointslices"))
 		return ctrl.NewControllerManagedBy(mgr).

docs/deploy/configurations.md

@@ -93,7 +93,7 @@ Currently, you can set only 1 namespace to watch in this flag. See [this Kuberne
 |sync-period                            | duration                        | 1h0m0s          | Period at which the controller forces the repopulation of its local object stores|
 |targetgroupbinding-max-concurrent-reconciles | int                       | 3               | Maximum number of concurrently running reconcile loops for targetGroupBinding |
 |targetgroupbinding-max-exponential-backoff-delay | duration              | 16m40s          | Maximum duration of exponential backoff for targetGroupBinding reconcile failures |
-|use-endpoint-slices                    | boolean                         | false           | Use EndpointSlices instead of Endpoints for pod endpoint and TargetGroupBinding resolution. |
+|enable-endpoint-slices                 | boolean                         | false           | Use EndpointSlices instead of Endpoints for pod endpoint and TargetGroupBinding resolution for load balancers with IP targets. |
 |watch-namespace                        | string                          |                 | Namespace the controller watches for updates to Kubernetes objects, If empty, all namespaces are watched. |
 |webhook-bind-port                      | int                             | 9443            | The TCP port the Webhook server binds to |
 |webhook-cert-dir                       | string                          | /tmp/k8s-webhook-server/serving-certs | The directory that contains the server key and certificate |

main.go

@@ -106,7 +106,7 @@ func main() {
 	subnetResolver := networking.NewDefaultSubnetsResolver(azInfoProvider, cloud.EC2(), cloud.VpcID(), controllerCFG.ClusterName, ctrl.Log.WithName("subnets-resolver"))
 	vpcResolver := networking.NewDefaultVPCResolver(cloud.EC2(), cloud.VpcID(), ctrl.Log.WithName("vpc-resolver"))
 	tgbResManager := targetgroupbinding.NewDefaultResourceManager(mgr.GetClient(), cloud.ELBV2(), cloud.EC2(),
-		podInfoRepo, sgManager, sgReconciler, cloud.VpcID(), controllerCFG.ClusterName, mgr.GetEventRecorderFor("targetGroupBinding"), ctrl.Log, controllerCFG.UseEndpointSlices)
+		podInfoRepo, sgManager, sgReconciler, cloud.VpcID(), controllerCFG.ClusterName, mgr.GetEventRecorderFor("targetGroupBinding"), ctrl.Log, controllerCFG.EnableEndpointSlices)
 	backendSGProvider := networking.NewBackendSGProvider(controllerCFG.ClusterName, controllerCFG.BackendSecurityGroup,
 		cloud.VpcID(), cloud.EC2(), mgr.GetClient(), ctrl.Log.WithName("backend-sg-provider"))
 	ingGroupReconciler := ingress.NewGroupReconciler(cloud, mgr.GetClient(), mgr.GetEventRecorderFor("ingress"),

pkg/backend/endpoint_resolver.go

@@ -139,10 +139,11 @@ func (r *defaultEndpointResolver) ResolvePodEndpointsFromSlices(ctx context.Cont
 		return nil, false, err
 	}
 
+	const svcNameLabel = "kubernetes.io/service-name"
 	epSlicesList := &discv1.EndpointSliceList{}
 	if err := r.k8sClient.List(ctx, epSlicesList,
 		client.InNamespace(svcKey.Namespace),
-		client.MatchingLabels{"kubernetes.io/service-name": svcKey.Name}); err != nil {
+		client.MatchingLabels{svcNameLabel: svcKey.Name}); err != nil {
 		return nil, false, err
 	}
 	if len(epSlicesList.Items) == 0 {
@@ -151,7 +152,7 @@ func (r *defaultEndpointResolver) ResolvePodEndpointsFromSlices(ctx context.Cont
 
 	containsPotentialReadyEndpoints := false
 	var endpoints []PodEndpoint
-	used_addr := make(sets.String)
+	usedAddrs := make(sets.String)
 	for _, epSlice := range epSlicesList.Items {
 		// process epSlice
 		for _, epPort := range epSlice.Ports {
@@ -160,33 +161,25 @@ func (r *defaultEndpointResolver) ResolvePodEndpointsFromSlices(ctx context.Cont
 				continue
 			}
 			for _, ep := range epSlice.Endpoints {
-				if ep.Conditions.Ready == nil || *ep.Conditions.Ready {
-					for _, epAddr := range ep.Addresses {
-						if used_addr.Has(epAddr) {
-							continue
-						}
-						if ep.TargetRef == nil || ep.TargetRef.Kind != "Pod" {
-							continue
-						}
+				for _, epAddr := range ep.Addresses {
+					if usedAddrs.Has(epAddr) {
+						continue
+					}
+					if ep.TargetRef == nil || ep.TargetRef.Kind != "Pod" {
+						continue
+					}
+					if ep.Conditions.Ready == nil || *ep.Conditions.Ready {
 						pod, exists, err := r.findPodByReference(ctx, svc.Namespace, *ep.TargetRef)
 						if err != nil {
 							return nil, false, err
 						}
 						if !exists {
 							return nil, false, errors.New("couldn't find podInfo for ready endpoint")
 						}
-						used_addr.Insert(epAddr)
+						usedAddrs.Insert(epAddr)
 						endpoints = append(endpoints, buildPodEndpointFromSlice(pod, epAddr, epPort))
 					}
-				}
-				if len(resolveOpts.PodReadinessGates) != 0 {
-					for _, epAddr := range ep.Addresses {
-						if used_addr.Has(epAddr) {
-							continue
-						}
-						if ep.TargetRef == nil || ep.TargetRef.Kind != "Pod" {
-							continue
-						}
+					if len(resolveOpts.PodReadinessGates) != 0 {
 						pod, exists, err := r.findPodByReference(ctx, svc.Namespace, *ep.TargetRef)
 						if err != nil {
 							return nil, false, err
@@ -202,7 +195,7 @@ func (r *defaultEndpointResolver) ResolvePodEndpointsFromSlices(ctx context.Cont
 							containsPotentialReadyEndpoints = true
 							continue
 						}
-						used_addr.Insert(epAddr)
+						usedAddrs.Insert(epAddr)
 						endpoints = append(endpoints, buildPodEndpointFromSlice(pod, epAddr, epPort))
 					}
 				}

pkg/config/controller_config.go

@@ -20,15 +20,15 @@ const (
 	flagTargetGroupBindingMaxConcurrentReconciles    = "targetgroupbinding-max-concurrent-reconciles"
 	flagTargetGroupBindingMaxExponentialBackoffDelay = "targetgroupbinding-max-exponential-backoff-delay"
 	flagDefaultSSLPolicy                             = "default-ssl-policy"
-	flagUseEndpointSlices                            = "use-endpoint-slices"
 	flagEnableBackendSG                              = "enable-backend-security-group"
 	flagBackendSecurityGroup                         = "backend-security-group"
+	flagEnableEndpointSlices                         = "enable-endpoint-slices"
 	defaultLogLevel                                  = "info"
 	defaultMaxConcurrentReconciles                   = 3
 	defaultMaxExponentialBackoffDelay                = time.Second * 1000
 	defaultSSLPolicy                                 = "ELBSecurityPolicy-2016-08"
-	defaultUseEndpointSlices                         = false
 	defaultEnableBackendSG                           = true
+	defaultEnableEndpointSlices                      = false
 )
 
 var (
@@ -68,8 +68,8 @@ type ControllerConfig struct {
 	// the SSL Policy annotation.
 	DefaultSSLPolicy string
 
-	// Whether to use EndpointSlices resources(true) or Endpoints resources(false) in endpoint and TGB resolution.
-	UseEndpointSlices bool
+	// Whether to use endpointslices resources(true) or Endpoints resources(false) in endpoint and TGB resolution.
+	EnableEndpointSlices bool
 
 	// Max concurrent reconcile loops for Service objects
 	ServiceMaxConcurrentReconciles int
@@ -107,8 +107,7 @@ func (cfg *ControllerConfig) BindFlags(fs *pflag.FlagSet) {
 		"Enable sharing of security groups for backend traffic")
 	fs.StringVar(&cfg.BackendSecurityGroup, flagBackendSecurityGroup, "",
 		"Backend security group id to use for the ingress rules on the worker node SG")
-
-	fs.BoolVar(&cfg.UseEndpointSlices, flagUseEndpointSlices, defaultUseEndpointSlices,
+	fs.BoolVar(&cfg.EnableEndpointSlices, flagEnableEndpointSlices, defaultEnableEndpointSlices,
 		"Use EndpointSlices(true) or Endpoints(false) resources in endpoint and TGB resolution")
 	cfg.AWSConfig.BindFlags(fs)
 	cfg.RuntimeConfig.BindFlags(fs)

pkg/targetgroupbinding/resource_manager.go

@@ -39,7 +39,7 @@ type ResourceManager interface {
 // NewDefaultResourceManager constructs new defaultResourceManager.
 func NewDefaultResourceManager(k8sClient client.Client, elbv2Client services.ELBV2, ec2Client services.EC2,
 	podInfoRepo k8s.PodInfoRepo, sgManager networking.SecurityGroupManager, sgReconciler networking.SecurityGroupReconciler,
-	vpcID string, clusterName string, eventRecorder record.EventRecorder, logger logr.Logger, useEPSlices bool) *defaultResourceManager {
+	vpcID string, clusterName string, eventRecorder record.EventRecorder, logger logr.Logger, useEndpointSlices bool) *defaultResourceManager {
 	targetsManager := NewCachedTargetsManager(elbv2Client, logger)
 	endpointResolver := backend.NewDefaultEndpointResolver(k8sClient, podInfoRepo, logger)
 
@@ -57,7 +57,7 @@ func NewDefaultResourceManager(k8sClient client.Client, elbv2Client services.ELB
 		logger:            logger,
 
 		targetHealthRequeueDuration: defaultTargetHealthRequeueDuration,
-		useEndpointSlices:           useEPSlices,
+		enableEndpointSlices:        useEndpointSlices,
 	}
 }
 
@@ -73,7 +73,7 @@ type defaultResourceManager struct {
 	logger            logr.Logger
 
 	targetHealthRequeueDuration time.Duration
-	useEndpointSlices           bool
+	enableEndpointSlices        bool
 }
 
 func (m *defaultResourceManager) Reconcile(ctx context.Context, tgb *elbv2api.TargetGroupBinding) error {
@@ -108,7 +108,7 @@ func (m *defaultResourceManager) reconcileWithIPTargetType(ctx context.Context,
 	var err error
 
 	// Decide whether to use Endpoints or EndpointSlices based on config flag
-	if m.useEndpointSlices {
+	if m.enableEndpointSlices {
 		endpoints, containsPotentialReadyEndpoints, err = m.endpointResolver.ResolvePodEndpointsFromSlices(ctx, svcKey, tgb.Spec.ServiceRef.Port, resolveOpts...)
 	} else {
 		endpoints, containsPotentialReadyEndpoints, err = m.endpointResolver.ResolvePodEndpoints(ctx, svcKey, tgb.Spec.ServiceRef.Port, resolveOpts...)