From 63062fa18a8f7350786162b8cb4b73fc4413ee91 Mon Sep 17 00:00:00 2001 From: Gianluca Mardente Date: Sun, 11 Oct 2026 15:12:00 +0200 Subject: [PATCH] feat: record events for classifier failures and label conflicts Classifier errors were only logged, many of them at debug level, so a user had no way to see from the cluster why a Classifier or a ManagementClusterClassifier was not working. Record Warning events, using the reasons and actions from libsveltos lib/eventreasons. Classifier: - ClassifierFailed (Reconciling) when one of the reconcileNormal steps fails: matching clusters, cleanup, registrations, applying labels, ensuring ClassifierReports, updating internal maps. - ClassifierLabelConflict for a cluster whose labels are already managed by another Classifier. - ClassifierFailed (Deploying) when the deployment in a cluster ended with status Failed. ManagementClusterClassifier: ClassifierFailed for failures building the cluster sets, getting the key manager, removing stale labels, updating a report or applying labels, and for Lua evaluation errors; ClassifierLabelConflict as above. Deliberately no event for deployments still in progress, paused or not ready clusters, or an unhealthy agent: those are expected states, and the agent problem is a cluster problem that sveltoscluster-manager will report once per cluster. Note: errors in the report collection loop are not covered. Both reconcilers get an EventRecorder (nil means no event is recorded) and the manager role the events.k8s.io permission. The label application loop of the ManagementClusterClassifier moves into its own method to keep reconcileNormal within the statement limit. Known limitation: the events recorder merges events with the same reason, action and regarding object, keeping only the first note, so events for different clusters of the same Classifier are merged. --- Makefile | 2 +- config/default/manager_auth_proxy_patch.yaml | 2 +- config/default/manager_image_patch.yaml | 4 +- config/rbac/role.yaml | 8 + controllers/classifier_controller.go | 25 +++ controllers/classifier_controller_test.go | 10 + controllers/classifier_deployer.go | 26 +++ controllers/events.go | 53 +++++ controllers/events_test.go | 184 ++++++++++++++++++ controllers/export_test.go | 7 + .../mgmtcluster_classifier_controller.go | 80 ++++++-- go.mod | 2 +- go.sum | 4 +- main.go | 2 + manifest/deployment-agentless.yaml | 6 +- manifest/deployment-shard.yaml | 6 +- manifest/manifest.yaml | 14 +- pkg/agent/sveltos-agent-in-mgmt-cluster.go | 4 +- pkg/agent/sveltos-agent-in-mgmt-cluster.yaml | 4 +- pkg/agent/sveltos-agent.go | 4 +- pkg/agent/sveltos-agent.yaml | 4 +- pkg/agent/sveltos-applier.go | 4 +- pkg/agent/sveltos-applier.yaml | 4 +- test/pullmode-sveltosapplier.yaml | 2 +- 24 files changed, 417 insertions(+), 44 deletions(-) create mode 100644 controllers/events.go create mode 100644 controllers/events_test.go diff --git a/Makefile b/Makefile index c5a8929..ebb4131 100644 --- a/Makefile +++ b/Makefile @@ -46,7 +46,7 @@ ARCH ?= $(shell go env GOARCH) OS ?= $(shell uname -s | tr A-Z a-z) K8S_LATEST_VER ?= $(shell curl -s https://storage.googleapis.com/kubernetes-release/release/stable.txt) export CONTROLLER_IMG ?= $(REGISTRY)/$(IMAGE_NAME) -TAG ?= v1.16.0 +TAG ?= main ## Tool Binaries CONTROLLER_GEN := $(TOOLS_BIN_DIR)/controller-gen diff --git a/config/default/manager_auth_proxy_patch.yaml b/config/default/manager_auth_proxy_patch.yaml index 1c440e7..9839d83 100644 --- a/config/default/manager_auth_proxy_patch.yaml +++ b/config/default/manager_auth_proxy_patch.yaml @@ -16,7 +16,7 @@ spec: - "--shard-key=" - --capi-onboard-annotation= - "--v=5" - - "--version=v1.16.0" + - "--version=main" - "--registry=" - "--agent-in-mgmt-cluster=false" env: diff --git a/config/default/manager_image_patch.yaml b/config/default/manager_image_patch.yaml index 9662f31..d94ebc6 100644 --- a/config/default/manager_image_patch.yaml +++ b/config/default/manager_image_patch.yaml @@ -7,9 +7,9 @@ spec: template: spec: initContainers: - - image: docker.io/projectsveltos/classifier:v1.16.0 + - image: docker.io/projectsveltos/classifier:main name: migrate containers: # Change the value of image field below to your controller image URL - - image: docker.io/projectsveltos/classifier:v1.16.0 + - image: docker.io/projectsveltos/classifier:main name: manager diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index 7bf52b8..505f543 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -66,6 +66,14 @@ rules: - get - list - watch +- apiGroups: + - events.k8s.io + resources: + - events + verbs: + - create + - patch + - update - apiGroups: - lib.projectsveltos.io resources: diff --git a/controllers/classifier_controller.go b/controllers/classifier_controller.go index 9931dba..33f4550 100644 --- a/controllers/classifier_controller.go +++ b/controllers/classifier_controller.go @@ -27,6 +27,7 @@ import ( corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/client-go/tools/events" clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta2" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/builder" @@ -42,6 +43,7 @@ import ( libsveltosv1beta1 "github.com/projectsveltos/libsveltos/api/v1beta1" "github.com/projectsveltos/libsveltos/lib/clusterproxy" "github.com/projectsveltos/libsveltos/lib/deployer" + "github.com/projectsveltos/libsveltos/lib/eventreasons" logs "github.com/projectsveltos/libsveltos/lib/logsettings" predicates "github.com/projectsveltos/libsveltos/lib/predicates" libsveltosset "github.com/projectsveltos/libsveltos/lib/set" @@ -115,6 +117,9 @@ type ClassifierReconciler struct { // List of current existing Classifiers AllClassifierSet libsveltosset.Set + // EventRecorder is used to record events on Classifier instances. No event is recorded when nil. + EventRecorder events.EventRecorder + Logger logr.Logger } @@ -133,6 +138,7 @@ type ClassifierReconciler struct { //+kubebuilder:rbac:groups="",resources=secrets,verbs=get;list;watch //+kubebuilder:rbac:groups="",resources=configmaps,verbs=get;list;watch;delete //+kubebuilder:rbac:groups="apps",resources=deployments,verbs=get;list;watch +//+kubebuilder:rbac:groups="events.k8s.io",resources=events,verbs=create;update;patch func (r *ClassifierReconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ ctrl.Result, reterr error) { if r.ShardKey != "" { @@ -279,6 +285,7 @@ func (r *ClassifierReconciler) reconcileNormal( matchingClusters, err := r.syncAndGetMatchingClusters(ctx, classifierScope, logger) if err != nil { logger.V(logs.LogDebug).Error(err, "failed to get matching clusters") + r.recordReconcileFailure(classifierScope.Classifier, "get matching clusters", err) return reconcile.Result{RequeueAfter: normalRequeueAfter}, nil } trackMatchingClusters(classifierScope.Classifier.Name, len(matchingClusters), logger) @@ -288,6 +295,7 @@ func (r *ClassifierReconciler) reconcileNormal( if err != nil { // Use Error level because this indicates a failure to clean up resources logger.V(logs.LogDebug).Error(err, "failed to clean up labels/registrations for non-matching clusters") + r.recordReconcileFailure(classifierScope.Classifier, "clean up labels and registrations for clusters no longer matching", err) return reconcile.Result{RequeueAfter: normalRequeueAfter}, nil } @@ -295,6 +303,7 @@ func (r *ClassifierReconciler) reconcileNormal( matchingStatuses, err := r.updateMatchingClustersAndRegistrations(ctx, classifierScope, matchingClusters, logger) if err != nil { logger.V(logs.LogDebug).Error(err, "failed to update status/registrations for matching clusters") + r.recordReconcileFailure(classifierScope.Classifier, "update status and registrations for matching clusters", err) return reconcile.Result{RequeueAfter: normalRequeueAfter}, nil } trackLabelConflicts(classifierScope.Classifier.Name, countUnManagedLabelClusters(matchingStatuses), logger) @@ -303,23 +312,28 @@ func (r *ClassifierReconciler) reconcileNormal( err = r.updateLabelsOnMatchingClusters(ctx, classifierScope, matchingStatuses, logger) if err != nil { logger.V(logs.LogDebug).Error(err, "failed to apply labels to matching clusters") + r.recordReconcileFailure(classifierScope.Classifier, "apply labels to matching clusters", err) return reconcile.Result{RequeueAfter: normalRequeueAfter}, nil } err = r.ensureClassifierReports(ctx, classifierScope) if err != nil { logger.V(logs.LogDebug).Error(err, "failed to ensure ClassifierReports") + r.recordReconcileFailure(classifierScope.Classifier, "ensure ClassifierReports", err) return reconcile.Result{RequeueAfter: normalRequeueAfter}, nil } if err := r.updateMaps(ctx, classifierScope); err != nil { logger.V(logs.LogDebug).Error(err, "failed to update internal maps") + r.recordReconcileFailure(classifierScope.Classifier, "update internal maps", err) return reconcile.Result{RequeueAfter: normalRequeueAfter}, nil } f := getHandlersForFeature(libsveltosv1beta1.FeatureClassifier) if err := r.deployClassifier(ctx, classifierScope, f, logger); err != nil { + // No event here: the error is also returned while the deployment is still in progress. + // Terminal failures are recorded per cluster, in deployClassifier. logger.V(logs.LogInfo).Error(err, "failed to deploy") return reconcile.Result{RequeueAfter: normalRequeueAfter}, nil } @@ -332,6 +346,14 @@ func (r *ClassifierReconciler) reconcileNormal( return reconcile.Result{}, nil } +// recordReconcileFailure records a Warning event on the Classifier for a reconcile step that failed. +// Those errors are only logged at debug level and the reconciler retries, so without the event +// the failure is not visible to the user. +func (r *ClassifierReconciler) recordReconcileFailure(classifier *libsveltosv1beta1.Classifier, step string, err error) { + recordWarningf(r.EventRecorder, classifier, eventreasons.ClassifierFailed, eventreasons.ActionReconciling, + "failed to %s: %v", step, err) +} + // SetupWithManager sets up the controller with the Manager. func (r *ClassifierReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, logger logr.Logger) (controller.Controller, error) { @@ -525,6 +547,9 @@ func (r *ClassifierReconciler) updateMatchingClustersAndRegistrations(ctx contex if len(unmanaged) > 0 { hasUnmanaged = true + recordWarningf(r.EventRecorder, classifierScope.Classifier, eventreasons.ClassifierLabelConflict, + eventreasons.ActionReconciling, "cluster %s/%s: label keys %s are managed by another Classifier", + c.Namespace, c.Name, unmanagedLabelKeys(unmanaged)) } if err := updateClassifierReportLabelStatus(ctx, r.Client, classifierScope.Classifier.Name, diff --git a/controllers/classifier_controller_test.go b/controllers/classifier_controller_test.go index 97ba08c..f3ff274 100644 --- a/controllers/classifier_controller_test.go +++ b/controllers/classifier_controller_test.go @@ -28,6 +28,7 @@ import ( apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/tools/events" "k8s.io/klog/v2/textlogger" clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta2" ctrl "sigs.k8s.io/controller-runtime" @@ -321,12 +322,14 @@ var _ = Describe("Classifier: Reconciler", func() { // cluster even though cluster is a match manager.RegisterClassifierForLabels(classifier1, clusterNamespace, clusterName, libsveltosv1beta1.ClusterTypeCapi) + recorder := events.NewFakeRecorder(eventBufferSize) reconciler := &controllers.ClassifierReconciler{ Client: c, Scheme: scheme, ClusterMap: make(map[corev1.ObjectReference]*libsveltosset.Set), ClassifierMap: make(map[corev1.ObjectReference]*libsveltosset.Set), Mux: sync.Mutex{}, + EventRecorder: recorder, Logger: logger, } @@ -358,6 +361,13 @@ var _ = Describe("Classifier: Reconciler", func() { updatedReport)).To(Succeed()) Expect(updatedReport.Status.ManagedLabels).To(BeEmpty()) Expect(updatedReport.Status.UnManagedLabels).To(HaveLen(len(classifier.Spec.ClassifierLabels))) + + // The conflict is also reported as an event, so it is visible without reading ClassifierReport. + var recorded string + Expect(recorder.Events).To(Receive(&recorded)) + Expect(recorded).To(HavePrefix("Warning ClassifierLabelConflict cluster " + clusterNamespace + "/" + clusterName)) + Expect(recorded).To(ContainSubstring("managed by another Classifier")) + Expect(recorded).To(ContainSubstring(classifier.Spec.ClassifierLabels[0].Key)) }) It("updateLabelsOnMatchingClusters updates CAPI Cluster labels", func() { diff --git a/controllers/classifier_deployer.go b/controllers/classifier_deployer.go index a44181c..9c08304 100644 --- a/controllers/classifier_deployer.go +++ b/controllers/classifier_deployer.go @@ -52,6 +52,7 @@ import ( "github.com/projectsveltos/libsveltos/lib/clusterproxy" "github.com/projectsveltos/libsveltos/lib/crd" "github.com/projectsveltos/libsveltos/lib/deployer" + "github.com/projectsveltos/libsveltos/lib/eventreasons" "github.com/projectsveltos/libsveltos/lib/k8s_utils" logs "github.com/projectsveltos/libsveltos/lib/logsettings" "github.com/projectsveltos/libsveltos/lib/patcher" @@ -188,6 +189,7 @@ func (r *ClassifierReconciler) deployClassifier(ctx context.Context, classifierS errorSeen = updateErr } else { trackClassifierOutcome(classifierScope.Classifier.Name, cluster, clusterInfo, l) + r.recordDeploymentFailure(classifierScope.Classifier, cluster, clusterInfo) } if clusterInfo.Status != libsveltosv1beta1.SveltosStatusProvisioned { allProcessed = false @@ -204,6 +206,30 @@ func (r *ClassifierReconciler) deployClassifier(ctx context.Context, classifierS return nil } +// recordDeploymentFailure records a Warning event on the Classifier when the deployment in a cluster +// failed. Only terminal failures are recorded: a deployment that is still in progress, or a cluster +// that is paused or not ready yet, is an expected state and not something the user must act on. +func (r *ClassifierReconciler) recordDeploymentFailure(classifier *libsveltosv1beta1.Classifier, + cluster *corev1.ObjectReference, clusterInfo *libsveltosv1beta1.ClusterInfo) { + + if clusterInfo.FailureMessage == nil || *clusterInfo.FailureMessage == "" { + return + } + + switch clusterInfo.Status { + case libsveltosv1beta1.SveltosStatusFailed: + recordWarningf(r.EventRecorder, classifier, eventreasons.ClassifierFailed, eventreasons.ActionDeploying, + "cluster %s/%s: %s", cluster.Namespace, cluster.Name, *clusterInfo.FailureMessage) + case libsveltosv1beta1.SveltosStatusFailedNonRetriable: + // The only non retriable failure is the agent of a cluster in pull mode not reporting. That is a + // problem of the cluster, not of this Classifier: it is not reported here, once per Classifier, but + // once per cluster by sveltoscluster-manager. + case libsveltosv1beta1.SveltosStatusProvisioning, libsveltosv1beta1.SveltosStatusProvisioned, + libsveltosv1beta1.SveltosStatusRemoving, libsveltosv1beta1.SveltosStatusRemoved: + // Not a failure: nothing to report + } +} + func (r *ClassifierReconciler) undeployClassifier(ctx context.Context, classifierScope *scope.ClassifierScope, f feature, logger logr.Logger) error { diff --git a/controllers/events.go b/controllers/events.go new file mode 100644 index 0000000..93d15c5 --- /dev/null +++ b/controllers/events.go @@ -0,0 +1,53 @@ +/* +Copyright 2026. projectsveltos.io. All rights reserved. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package controllers + +import ( + "fmt" + "strings" + + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/client-go/tools/events" + + libsveltosv1beta1 "github.com/projectsveltos/libsveltos/api/v1beta1" + "github.com/projectsveltos/libsveltos/lib/eventreasons" +) + +// recordWarningf records a Warning event on regarding. It does nothing when no recorder is set. +// The note is capped, so an oversized message is not rejected by the API server. Identical events +// are aggregated by the recorder, so notes must not contain anything that changes between +// reconciliations (timestamps, counters). +func recordWarningf(recorder events.EventRecorder, regarding runtime.Object, reason, action, format string, args ...any) { + if recorder == nil { + return + } + + note := eventreasons.TruncateNote(fmt.Sprintf(format, args...)) + recorder.Eventf(regarding, nil, corev1.EventTypeWarning, reason, action, "%s", note) +} + +// unmanagedLabelKeys returns the keys of the labels the Classifier wants to manage but cannot, +// because they are managed by another instance, as a single comma separated string. +func unmanagedLabelKeys(unmanaged []libsveltosv1beta1.UnManagedLabel) string { + keys := make([]string, len(unmanaged)) + for i := range unmanaged { + keys[i] = unmanaged[i].Key + } + + return strings.Join(keys, ", ") +} diff --git a/controllers/events_test.go b/controllers/events_test.go new file mode 100644 index 0000000..2c7fcec --- /dev/null +++ b/controllers/events_test.go @@ -0,0 +1,184 @@ +/* +Copyright 2026. projectsveltos.io. All rights reserved. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package controllers_test + +import ( + "context" + "errors" + "strings" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/tools/events" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + + "github.com/projectsveltos/classifier/controllers" + libsveltosv1beta1 "github.com/projectsveltos/libsveltos/api/v1beta1" + "github.com/projectsveltos/libsveltos/lib/eventreasons" +) + +const eventBufferSize = 10 + +var _ = Describe("Classifier events", func() { + var classifier *libsveltosv1beta1.Classifier + var cluster *corev1.ObjectReference + var recorder *events.FakeRecorder + var reconciler *controllers.ClassifierReconciler + + BeforeEach(func() { + classifier = getClassifierInstance(randomString()) + cluster = &corev1.ObjectReference{ + Namespace: randomString(), + Name: randomString(), + APIVersion: libsveltosv1beta1.GroupVersion.String(), + Kind: libsveltosv1beta1.SveltosClusterKind, + } + recorder = events.NewFakeRecorder(eventBufferSize) + reconciler = &controllers.ClassifierReconciler{EventRecorder: recorder} + }) + + Context("recordWarningf", func() { + It("does nothing when no recorder is set", func() { + Expect(func() { + controllers.RecordWarningf(nil, classifier, eventreasons.ClassifierFailed, + eventreasons.ActionReconciling, "failed: %v", errors.New("boom")) + }).ToNot(Panic()) + }) + + It("records a Warning event with the reason and the note", func() { + controllers.RecordWarningf(recorder, classifier, eventreasons.ClassifierFailed, + eventreasons.ActionReconciling, "failed to do %s", "something") + + Expect(recorder.Events).To(Receive(Equal("Warning ClassifierFailed failed to do something"))) + }) + + It("does not interpret a percent sign in the message", func() { + controllers.RecordWarningf(recorder, classifier, eventreasons.ClassifierFailed, + eventreasons.ActionReconciling, "%s", "disk usage 100%d") + + Expect(recorder.Events).To(Receive(Equal("Warning ClassifierFailed disk usage 100%d"))) + }) + + It("caps a note longer than the API server accepts", func() { + longMessage := strings.Repeat("a", eventreasons.NoteMaxLength*2) + controllers.RecordWarningf(recorder, classifier, eventreasons.ClassifierFailed, + eventreasons.ActionReconciling, "%s", longMessage) + + var recorded string + Expect(recorder.Events).To(Receive(&recorded)) + note := strings.TrimPrefix(recorded, "Warning ClassifierFailed ") + Expect(len(note)).To(Equal(eventreasons.NoteMaxLength)) + Expect(note).To(HaveSuffix("...(truncated)")) + }) + }) + + Context("unmanagedLabelKeys", func() { + It("returns the keys separated by a comma", func() { + unmanaged := []libsveltosv1beta1.UnManagedLabel{{Key: "env"}, {Key: "region"}} + Expect(controllers.UnmanagedLabelKeys(unmanaged)).To(Equal("env, region")) + }) + }) + + Context("recordReconcileFailure", func() { + It("records a Warning ClassifierFailed event naming the step and the error", func() { + controllers.RecordReconcileFailure(reconciler, classifier, "apply labels to matching clusters", errors.New("boom")) + + Expect(recorder.Events).To(Receive(Equal( + "Warning ClassifierFailed failed to apply labels to matching clusters: boom"))) + }) + }) + + Context("recordDeploymentFailure", func() { + clusterInfo := func(status libsveltosv1beta1.SveltosFeatureStatus, failureMessage *string, + ) *libsveltosv1beta1.ClusterInfo { + + return &libsveltosv1beta1.ClusterInfo{Status: status, FailureMessage: failureMessage} + } + + message := func(s string) *string { return &s } + + It("records ClassifierFailed when the deployment failed", func() { + controllers.RecordDeploymentFailure(reconciler, classifier, cluster, + clusterInfo(libsveltosv1beta1.SveltosStatusFailed, message("failed to deploy CRDs"))) + + Expect(recorder.Events).To(Receive(Equal( + "Warning ClassifierFailed cluster " + cluster.Namespace + "/" + cluster.Name + ": failed to deploy CRDs"))) + }) + + It("records nothing when the agent of the cluster is not healthy: that is reported once per cluster", func() { + controllers.RecordDeploymentFailure(reconciler, classifier, cluster, + clusterInfo(libsveltosv1beta1.SveltosStatusFailedNonRetriable, + message("agent in managed cluster is not healthy."))) + + Expect(recorder.Events).To(BeEmpty()) + }) + + It("records nothing while the deployment is in progress, even with a message", func() { + controllers.RecordDeploymentFailure(reconciler, classifier, cluster, + clusterInfo(libsveltosv1beta1.SveltosStatusProvisioning, message("cleanup is in progress"))) + + Expect(recorder.Events).To(BeEmpty()) + }) + + It("records nothing when there is no failure message", func() { + controllers.RecordDeploymentFailure(reconciler, classifier, cluster, + clusterInfo(libsveltosv1beta1.SveltosStatusFailed, message(""))) + controllers.RecordDeploymentFailure(reconciler, classifier, cluster, + clusterInfo(libsveltosv1beta1.SveltosStatusFailed, nil)) + + Expect(recorder.Events).To(BeEmpty()) + }) + }) +}) + +var _ = Describe("ManagementClusterClassifier events", func() { + It("records a Warning ClassifierFailed event when the classification script cannot run", func() { + mcc := &libsveltosv1beta1.ManagementClusterClassifier{ + ObjectMeta: metav1.ObjectMeta{Name: randomString()}, + Spec: libsveltosv1beta1.ManagementClusterClassifierSpec{ + ClassificationLua: "not valid lua !@#", + ClassifierLabels: []libsveltosv1beta1.ClassifierLabel{ + {Key: testLabelEnv, Value: testValueProd}, + }, + }, + } + + fakeClient := fake.NewClientBuilder(). + WithScheme(mgmtTestScheme()). + WithObjects(mcc). + Build() + + recorder := events.NewFakeRecorder(eventBufferSize) + reconciler := &controllers.ManagementClusterClassifierReconciler{ + Client: fakeClient, + EventRecorder: recorder, + } + + _, err := reconciler.Reconcile(context.TODO(), + ctrl.Request{NamespacedName: types.NamespacedName{Name: mcc.Name}}) + Expect(err).ToNot(HaveOccurred()) + + var recorded string + Expect(recorder.Events).To(Receive(&recorded)) + Expect(recorded).To(HavePrefix("Warning ClassifierFailed failed to build cluster sets: ")) + }) +}) diff --git a/controllers/export_test.go b/controllers/export_test.go index 54d7860..f24ca7d 100644 --- a/controllers/export_test.go +++ b/controllers/export_test.go @@ -89,6 +89,13 @@ var ( CleanUpNonMatchingClusters = (*ClassifierReconciler).cleanUpNonMatchingClusters RemoveStaleLabelsFromMatchingClusters = (*ClassifierReconciler).removeStaleLabelsFromMatchingClusters CleanUpManagedResources = (*ClassifierReconciler).cleanUpManagedResources + RecordDeploymentFailure = (*ClassifierReconciler).recordDeploymentFailure + RecordReconcileFailure = (*ClassifierReconciler).recordReconcileFailure +) + +var ( + RecordWarningf = recordWarningf + UnmanagedLabelKeys = unmanagedLabelKeys ) var ( diff --git a/controllers/mgmtcluster_classifier_controller.go b/controllers/mgmtcluster_classifier_controller.go index e3b9618..0ec5c04 100644 --- a/controllers/mgmtcluster_classifier_controller.go +++ b/controllers/mgmtcluster_classifier_controller.go @@ -28,6 +28,7 @@ import ( "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/client-go/tools/events" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/builder" "sigs.k8s.io/controller-runtime/pkg/client" @@ -39,6 +40,7 @@ import ( "github.com/projectsveltos/classifier/controllers/keymanager" libsveltosv1beta1 "github.com/projectsveltos/libsveltos/api/v1beta1" + "github.com/projectsveltos/libsveltos/lib/eventreasons" logs "github.com/projectsveltos/libsveltos/lib/logsettings" libsveltosset "github.com/projectsveltos/libsveltos/lib/set" ) @@ -66,6 +68,9 @@ type ManagementClusterClassifierReconciler struct { watchedGVKs map[schema.GroupVersionKind]bool watchedGVKsMux sync.RWMutex + // EventRecorder is used to record events on ManagementClusterClassifier instances. No event is recorded when nil. + EventRecorder events.EventRecorder + Logger logr.Logger } @@ -122,6 +127,7 @@ func (r *ManagementClusterClassifierReconciler) reconcileNormal( if err != nil { logger.V(logs.LogInfo).Error(err, "failed to build cluster sets") _ = setMgmtClassifierFailureMessage(ctx, r.Client, mcc, err.Error()) + r.recordReconcileFailure(mcc, "build cluster sets", err) return reconcile.Result{RequeueAfter: normalRequeueAfter}, nil } trackMgmtClusterMatchingClusters(mcc.Name, len(newClusters), logger) @@ -129,6 +135,7 @@ func (r *ManagementClusterClassifierReconciler) reconcileNormal( km, err := keymanager.GetKeyManagerInstance(ctx, r.Client) if err != nil { logger.V(logs.LogInfo).Error(err, "failed to get keymanager") + r.recordReconcileFailure(mcc, "get keymanager", err) return reconcile.Result{RequeueAfter: normalRequeueAfter}, nil } @@ -137,12 +144,54 @@ func (r *ManagementClusterClassifierReconciler) reconcileNormal( // Remove labels from clusters that are no longer matched and delete their reports. if err := r.removeStaleClusterLabels(ctx, fakeClassifier, mcc.Name, oldClusters, newClusters, logger); err != nil { logger.V(logs.LogInfo).Error(err, "failed to remove stale cluster labels") + r.recordReconcileFailure(mcc, "remove stale cluster labels", err) return reconcile.Result{RequeueAfter: normalRequeueAfter}, nil } + conflictCount := r.applyLabelsToMatchingClusters(ctx, mcc, fakeClassifier, km, newClusters, logger) + trackMgmtClusterLabelConflicts(mcc.Name, conflictCount, logger) + + // Update failure message: Lua errors plus any recorded conflicts. + failMsg := strings.Join(luaFailures, "; ") + if err := setMgmtClassifierFailureMessage(ctx, r.Client, mcc, failMsg); err != nil { + logger.V(logs.LogInfo).Error(err, "failed to update failure message") + } + if failMsg != "" { + recordWarningf(r.EventRecorder, mcc, eventreasons.ClassifierFailed, eventreasons.ActionEvaluating, + "failed to evaluate matching resources: %s", failMsg) + } + + // Sync GVK map and register dynamic watches for spec.matchResources. + r.syncGVKWatches(mcc, logger) + + if conflictCount > 0 { + return reconcile.Result{RequeueAfter: conflictRequeueAfter}, nil + } + + logger.V(logs.LogDebug).Info("Reconcile success") + return reconcile.Result{}, nil +} + +// mgmtLabelKeyManager is the part of the label key manager used to register the labels of a +// ManagementClusterClassifier on a cluster and to find out which of them it can manage. +type mgmtLabelKeyManager interface { + RemoveStaleRegistrations(*libsveltosv1beta1.Classifier, string, string, libsveltosv1beta1.ClusterType) + RegisterClassifierForLabels(*libsveltosv1beta1.Classifier, string, string, libsveltosv1beta1.ClusterType) + CanManageLabel(*libsveltosv1beta1.Classifier, string, string, string, libsveltosv1beta1.ClusterType) bool + GetManagerForKey(string, string, string, libsveltosv1beta1.ClusterType) (string, error) +} + +// applyLabelsToMatchingClusters registers the labels of the ManagementClusterClassifier on each matching +// cluster, updates the report of the cluster and applies the labels the classifier can manage. +// It returns the number of clusters where at least one label is already managed by another classifier. +func (r *ManagementClusterClassifierReconciler) applyLabelsToMatchingClusters( + ctx context.Context, mcc *libsveltosv1beta1.ManagementClusterClassifier, + fakeClassifier *libsveltosv1beta1.Classifier, km mgmtLabelKeyManager, + newClusters map[string]clusterRef, logger logr.Logger, +) int { + conflictCount := 0 - // Apply labels to newly matched clusters. for key, ref := range newClusters { clusterType, err := clusterTypeFromKind(ref.Kind) if err != nil { @@ -152,6 +201,7 @@ func (r *ManagementClusterClassifierReconciler) reconcileNormal( report, err := ensureMgmtClassifierReport(ctx, r.Client, mcc.Name, ref.Namespace, ref.Name, clusterType) if err != nil { logger.V(logs.LogInfo).Error(err, fmt.Sprintf("failed to ensure report for cluster %s", key)) + r.recordReconcileFailure(mcc, fmt.Sprintf("ensure report for cluster %s", key), err) continue } @@ -163,10 +213,14 @@ func (r *ManagementClusterClassifierReconciler) reconcileNormal( if len(unmanaged) > 0 { conflictCount++ + recordWarningf(r.EventRecorder, mcc, eventreasons.ClassifierLabelConflict, eventreasons.ActionReconciling, + "cluster %s/%s: label keys %s are managed by another Classifier", + ref.Namespace, ref.Name, unmanagedLabelKeys(unmanaged)) } if err := updateMgmtClassifierReportStatus(ctx, r.Client, report, managed, unmanaged); err != nil { logger.V(logs.LogInfo).Error(err, fmt.Sprintf("failed to update report status for cluster %s", key)) + r.recordReconcileFailure(mcc, fmt.Sprintf("update report status for cluster %s", key), err) } labelsToApply := r.filterManagedLabels(mcc.Spec.ClassifierLabels, managed) @@ -174,26 +228,22 @@ func (r *ManagementClusterClassifierReconciler) reconcileNormal( labelsToApply, logger); err != nil { if !apierrors.IsNotFound(err) { logger.V(logs.LogInfo).Error(err, fmt.Sprintf("failed to apply labels to cluster %s", key)) + r.recordReconcileFailure(mcc, fmt.Sprintf("apply labels to cluster %s", key), err) } } } - trackMgmtClusterLabelConflicts(mcc.Name, conflictCount, logger) - // Update failure message: Lua errors plus any recorded conflicts. - failMsg := strings.Join(luaFailures, "; ") - if err := setMgmtClassifierFailureMessage(ctx, r.Client, mcc, failMsg); err != nil { - logger.V(logs.LogInfo).Error(err, "failed to update failure message") - } + return conflictCount +} - // Sync GVK map and register dynamic watches for spec.matchResources. - r.syncGVKWatches(mcc, logger) +// recordReconcileFailure records a Warning event on the ManagementClusterClassifier for a reconcile step +// that failed. Those errors are only logged and the reconciler retries, so without the event the +// failure is not visible to the user. +func (r *ManagementClusterClassifierReconciler) recordReconcileFailure(mcc *libsveltosv1beta1.ManagementClusterClassifier, + step string, err error) { - if conflictCount > 0 { - return reconcile.Result{RequeueAfter: conflictRequeueAfter}, nil - } - - logger.V(logs.LogDebug).Info("Reconcile success") - return reconcile.Result{}, nil + recordWarningf(r.EventRecorder, mcc, eventreasons.ClassifierFailed, eventreasons.ActionReconciling, + "failed to %s: %v", step, err) } func (r *ManagementClusterClassifierReconciler) reconcileDelete( diff --git a/go.mod b/go.mod index 62b398d..07cad33 100644 --- a/go.mod +++ b/go.mod @@ -9,7 +9,7 @@ require ( github.com/onsi/ginkgo/v2 v2.33.0 github.com/onsi/gomega v1.44.0 github.com/pkg/errors v0.9.1 - github.com/projectsveltos/libsveltos v1.16.0 + github.com/projectsveltos/libsveltos v1.16.1-0.20261011120654-3d6f8446a989 github.com/prometheus/client_golang v1.24.1 github.com/spf13/pflag v1.0.10 github.com/yuin/gopher-lua v1.1.2 diff --git a/go.sum b/go.sum index 1ad48e2..07ed008 100644 --- a/go.sum +++ b/go.sum @@ -208,8 +208,8 @@ github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINE github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 h1:Jamvg5psRIccs7FGNTlIRMkT8wgtp5eCXdBlqhYGL6U= github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= -github.com/projectsveltos/libsveltos v1.16.0 h1:XdmoMatjwU1a2Ede0FY5eddxlvD4CGx7Bse+VZ7qxFc= -github.com/projectsveltos/libsveltos v1.16.0/go.mod h1:wup25BAf1SHuG6FsY8D+tnm1U3/3NiuzcKec/In/fB4= +github.com/projectsveltos/libsveltos v1.16.1-0.20261011120654-3d6f8446a989 h1:jVCF78Z8fn6bQhDlf4QqTe7tsZEQ0jxM6ApgI0MR6n8= +github.com/projectsveltos/libsveltos v1.16.1-0.20261011120654-3d6f8446a989/go.mod h1:wup25BAf1SHuG6FsY8D+tnm1U3/3NiuzcKec/In/fB4= github.com/projectsveltos/lua-utils/glua-json v0.0.0-20251212200258-2b3cdcb7c0f5 h1:khnc+994UszxZYu69J+R5FKiLA/Nk1JQj0EYAkwTWz0= github.com/projectsveltos/lua-utils/glua-json v0.0.0-20251212200258-2b3cdcb7c0f5/go.mod h1:yVL8KQFa9tmcxgwl9nwIMtKgtmIVC1zaFRSCfOwYvPY= github.com/projectsveltos/lua-utils/glua-runes v0.0.0-20251212200258-2b3cdcb7c0f5 h1:YbsebwRwTRhV8QacvEAdFqxcxHdeu7JTVtsBovbkgos= diff --git a/main.go b/main.go index 5ee3e41..b723918 100644 --- a/main.go +++ b/main.go @@ -381,6 +381,7 @@ func getMgmtClassifierReconciler(mgr manager.Manager) *controllers.ManagementClu Scheme: mgr.GetScheme(), ShardKey: shardKey, GVKToClassifiers: make(map[schema.GroupVersionKind]*libsveltosset.Set), + EventRecorder: mgr.GetEventRecorder("event-recorder"), Logger: ctrl.Log.WithName("managementclusterclassifierreconciler"), } } @@ -398,6 +399,7 @@ func getClassifierReconciler(mgr manager.Manager) *controllers.ClassifierReconci ClassifierReportMode: reportMode, ControlPlaneEndpoint: managementClusterControlPlaneEndpoint, Mux: sync.Mutex{}, + EventRecorder: mgr.GetEventRecorder("event-recorder"), Logger: ctrl.Log.WithName("classifierreconiler"), } } diff --git a/manifest/deployment-agentless.yaml b/manifest/deployment-agentless.yaml index ee08af7..cf76e69 100644 --- a/manifest/deployment-agentless.yaml +++ b/manifest/deployment-agentless.yaml @@ -26,7 +26,7 @@ spec: - --shard-key= - --capi-onboard-annotation= - --v=5 - - --version=v1.16.0 + - --version=main - --registry= - --agent-in-mgmt-cluster=true command: @@ -44,7 +44,7 @@ spec: valueFrom: fieldRef: fieldPath: metadata.namespace - image: docker.io/projectsveltos/classifier:v1.16.0 + image: docker.io/projectsveltos/classifier:main imagePullPolicy: IfNotPresent livenessProbe: failureThreshold: 3 @@ -92,7 +92,7 @@ spec: fieldPath: metadata.namespace - name: IS_INITIALIZATION value: "true" - image: docker.io/projectsveltos/classifier:v1.16.0 + image: docker.io/projectsveltos/classifier:main imagePullPolicy: IfNotPresent name: migrate resources: diff --git a/manifest/deployment-shard.yaml b/manifest/deployment-shard.yaml index 616e7ff..dc207b7 100644 --- a/manifest/deployment-shard.yaml +++ b/manifest/deployment-shard.yaml @@ -26,7 +26,7 @@ spec: - --shard-key={{.SHARD}} - --capi-onboard-annotation= - --v=5 - - --version=v1.16.0 + - --version=main - --registry= - --agent-in-mgmt-cluster=false command: @@ -44,7 +44,7 @@ spec: valueFrom: fieldRef: fieldPath: metadata.namespace - image: docker.io/projectsveltos/classifier:v1.16.0 + image: docker.io/projectsveltos/classifier:main imagePullPolicy: IfNotPresent livenessProbe: failureThreshold: 3 @@ -92,7 +92,7 @@ spec: fieldPath: metadata.namespace - name: IS_INITIALIZATION value: "true" - image: docker.io/projectsveltos/classifier:v1.16.0 + image: docker.io/projectsveltos/classifier:main imagePullPolicy: IfNotPresent name: migrate resources: diff --git a/manifest/manifest.yaml b/manifest/manifest.yaml index 0831049..5c11792 100644 --- a/manifest/manifest.yaml +++ b/manifest/manifest.yaml @@ -81,6 +81,14 @@ rules: - get - list - watch +- apiGroups: + - events.k8s.io + resources: + - events + verbs: + - create + - patch + - update - apiGroups: - lib.projectsveltos.io resources: @@ -259,7 +267,7 @@ spec: - --shard-key= - --capi-onboard-annotation= - --v=5 - - --version=v1.16.0 + - --version=main - --registry= - --agent-in-mgmt-cluster=false command: @@ -277,7 +285,7 @@ spec: valueFrom: fieldRef: fieldPath: metadata.namespace - image: docker.io/projectsveltos/classifier:v1.16.0 + image: docker.io/projectsveltos/classifier:main imagePullPolicy: IfNotPresent livenessProbe: failureThreshold: 3 @@ -325,7 +333,7 @@ spec: fieldPath: metadata.namespace - name: IS_INITIALIZATION value: "true" - image: docker.io/projectsveltos/classifier:v1.16.0 + image: docker.io/projectsveltos/classifier:main imagePullPolicy: IfNotPresent name: migrate resources: diff --git a/pkg/agent/sveltos-agent-in-mgmt-cluster.go b/pkg/agent/sveltos-agent-in-mgmt-cluster.go index cc145f3..d163c72 100644 --- a/pkg/agent/sveltos-agent-in-mgmt-cluster.go +++ b/pkg/agent/sveltos-agent-in-mgmt-cluster.go @@ -42,7 +42,7 @@ spec: - --cluster-namespace= - --cluster-name= - --cluster-type= - - --version=v1.16.0 + - --version=main - --current-cluster=management-cluster - --run-mode=do-not-send-reports - --discard-managed-fields=true @@ -63,7 +63,7 @@ spec: valueFrom: fieldRef: fieldPath: metadata.namespace - image: docker.io/projectsveltos/sveltos-agent@sha256:879d47e86797a36df50f984ca43b5d75a2da39bb8c36bc8f665d239ea9f29bfc + image: docker.io/projectsveltos/sveltos-agent@sha256:0bd50ceef7cb9599f5bbf57d07ad01fba2e8cf5d9650b2a3f41229ae30f6a55b livenessProbe: failureThreshold: 3 httpGet: diff --git a/pkg/agent/sveltos-agent-in-mgmt-cluster.yaml b/pkg/agent/sveltos-agent-in-mgmt-cluster.yaml index e4df6c2..7c57de0 100644 --- a/pkg/agent/sveltos-agent-in-mgmt-cluster.yaml +++ b/pkg/agent/sveltos-agent-in-mgmt-cluster.yaml @@ -24,7 +24,7 @@ spec: - --cluster-namespace= - --cluster-name= - --cluster-type= - - --version=v1.16.0 + - --version=main - --current-cluster=management-cluster - --run-mode=do-not-send-reports - --discard-managed-fields=true @@ -45,7 +45,7 @@ spec: valueFrom: fieldRef: fieldPath: metadata.namespace - image: docker.io/projectsveltos/sveltos-agent@sha256:879d47e86797a36df50f984ca43b5d75a2da39bb8c36bc8f665d239ea9f29bfc + image: docker.io/projectsveltos/sveltos-agent@sha256:0bd50ceef7cb9599f5bbf57d07ad01fba2e8cf5d9650b2a3f41229ae30f6a55b livenessProbe: failureThreshold: 3 httpGet: diff --git a/pkg/agent/sveltos-agent.go b/pkg/agent/sveltos-agent.go index 939555c..67f987a 100644 --- a/pkg/agent/sveltos-agent.go +++ b/pkg/agent/sveltos-agent.go @@ -191,7 +191,7 @@ spec: - --cluster-namespace= - --cluster-name= - --cluster-type= - - --version=v1.16.0 + - --version=main - --current-cluster=managed-cluster - --run-mode=do-not-send-reports - --discard-managed-fields=true @@ -212,7 +212,7 @@ spec: valueFrom: fieldRef: fieldPath: metadata.namespace - image: docker.io/projectsveltos/sveltos-agent@sha256:879d47e86797a36df50f984ca43b5d75a2da39bb8c36bc8f665d239ea9f29bfc + image: docker.io/projectsveltos/sveltos-agent@sha256:0bd50ceef7cb9599f5bbf57d07ad01fba2e8cf5d9650b2a3f41229ae30f6a55b livenessProbe: failureThreshold: 3 httpGet: diff --git a/pkg/agent/sveltos-agent.yaml b/pkg/agent/sveltos-agent.yaml index 4f9bd86..f59add6 100644 --- a/pkg/agent/sveltos-agent.yaml +++ b/pkg/agent/sveltos-agent.yaml @@ -173,7 +173,7 @@ spec: - --cluster-namespace= - --cluster-name= - --cluster-type= - - --version=v1.16.0 + - --version=main - --current-cluster=managed-cluster - --run-mode=do-not-send-reports - --discard-managed-fields=true @@ -194,7 +194,7 @@ spec: valueFrom: fieldRef: fieldPath: metadata.namespace - image: docker.io/projectsveltos/sveltos-agent@sha256:879d47e86797a36df50f984ca43b5d75a2da39bb8c36bc8f665d239ea9f29bfc + image: docker.io/projectsveltos/sveltos-agent@sha256:0bd50ceef7cb9599f5bbf57d07ad01fba2e8cf5d9650b2a3f41229ae30f6a55b livenessProbe: failureThreshold: 3 httpGet: diff --git a/pkg/agent/sveltos-applier.go b/pkg/agent/sveltos-applier.go index f911721..bb68927 100644 --- a/pkg/agent/sveltos-applier.go +++ b/pkg/agent/sveltos-applier.go @@ -102,7 +102,7 @@ spec: - --secret-with-kubeconfig= - --watch-namespaces= - --v=5 - - --version=v1.16.0 + - --version=main command: - /manager env: @@ -118,7 +118,7 @@ spec: valueFrom: fieldRef: fieldPath: metadata.namespace - image: docker.io/projectsveltos/sveltos-applier@sha256:184195269045fce39e9f4e55591280a849ee373c62672624ce694f87ac70ae87 + image: docker.io/projectsveltos/sveltos-applier@sha256:b2bc482858176524c7cd28e272794ced4156359bd58c1c21de043ac18c7e3221 livenessProbe: failureThreshold: 3 httpGet: diff --git a/pkg/agent/sveltos-applier.yaml b/pkg/agent/sveltos-applier.yaml index c816b8b..f7aedb5 100644 --- a/pkg/agent/sveltos-applier.yaml +++ b/pkg/agent/sveltos-applier.yaml @@ -84,7 +84,7 @@ spec: - --secret-with-kubeconfig= - --watch-namespaces= - --v=5 - - --version=v1.16.0 + - --version=main command: - /manager env: @@ -100,7 +100,7 @@ spec: valueFrom: fieldRef: fieldPath: metadata.namespace - image: docker.io/projectsveltos/sveltos-applier@sha256:184195269045fce39e9f4e55591280a849ee373c62672624ce694f87ac70ae87 + image: docker.io/projectsveltos/sveltos-applier@sha256:b2bc482858176524c7cd28e272794ced4156359bd58c1c21de043ac18c7e3221 livenessProbe: failureThreshold: 3 httpGet: diff --git a/test/pullmode-sveltosapplier.yaml b/test/pullmode-sveltosapplier.yaml index 6546dff..9c3adb6 100644 --- a/test/pullmode-sveltosapplier.yaml +++ b/test/pullmode-sveltosapplier.yaml @@ -100,7 +100,7 @@ spec: valueFrom: fieldRef: fieldPath: metadata.namespace - image: docker.io/projectsveltos/sveltos-applier@sha256:184195269045fce39e9f4e55591280a849ee373c62672624ce694f87ac70ae87 + image: docker.io/projectsveltos/sveltos-applier@sha256:b2bc482858176524c7cd28e272794ced4156359bd58c1c21de043ac18c7e3221 livenessProbe: failureThreshold: 3 httpGet: