Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

first pass making gateway controllers less chatty #2524

Merged
merged 4 commits into from
Jul 11, 2023
Merged

first pass making gateway controllers less chatty #2524

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8b1f90b
first pass making gateway controllers less chatty
missylbytes Jul 10, 2023
729dbd5
lint
missylbytes Jul 10, 2023
2ebd06f
cleanup and address PR comments
missylbytes Jul 10, 2023
73afdc0
adds changelog
missylbytes Jul 11, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 23 additions & 2 deletions control-plane/api-gateway/controllers/gateway_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ func (r *GatewayController) Reconcile(ctx context.Context, req ctrl.Request) (ct

var gateway gwv1beta1.Gateway

log := r.Log.WithValues("gateway", req.NamespacedName)
log := r.Log.V(1).WithValues("gateway", req.NamespacedName)
log.Info("Reconciling Gateway")

// get the gateway
Expand Down Expand Up @@ -199,18 +199,28 @@ func (r *GatewayController) Reconcile(ctx context.Context, req ctrl.Request) (ct

err := r.updateGatekeeperResources(ctx, log, &gateway, updates.GatewayClassConfig)
if err != nil {
if k8serrors.IsConflict(err) {
log.Info("error updating object when updating gateway resources, will try to re-reconcile")

return ctrl.Result{Requeue: true}, nil
}
log.Error(err, "unable to update gateway resources")
return ctrl.Result{}, err
}
r.gatewayCache.EnsureSubscribed(nonNormalizedConsulKey, req.NamespacedName)
} else {
err := r.deleteGatekeeperResources(ctx, log, &gateway)
if err != nil {
if k8serrors.IsConflict(err) {
log.Info("error updating object when deleting gateway resources, will try to re-reconcile")

return ctrl.Result{Requeue: true}, nil
}
log.Error(err, "unable to delete gateway resources")
return ctrl.Result{}, err
}
r.gatewayCache.RemoveSubscription(nonNormalizedConsulKey)
// make sure we have deregister all services even if they haven't
// make sure we have deregistered all services even if they haven't
// hit cache yet
if err := r.deregisterAllServices(ctx, nonNormalizedConsulKey); err != nil {
log.Error(err, "error deregistering services")
Expand Down Expand Up @@ -266,6 +276,11 @@ func (r *GatewayController) Reconcile(ctx context.Context, req ctrl.Request) (ct
for _, update := range updates.Kubernetes.Updates.Operations() {
log.Info("update in Kubernetes", "kind", update.GetObjectKind().GroupVersionKind().Kind, "namespace", update.GetNamespace(), "name", update.GetName())
if err := r.updateAndResetStatus(ctx, update); err != nil {
if k8serrors.IsConflict(err) {
log.Info("error updating object for gateway, will try to re-reconcile")

return ctrl.Result{Requeue: true}, nil
}
log.Error(err, "error updating object")
return ctrl.Result{}, err
}
Expand All @@ -274,6 +289,11 @@ func (r *GatewayController) Reconcile(ctx context.Context, req ctrl.Request) (ct
for _, update := range updates.Kubernetes.StatusUpdates.Operations() {
log.Info("update status in Kubernetes", "kind", update.GetObjectKind().GroupVersionKind().Kind, "namespace", update.GetNamespace(), "name", update.GetName())
if err := r.Client.Status().Update(ctx, update); err != nil {
if k8serrors.IsConflict(err) {
log.Info("error updating status for gateway, will try to re-reconcile")

return ctrl.Result{Requeue: true}, nil
}
log.Error(err, "error updating status")
return ctrl.Result{}, err
}
Expand Down Expand Up @@ -305,6 +325,7 @@ func (r *GatewayController) updateAndResetStatus(ctx context.Context, o client.O
if err := r.Client.Update(ctx, o); err != nil {
return err
}

// reset the status in case it needs to be updated below
reflect.ValueOf(o).Elem().FieldByName("Status").Set(status)
return nil
Expand Down
20 changes: 17 additions & 3 deletions control-plane/api-gateway/controllers/gatewayclass_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ package controllers
import (
"context"
"fmt"

"github.com/go-logr/logr"
"github.com/hashicorp/consul-k8s/control-plane/api/v1alpha1"
k8serrors "k8s.io/apimachinery/pkg/api/errors"
Expand Down Expand Up @@ -42,7 +41,7 @@ type GatewayClassController struct {
// Reconcile handles the reconciliation loop for GatewayClass objects.
func (r *GatewayClassController) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
log := r.Log.WithValues("gatewayClass", req.NamespacedName.Name)
log.Info("Reconciling GatewayClass")
log.V(1).Info("Reconciling GatewayClass")

gc := &gwv1beta1.GatewayClass{}

Expand Down Expand Up @@ -78,6 +77,11 @@ func (r *GatewayClassController) Reconcile(ctx context.Context, req ctrl.Request
}
// Remove our finalizer.
if _, err := RemoveFinalizer(ctx, r.Client, gc, gatewayClassFinalizer); err != nil {
if k8serrors.IsConflict(err) {
log.V(1).Info("error removing finalizer for gatewayClass, will try to re-reconcile")

return ctrl.Result{Requeue: true}, nil
}
log.Error(err, "unable to remove finalizer")
return ctrl.Result{}, err
}
Expand All @@ -87,6 +91,11 @@ func (r *GatewayClassController) Reconcile(ctx context.Context, req ctrl.Request
// We are creating or updating the GatewayClass.
didUpdate, err := EnsureFinalizer(ctx, r.Client, gc, gatewayClassFinalizer)
if err != nil {
if k8serrors.IsConflict(err) {
log.V(1).Info("error adding finalizer for gatewayClass, will try to re-reconcile")

return ctrl.Result{Requeue: true}, nil
}
log.Error(err, "unable to add finalizer")
return ctrl.Result{}, err
}
Expand All @@ -98,7 +107,12 @@ func (r *GatewayClassController) Reconcile(ctx context.Context, req ctrl.Request
didUpdate, err = r.validateParametersRef(ctx, gc, log)
if didUpdate {
if err := r.Client.Status().Update(ctx, gc); err != nil {
log.Error(err, "unable to update GatewayClass")
if k8serrors.IsConflict(err) {
log.V(1).Info("error updating status for gatewayClass, will try to re-reconcile")

return ctrl.Result{Requeue: true}, nil
}
log.Error(err, "unable to update status for GatewayClass")
return ctrl.Result{}, err
}
return ctrl.Result{}, nil
Expand Down
21 changes: 15 additions & 6 deletions control-plane/api-gateway/controllers/gatewayclassconfig_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,39 +37,48 @@ type GatewayClassConfigController struct {
// - https://pkg.go.dev/sigs.k8s.io/controller-runtime@v0.8.3/pkg/reconcile
func (r *GatewayClassConfigController) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
log := r.Log.WithValues("gatewayClassConfig", req.NamespacedName.Name)
log.Info("Reconciling GatewayClassConfig ")
log.V(1).Info("Reconciling GatewayClassConfig ")

gcc := &v1alpha1.GatewayClassConfig{}
if err := r.Client.Get(ctx, req.NamespacedName, gcc); err != nil {
if k8serrors.IsNotFound(err) {
return ctrl.Result{}, nil
}
r.Log.Error(err, "failed to get gateway class config")
log.Error(err, "failed to get gateway class config")
return ctrl.Result{}, err
}

if !gcc.ObjectMeta.DeletionTimestamp.IsZero() {
// We have a deletion, ensure we're not in use.
used, err := gatewayClassConfigInUse(ctx, r.Client, gcc)
if err != nil {
r.Log.Error(err, "failed to check if the gateway class config is still in use")
log.Error(err, "failed to check if the gateway class config is still in use")
return ctrl.Result{}, err
}
if used {
r.Log.Info("gateway class config still in use")
log.Info("gateway class config still in use")
// Requeue as to not block the reconciliation loop.
return ctrl.Result{RequeueAfter: 10 * time.Second}, nil
}
// gcc is no longer in use.
if _, err := RemoveFinalizer(ctx, r.Client, gcc, gatewayClassConfigFinalizer); err != nil {
r.Log.Error(err, "error removing gateway class config finalizer")
if k8serrors.IsConflict(err) {
log.V(1).Info("error removing gateway class config finalizer, will try to re-reconcile")
return ctrl.Result{Requeue: true}, nil
}
log.Error(err, "error removing gateway class config finalizer")
return ctrl.Result{}, err
}
return ctrl.Result{}, nil
}

if _, err := EnsureFinalizer(ctx, r.Client, gcc, gatewayClassConfigFinalizer); err != nil {
r.Log.Error(err, "error adding gateway class config finalizer")
if k8serrors.IsConflict(err) {
log.V(1).Info("error adding gateway class config finalizer, will try to re-reconcile")

return ctrl.Result{Requeue: true}, nil
}
log.Error(err, "error adding gateway class config finalizer")
return ctrl.Result{}, err
}

Expand Down
8 changes: 4 additions & 4 deletions control-plane/api-gateway/gatekeeper/deployment.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ func (g *Gatekeeper) upsertDeployment(ctx context.Context, gateway gwv1beta1.Gat
}

if exists {
g.Log.Info("Existing Gateway Deployment found.")
g.Log.V(1).Info("Existing Gateway Deployment found.")

// If the user has set the number of replicas, let's respect that.
deployment.Spec.Replicas = existingDeployment.Spec.Replicas
Expand All @@ -65,11 +65,11 @@ func (g *Gatekeeper) upsertDeployment(ctx context.Context, gateway gwv1beta1.Gat

switch result {
case controllerutil.OperationResultCreated:
g.Log.Info("Created Deployment")
g.Log.V(1).Info("Created Deployment")
case controllerutil.OperationResultUpdated:
g.Log.Info("Updated Deployment")
g.Log.V(1).Info("Updated Deployment")
case controllerutil.OperationResultNone:
g.Log.Info("No change to deployment")
g.Log.V(1).Info("No change to deployment")
}

return nil
Expand Down
4 changes: 2 additions & 2 deletions control-plane/api-gateway/gatekeeper/gatekeeper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ func New(log logr.Logger, client client.Client) *Gatekeeper {
// Upsert creates or updates the resources for handling routing of network traffic.
// This is done in order based on dependencies between resources.
func (g *Gatekeeper) Upsert(ctx context.Context, gateway gwv1beta1.Gateway, gcc v1alpha1.GatewayClassConfig, config common.HelmConfig) error {
g.Log.Info(fmt.Sprintf("Upsert Gateway Deployment %s/%s", gateway.Namespace, gateway.Name))
g.Log.V(1).Info(fmt.Sprintf("Upsert Gateway Deployment %s/%s", gateway.Namespace, gateway.Name))

if err := g.upsertRole(ctx, gateway, gcc, config); err != nil {
return err
Expand Down Expand Up @@ -60,7 +60,7 @@ func (g *Gatekeeper) Upsert(ctx context.Context, gateway gwv1beta1.Gateway, gcc
// Delete removes the resources for handling routing of network traffic.
// This is done in the reverse order of Upsert due to dependencies between resources.
func (g *Gatekeeper) Delete(ctx context.Context, gatewayName types.NamespacedName) error {
g.Log.Info(fmt.Sprintf("Delete Gateway Deployment %s/%s", gatewayName.Namespace, gatewayName.Name))
g.Log.V(1).Info(fmt.Sprintf("Delete Gateway Deployment %s/%s", gatewayName.Namespace, gatewayName.Name))

if err := g.deleteDeployment(ctx, gatewayName); err != nil {
return err
Expand Down
6 changes: 3 additions & 3 deletions control-plane/api-gateway/gatekeeper/service.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,11 @@ func (g *Gatekeeper) upsertService(ctx context.Context, gateway gwv1beta1.Gatewa

switch result {
case controllerutil.OperationResultCreated:
g.Log.Info("Created Service")
g.Log.V(1).Info("Created Service")
case controllerutil.OperationResultUpdated:
g.Log.Info("Updated Service")
g.Log.V(1).Info("Updated Service")
case controllerutil.OperationResultNone:
g.Log.Info("No change to service")
g.Log.V(1).Info("No change to service")
}

return nil
Expand Down