From fae2e410e8db2bf7643d75bb3397ab02b9335e72 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20=C3=81guas?= Date: Thu, 31 Oct 2024 21:25:05 +0100 Subject: [PATCH] Record event if GSLB references cannot be resolved (#1769) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In the [PR](https://github.com/k8gb-io/k8gb/pull/1672#pullrequestreview-2305623370) 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 --- .golangci.yaml | 2 +- Makefile | 2 +- chart/k8gb/templates/role.yaml | 7 +++++++ controllers/gslb_controller_reconciliation.go | 14 +++++++++++--- controllers/gslb_controller_setup.go | 2 ++ controllers/refresolver/ingress/ingress.go | 4 ++-- .../istiovirtualservice/istiovirtualservice.go | 6 +++--- controllers/refresolver/refresolver.go | 2 +- 8 files changed, 28 insertions(+), 11 deletions(-) diff --git a/.golangci.yaml b/.golangci.yaml index 4a9f7ef5bf..71439cf05d 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -23,7 +23,7 @@ linters: - unused - depguard - dogsled - - exportloopref + - copyloopvar - goconst - goprintffuncname - nolintlint diff --git a/Makefile b/Makefile index 3d062f4126..97b3ece241 100644 --- a/Makefile +++ b/Makefile @@ -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 diff --git a/chart/k8gb/templates/role.yaml b/chart/k8gb/templates/role.yaml index 2455c5b263..fff21f50e7 100644 --- a/chart/k8gb/templates/role.yaml +++ b/chart/k8gb/templates/role.yaml @@ -7,6 +7,13 @@ metadata: labels: {{ include "chart.labels" . | indent 4 }} rules: +- apiGroups: + - "" + resources: + - events + verbs: + - create + - patch - apiGroups: - "" resources: diff --git a/controllers/gslb_controller_reconciliation.go b/controllers/gslb_controller_reconciliation.go index 63434d54a1..c626d74e0b 100644 --- a/controllers/gslb_controller_reconciliation.go +++ b/controllers/gslb_controller_reconciliation.go @@ -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" ) @@ -47,6 +51,7 @@ type GslbReconciler struct { Config *depresolver.Config DepResolver depresolver.GslbResolver DNSProvider dns.Provider + Recorder record.EventRecorder Tracer trace.Tracer } @@ -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) { @@ -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 @@ -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 { diff --git a/controllers/gslb_controller_setup.go b/controllers/gslb_controller_setup.go index 3c1d7e60ca..a62a3db8c6 100644 --- a/controllers/gslb_controller_setup.go +++ b/controllers/gslb_controller_setup.go @@ -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{} diff --git a/controllers/refresolver/ingress/ingress.go b/controllers/refresolver/ingress/ingress.go index a6aed4a7ad..8c4a4c5034 100644 --- a/controllers/refresolver/ingress/ingress.go +++ b/controllers/refresolver/ingress/ingress.go @@ -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{ @@ -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{ diff --git a/controllers/refresolver/istiovirtualservice/istiovirtualservice.go b/controllers/refresolver/istiovirtualservice/istiovirtualservice.go index 87d824e517..93f4050f68 100644 --- a/controllers/refresolver/istiovirtualservice/istiovirtualservice.go +++ b/controllers/refresolver/istiovirtualservice/istiovirtualservice.go @@ -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] @@ -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] @@ -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] diff --git a/controllers/refresolver/refresolver.go b/controllers/refresolver/refresolver.go index dc4a60a411..6b9067f1a2 100644 --- a/controllers/refresolver/refresolver.go +++ b/controllers/refresolver/refresolver.go @@ -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) }