Skip to content

Commit

Permalink
Record event if GSLB references cannot be resolved
Browse files Browse the repository at this point in the history
In the [PR](#1672 (review)) where the istio ingress integration was introduced, it was suggested to record events if GSLB references cannot be resolved. The controller was already generating logs, but an event would help the users debugging using `kubectl describe`.
This is a new feature that was not yet present in K8GB. Therefore, to implement it we need to fetch an EventRecorder and grant the controller the appropriate permissions. The error messages were also adapted and currently look like:
```
[k3d-test-gslb2|test-gslb] ➜ k describe gslb roundrobin-ingress-broken
...
Events:
  Type     Reason          Age                From             Message
  ----     ------          ----               ----             -------
  Warning  ReconcileError  2s (x12 over 12s)  gslb-controller  error resolving references (APIVersion:networking.k8s.io/v1, Kind:Degress not supported)
```
```
[k3d-test-gslb2|test-gslb] ➜ k get events
LAST SEEN   TYPE      REASON           OBJECT                           MESSAGE
5s          Warning   ReconcileError   gslb/roundrobin-ingress-broken   error resolving references (APIVersion:networking.k8s.io/v1, Kind:Degress not supported)
```

The controller also logs a message (this was already the case before this PR):
```
2024-10-31T16:47:29Z ERR github.com/k8gb-io/k8gb/controllers/logging/logr.go:72 > events: Reconciler error {"Gslb":"test-gslb/roundrobin-ingress-broken","controller":"gslb","controllerGroup":"k8gb.absa.oss","controllerKind":"Gslb","name":"roundrobin-ingress-broken","namespace":"test-gslb","reconcileID":"2a6a4ad5-96f6-4ee6-8c04-28de0016fc57"} error="error resolving references (APIVersion:networking.k8s.io/v1, Kind:Degress not supported)"
```

There are many other situations where we can generate events, but they won't be included in this PR to avoid bloating it.

[Documentation about events using kubebuilder](https://book-v1.book.kubebuilder.io/beyond_basics/creating_events)

---

This PR also upgrades the golinter since the old version was not compatible with my local go version. Upon upgrading I also resolved the following warning:
```
WARN The linter 'exportloopref' is deprecated (since v1.60.2) due to: Since Go1.22 (loopvar) this linter is no longer relevant. Replaced by copyloopvar.
```

Signed-off-by: Andre Aguas <[email protected]>
  • Loading branch information
abaguas committed Oct 31, 2024
1 parent 8e22d67 commit b5ca251
Show file tree
Hide file tree
Showing 8 changed files with 28 additions and 11 deletions.
2 changes: 1 addition & 1 deletion .golangci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ linters:
- unused
- depguard
- dogsled
- exportloopref
- copyloopvar
- goconst
- goprintffuncname
- nolintlint
Expand Down
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ LOG_FORMAT ?= simple
LOG_LEVEL ?= debug
CONTROLLER_GEN_VERSION ?= v0.15.0
GOLIC_VERSION ?= v0.7.2
GOLANGCI_VERSION ?= v1.59.1
GOLANGCI_VERSION ?= v1.60.3
POD_NAMESPACE ?= k8gb
CLUSTER_GEO_TAG ?= eu
EXT_GSLB_CLUSTERS_GEO_TAGS ?= us
Expand Down
7 changes: 7 additions & 0 deletions chart/k8gb/templates/role.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,13 @@ metadata:
labels:
{{ include "chart.labels" . | indent 4 }}
rules:
- apiGroups:
- ""
resources:
- events
verbs:
- create
- patch
- apiGroups:
- ""
resources:
Expand Down
14 changes: 11 additions & 3 deletions controllers/gslb_controller_reconciliation.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,14 +28,18 @@ import (
"github.com/k8gb-io/k8gb/controllers/providers/metrics"
"github.com/k8gb-io/k8gb/controllers/refresolver"

"errors"

k8gbv1beta1 "github.com/k8gb-io/k8gb/api/v1beta1"
"github.com/k8gb-io/k8gb/controllers/depresolver"
"github.com/k8gb-io/k8gb/controllers/logging"
"github.com/k8gb-io/k8gb/controllers/providers/dns"
"go.opentelemetry.io/otel/codes"
"go.opentelemetry.io/otel/trace"
"k8s.io/apimachinery/pkg/api/errors"
corev1 "k8s.io/api/core/v1"
k8serrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/client-go/tools/record"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
)
Expand All @@ -47,6 +51,7 @@ type GslbReconciler struct {
Config *depresolver.Config
DepResolver depresolver.GslbResolver
DNSProvider dns.Provider
Recorder record.EventRecorder
Tracer trace.Tracer
}

Expand All @@ -66,6 +71,7 @@ var m = metrics.Metrics()

// +kubebuilder:rbac:groups=k8gb.absa.oss,resources=gslbs,verbs=get;list;watch;create;update;patch;delete
// +kubebuilder:rbac:groups=k8gb.absa.oss,resources=gslbs/status,verbs=get;update;patch
// +kubebuilder:rbac:groups="",resources=events,verbs=create;patch

// Reconcile runs main reconciliation loop
func (r *GslbReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
Expand All @@ -77,7 +83,7 @@ func (r *GslbReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.
gslb := &k8gbv1beta1.Gslb{}
err := r.Get(ctx, req.NamespacedName, gslb)
if err != nil {
if errors.IsNotFound(err) {
if k8serrors.IsNotFound(err) {
// Request object not found, could have been deleted after reconcile request.
// Owned objects are automatically garbage collected. For additional cleanup logic use finalizers.
// Return and don't requeue
Expand Down Expand Up @@ -161,7 +167,9 @@ func (r *GslbReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.
refResolver, err := refresolver.New(gslb, r.Client)
if err != nil {
m.IncrementError(gslb)
return result.RequeueError(fmt.Errorf("error resolving references to the refresolver object (%s)", err))
errorMsg := fmt.Sprintf("error resolving references (%s)", err)
r.Recorder.Event(gslb, corev1.EventTypeWarning, "ReconcileError", errorMsg)
return result.RequeueError(errors.New(errorMsg))
}
servers, err := refResolver.GetServers()
if err != nil {
Expand Down
2 changes: 2 additions & 0 deletions controllers/gslb_controller_setup.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@ import (
func (r *GslbReconciler) SetupWithManager(mgr ctrl.Manager) error {
// Figure out Gslb resource name to Reconcile when non controlled Name is updated

r.Recorder = mgr.GetEventRecorderFor("gslb-controller")

endpointMapHandler := handler.EnqueueRequestsFromMapFunc(
func(_ context.Context, a client.Object) []reconcile.Request {
gslbList := &k8gbv1beta1.GslbList{}
Expand Down
4 changes: 2 additions & 2 deletions controllers/refresolver/ingress/ingress.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ func NewReferenceResolver(gslb *k8gbv1beta1.Gslb, k8sClient client.Client) (*Ref
}

if len(ingressList) != 1 {
return nil, fmt.Errorf("exactly one Ingress resource expected but %d were found", len(ingressList))
return nil, fmt.Errorf("exactly 1 Ingress resource expected but %d were found", len(ingressList))
}

return &ReferenceResolver{
Expand Down Expand Up @@ -93,7 +93,7 @@ func NewEmbeddedResolver(gslb *k8gbv1beta1.Gslb, k8sClient client.Client) (*Refe
return nil, err
}
if ingressEmbedded == nil {
return nil, fmt.Errorf("exactly one Ingress resource expected but none was found")
return nil, fmt.Errorf("exactly 1 Ingress resource expected but none was found")
}

return &ReferenceResolver{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ func NewReferenceResolver(gslb *k8gbv1beta1.Gslb, k8sClient client.Client) (*Ref
}

if len(virtualServiceList) != 1 {
return nil, fmt.Errorf("exactly one VirtualService resource expected but %d were found", len(virtualServiceList))
return nil, fmt.Errorf("exactly 1 VirtualService resource expected but %d were found", len(virtualServiceList))
}
virtualService := virtualServiceList[0]

Expand Down Expand Up @@ -98,7 +98,7 @@ func getGslbVirtualServiceRef(gslb *k8gbv1beta1.Gslb, k8sClient client.Client) (
// getGateway retrieves the istio gateway referenced by the istio virtual service
func getGateway(virtualService *istio.VirtualService, k8sClient client.Client) (*istio.Gateway, error) {
if len(virtualService.Spec.Gateways) != 1 {
return nil, fmt.Errorf("expected exactly one Gateway to be referenced by the VirtualService but %d were found", len(virtualService.Spec.Gateways))
return nil, fmt.Errorf("expected exactly 1 Gateway to be referenced by the VirtualService but %d were found", len(virtualService.Spec.Gateways))
}
gatewayRef := strings.Split(virtualService.Spec.Gateways[0], "/")
gatewayNamespace := gatewayRef[0]
Expand Down Expand Up @@ -140,7 +140,7 @@ func getLbService(gateway *istio.Gateway, k8sClient client.Client) (*corev1.Serv
}

if len(gatewayServiceList.Items) != 1 {
return nil, fmt.Errorf("exactly one Service resource expected but %d were found", len(gatewayServiceList.Items))
return nil, fmt.Errorf("exactly 1 Service resource expected but %d were found", len(gatewayServiceList.Items))
}

gatewayService := &gatewayServiceList.Items[0]
Expand Down
2 changes: 1 addition & 1 deletion controllers/refresolver/refresolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,5 +51,5 @@ func New(gslb *k8gbv1beta1.Gslb, k8sClient client.Client) (GslbReferenceResolver
return istiovirtualservice.NewReferenceResolver(gslb, k8sClient)
}
}
return nil, fmt.Errorf("incomplete gslb configuration, no ingress found")
return nil, fmt.Errorf("APIVersion:%s, Kind:%s not supported", gslb.Spec.ResourceRef.APIVersion, gslb.Spec.ResourceRef.Kind)
}

0 comments on commit b5ca251

Please sign in to comment.