From 32b38f809cd48d77c160468950c5d3da980dfcb6 Mon Sep 17 00:00:00 2001 From: Felix Breuer Date: Mon, 12 Oct 2026 16:24:33 +0200 Subject: [PATCH 1/2] chore: add error logging and event recording Signed-off-by: Felix Breuer --- controller/stackitcluster_bastion.go | 10 +++ controller/stackitcluster_controller_test.go | 77 ++++++++++++++++++++ controller/stackitcluster_infrastructure.go | 73 ++++++++++++++++++- controller/stackitmachine_controller_test.go | 61 ++++++++++++++++ controller/stackitmachine_infrastructure.go | 52 +++++++++++++ 5 files changed, 272 insertions(+), 1 deletion(-) diff --git a/controller/stackitcluster_bastion.go b/controller/stackitcluster_bastion.go index a4268d0..73dae55 100644 --- a/controller/stackitcluster_bastion.go +++ b/controller/stackitcluster_bastion.go @@ -23,6 +23,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" ctrl "sigs.k8s.io/controller-runtime" + logf "sigs.k8s.io/controller-runtime/pkg/log" infrav1 "github.com/stackitcloud/cluster-api-provider-stackit/api/v1alpha1" "github.com/stackitcloud/cluster-api-provider-stackit/cloud" @@ -39,6 +40,7 @@ func (r *StackitClusterReconciler) reconcileBastion( cloudClient cloud.Client, clusterScope *scope.ClusterScope, ) (ctrl.Result, bool, error) { + log := logf.FromContext(ctx) stackitCluster := clusterScope.StackitCluster input := bastionservice.Input(stackitCluster, nil) status := cloud.Bastion{ @@ -75,6 +77,10 @@ func (r *StackitClusterReconciler) reconcileBastion( } if err := validateBastionSpec(stackitCluster.Spec.Bastion); err != nil { + log.Error(err, "Invalid bastion spec", "cluster", stackitCluster.Name) + if r.Recorder != nil { + r.Recorder.Eventf(stackitCluster, nil, corev1.EventTypeWarning, "InvalidBastionSpec", "Reconcile", "Invalid bastion spec: %v", err) + } clusterScope.SetNotReady( "InvalidBastionSpec", err.Error(), @@ -86,6 +92,10 @@ func (r *StackitClusterReconciler) reconcileBastion( cloudInit, err := r.resolveBastionCloudInit(ctx, stackitCluster) if err != nil { + log.Error(err, "Failed to resolve bastion cloud-init", "cluster", stackitCluster.Name) + if r.Recorder != nil { + r.Recorder.Eventf(stackitCluster, nil, corev1.EventTypeWarning, "CloudInitRefError", "Reconcile", "Failed to resolve bastion cloudInitRef: %v", err) + } clusterScope.SetNotReady( "CloudInitRefError", err.Error(), diff --git a/controller/stackitcluster_controller_test.go b/controller/stackitcluster_controller_test.go index 6b1e72b..4ae794a 100644 --- a/controller/stackitcluster_controller_test.go +++ b/controller/stackitcluster_controller_test.go @@ -414,6 +414,8 @@ var _ = Describe("StackitCluster Controller", func() { }) It("marks the bastion not ready when cloud-init ref is missing", func() { + recorder := events.NewFakeRecorder(10) + reconciler.Recorder = recorder got := &infrav1.StackitCluster{} Expect(k8sClient.Get(ctx, stackitKey, got)).To(Succeed()) got.Spec.Bastion = validBastionSpec() @@ -432,6 +434,9 @@ var _ = Describe("StackitCluster Controller", func() { Expect(got.Status.Ready).To(BeFalse()) Expect(fakeCloud.ServerCount()).To(Equal(0)) expectCondition(got.Status.Conditions, infrav1.ClusterBastionReadyCondition, metav1.ConditionFalse, "CloudInitRefError") + + events := drainEvents(recorder) + Expect(events).To(ContainElement(ContainSubstring("CloudInitRefError"))) }) It("recreates the bastion and node SSH access when referenced cloud-init changes", func() { @@ -487,6 +492,8 @@ var _ = Describe("StackitCluster Controller", func() { }) It("marks the network not ready when the configured network does not exist", func() { + recorder := events.NewFakeRecorder(10) + reconciler.Recorder = recorder got := &infrav1.StackitCluster{} Expect(k8sClient.Get(ctx, stackitKey, got)).To(Succeed()) got.Spec.Network.ID = "99999999-9999-9999-9999-999999999999" @@ -500,6 +507,49 @@ var _ = Describe("StackitCluster Controller", func() { Expect(got.Status.Ready).To(BeFalse()) expectCondition(got.Status.Conditions, infrav1.ClusterNetworkReadyCondition, metav1.ConditionFalse, "NetworkNotFound") expectCondition(got.Status.Conditions, infrav1.ClusterReadyCondition, metav1.ConditionFalse, "NetworkNotFound") + + events := drainEvents(recorder) + Expect(events).To(ContainElement(ContainSubstring("FailedGetNetwork"))) + }) + + It("records a warning event and logs when API server load balancer creation fails", func() { + recorder := events.NewFakeRecorder(10) + reconciler.Recorder = recorder + fakeCloud.FailNextEnsureLB = fmt.Errorf("load balancer creation rejected: %w", cloud.ErrInvalidInput) + + result, err := reconciler.Reconcile(ctx, request) + Expect(err).NotTo(HaveOccurred()) + Expect(result).To(Equal(reconcile.Result{})) + + got := &infrav1.StackitCluster{} + Expect(k8sClient.Get(ctx, stackitKey, got)).To(Succeed()) + Expect(got.Status.Ready).To(BeFalse()) + expectCondition(got.Status.Conditions, infrav1.ClusterLoadBalancerReadyCondition, metav1.ConditionFalse, "LoadBalancerError") + + events := drainEvents(recorder) + Expect(events).To(ContainElement(ContainSubstring("FailedCreateLoadBalancer"))) + }) + + It("records a warning event and logs when bastion creation fails", func() { + recorder := events.NewFakeRecorder(10) + reconciler.Recorder = recorder + got := &infrav1.StackitCluster{} + Expect(k8sClient.Get(ctx, stackitKey, got)).To(Succeed()) + got.Spec.Bastion = validBastionSpec() + Expect(k8sClient.Update(ctx, got)).To(Succeed()) + + fakeCloud.FailNextEnsureBastion = fmt.Errorf("bastion creation rejected: %w", cloud.ErrInvalidInput) + + result, err := reconciler.Reconcile(ctx, request) + Expect(err).NotTo(HaveOccurred()) + Expect(result).To(Equal(reconcile.Result{})) + + Expect(k8sClient.Get(ctx, stackitKey, got)).To(Succeed()) + Expect(got.Status.Ready).To(BeFalse()) + expectCondition(got.Status.Conditions, infrav1.ClusterBastionReadyCondition, metav1.ConditionFalse, "BastionError") + + events := drainEvents(recorder) + Expect(events).To(ContainElement(ContainSubstring("FailedCreateBastion"))) }) It("requeues when network lookup returns a transient error", func() { @@ -518,6 +568,8 @@ var _ = Describe("StackitCluster Controller", func() { // The requeue is what lets a corrected Secret take effect: nothing else // enqueues the cluster once the credentials are rejected. It("requeues on unauthorized credentials and recovers once they are corrected", func() { + recorder := events.NewFakeRecorder(10) + reconciler.Recorder = recorder reconciler.CloudClientFactory = func(context.Context, cloud.Credentials) (cloud.Client, error) { return nil, fmt.Errorf("authenticate: %w", cloud.ErrUnauthorized) } @@ -532,6 +584,9 @@ var _ = Describe("StackitCluster Controller", func() { expectCondition(got.Status.Conditions, infrav1.ClusterCredentialsReadyCondition, metav1.ConditionFalse, "CredentialsInvalid") expectCondition(got.Status.Conditions, infrav1.ClusterReadyCondition, metav1.ConditionFalse, "CredentialsInvalid") + events := drainEvents(recorder) + Expect(events).To(ContainElement(ContainSubstring("InvalidCredentials"))) + By("correcting the credentials") reconciler.CloudClientFactory = func(context.Context, cloud.Credentials) (cloud.Client, error) { return fakeCloud, nil @@ -612,6 +667,28 @@ var _ = Describe("StackitCluster Controller", func() { }).Should(BeTrue()) }) + It("records a warning event and logs when API server load balancer deletion fails", func() { + _, err := reconciler.Reconcile(ctx, request) + Expect(err).NotTo(HaveOccurred()) + + got := &infrav1.StackitCluster{} + Expect(k8sClient.Get(ctx, stackitKey, got)).To(Succeed()) + Expect(got.Status.APIServerLoadBalancerID).NotTo(BeEmpty()) + + recorder := events.NewFakeRecorder(10) + reconciler.Recorder = recorder + + Expect(k8sClient.Delete(ctx, got)).To(Succeed()) + + fakeCloud.FailNextDeleteLB = fmt.Errorf("delete load balancer timeout: %w", cloud.ErrTransient) + + _, err = reconciler.Reconcile(ctx, request) + Expect(err).To(HaveOccurred()) + + events := drainEvents(recorder) + Expect(events).To(ContainElement(ContainSubstring("FailedDeleteLoadBalancer"))) + }) + // GetNetwork is the first cloud call of the reconcile, so a hook there proves // the ordering rather than only the end result. It("persists the finalizer before the first cloud call", func() { diff --git a/controller/stackitcluster_infrastructure.go b/controller/stackitcluster_infrastructure.go index 92c4587..a251e23 100644 --- a/controller/stackitcluster_infrastructure.go +++ b/controller/stackitcluster_infrastructure.go @@ -12,6 +12,7 @@ package controller import ( "context" + "errors" "fmt" "net/netip" "time" @@ -51,6 +52,10 @@ func (r *StackitClusterReconciler) reconcileNormal(ctx context.Context, clusterS cloudClient, err := util.BuildCloudClient(ctx, r.Client, r.CloudClientFactory, stackitCluster) if err != nil { + log.Error(err, "Failed to build cloud client for cluster", "cluster", stackitCluster.Name) + if r.Recorder != nil { + r.Recorder.Eventf(stackitCluster, nil, corev1.EventTypeWarning, "InvalidCredentials", "Reconcile", "Failed to build cloud client: %v", err) + } stackitCluster.Status.Ready = false return util.CredentialFailureResult( &stackitCluster.Status.Conditions, @@ -70,6 +75,17 @@ func (r *StackitClusterReconciler) reconcileNormal(ctx context.Context, clusterS network, err := cloudClient.GetNetwork(ctx, stackitCluster.Spec.Network.ID) if err != nil { + log.Error(err, "Failed to get network", + "networkID", stackitCluster.Spec.Network.ID, + "region", stackitCluster.Spec.Region, + ) + if r.Recorder != nil { + r.Recorder.Eventf( + stackitCluster, nil, corev1.EventTypeWarning, "FailedGetNetwork", "Reconcile", + "Failed to get network %s in region %s: %v", + stackitCluster.Spec.Network.ID, stackitCluster.Spec.Region, err, + ) + } stackitCluster.Status.Ready = false return util.CloudFailureResult( &stackitCluster.Status.Conditions, @@ -124,6 +140,16 @@ func (r *StackitClusterReconciler) reconcileNormal(ctx context.Context, clusterS loadbalancerservice.APIServerInput(stackitCluster, seed), ) if err != nil { + log.Error(err, "Failed to create API server load balancer", + "cluster", stackitCluster.Name, + "region", stackitCluster.Spec.Region, + ) + if r.Recorder != nil { + r.Recorder.Eventf( + stackitCluster, nil, corev1.EventTypeWarning, "FailedCreateLoadBalancer", "Create", + "Failed to create API server load balancer: %v", err, + ) + } stackitCluster.Status.Ready = false return util.CloudFailureResult( &stackitCluster.Status.Conditions, @@ -215,6 +241,10 @@ func (r *StackitClusterReconciler) reconcileNormal(ctx context.Context, clusterS infrav1.ClusterLoadBalancerReadyCondition, ) } else { + log.Error(errors.New("controlPlaneEndpoint host is empty"), "API server load balancer is disabled and controlPlaneEndpoint is empty", "cluster", stackitCluster.Name) + if r.Recorder != nil { + r.Recorder.Eventf(stackitCluster, nil, corev1.EventTypeWarning, "EndpointMissing", "Reconcile", "apiServerLoadBalancer.enabled is false and controlPlaneEndpoint is empty") + } clusterScope.SetNotReady( "EndpointMissing", "apiServerLoadBalancer.enabled is false and controlPlaneEndpoint is empty", @@ -225,6 +255,15 @@ func (r *StackitClusterReconciler) reconcileNormal(ctx context.Context, clusterS } if result, ready, err := r.reconcileBastion(ctx, cloudClient, clusterScope); err != nil { + log.Error(err, "Failed to reconcile bastion", + "cluster", stackitCluster.Name, + ) + if r.Recorder != nil { + r.Recorder.Eventf( + stackitCluster, nil, corev1.EventTypeWarning, "FailedCreateBastion", "Create", + "Failed to reconcile bastion: %v", err, + ) + } stackitCluster.Status.Ready = false return util.CloudFailureResult( &stackitCluster.Status.Conditions, @@ -293,16 +332,22 @@ func bootstrapTargetIP(network *cloud.Network) string { } func (r *StackitClusterReconciler) reconcileDelete(ctx context.Context, clusterScope *scope.ClusterScope) (ctrl.Result, error) { + log := logf.FromContext(ctx) stackitCluster := clusterScope.StackitCluster // Machines resolve their credentials and project context through this // StackitCluster, so the finalizer has to stay while any of them remain. machines, err := collections.GetFilteredMachinesForCluster(ctx, r.Client, clusterScope.Cluster) if err != nil { + log.Error(err, "Failed to list Machines for cluster during deletion", "cluster", clusterScope.Cluster.Name) + if r.Recorder != nil { + r.Recorder.Eventf(stackitCluster, nil, corev1.EventTypeWarning, "FailedListMachines", "Delete", + "Failed to list Machines for cluster %s: %v", clusterScope.Cluster.Name, err) + } return ctrl.Result{}, fmt.Errorf("list Machines for cluster %s: %w", clusterScope.Cluster.Name, err) } if machines.Len() > 0 { - logf.FromContext(ctx).Info( + log.Info( "Waiting for Machines to be deleted before removing the StackitCluster finalizer", "remainingMachines", machines.Len(), ) @@ -319,6 +364,7 @@ func (r *StackitClusterReconciler) reconcileDelete(ctx context.Context, clusterS // possible leak loud rather than stranding the cluster in Terminating. // Every other credentials problem is fixable and keeps retrying. if apierrors.IsNotFound(err) { + log.Error(err, "Credentials secret is gone; finalizing without cloud cleanup", "cluster", stackitCluster.Name) if r.Recorder != nil { r.Recorder.Eventf(stackitCluster, nil, corev1.EventTypeWarning, "CleanupSkipped", "Delete", "Credentials Secret is gone; finalizing without cloud cleanup. "+ @@ -327,6 +373,11 @@ func (r *StackitClusterReconciler) reconcileDelete(ctx context.Context, clusterS controllerutil.RemoveFinalizer(stackitCluster, infrav1.ClusterFinalizer) return ctrl.Result{}, nil } + log.Error(err, "Failed to build cloud client for cluster deletion", "cluster", stackitCluster.Name) + if r.Recorder != nil { + r.Recorder.Eventf(stackitCluster, nil, corev1.EventTypeWarning, "InvalidCredentials", "Delete", + "Failed to build cloud client for deletion: %v", err) + } util.SetConditions( &stackitCluster.Status.Conditions, stackitCluster.Generation, @@ -339,10 +390,20 @@ func (r *StackitClusterReconciler) reconcileDelete(ctx context.Context, clusterS } loadBalancerID, err := loadbalancerservice.ResolveID(ctx, cloudClient, stackitCluster) if err != nil { + log.Error(err, "Failed to resolve API server load balancer for deletion", "cluster", stackitCluster.Name) + if r.Recorder != nil { + r.Recorder.Eventf(stackitCluster, nil, corev1.EventTypeWarning, "FailedResolveLoadBalancer", "Delete", + "Failed to resolve API server load balancer: %v", err) + } return ctrl.Result{}, err } if loadBalancerID != "" { if err := cloudClient.DeleteAPIServerLoadBalancer(ctx, loadBalancerID); err != nil && !cloud.IsNotFound(err) { + log.Error(err, "Failed to delete API server load balancer", "cluster", stackitCluster.Name, "loadBalancerID", loadBalancerID) + if r.Recorder != nil { + r.Recorder.Eventf(stackitCluster, nil, corev1.EventTypeWarning, "FailedDeleteLoadBalancer", "Delete", + "Failed to delete API server load balancer %s: %v", loadBalancerID, err) + } return ctrl.Result{}, err } stackitCluster.Status.APIServerLoadBalancerID = "" @@ -354,6 +415,11 @@ func (r *StackitClusterReconciler) reconcileDelete(ctx context.Context, clusterS } } if err := cloudClient.DeleteNodeSSHAccess(ctx, bastionservice.NodeSSHAccessTags(stackitCluster)); err != nil && !cloud.IsNotFound(err) { + log.Error(err, "Failed to delete bastion node SSH access", "cluster", stackitCluster.Name) + if r.Recorder != nil { + r.Recorder.Eventf(stackitCluster, nil, corev1.EventTypeWarning, "FailedDeleteNodeSSHAccess", "Delete", + "Failed to delete bastion node SSH access: %v", err) + } return ctrl.Result{}, err } if err := cloudClient.DeleteBastion(ctx, bastionservice.Input(stackitCluster, nil), cloud.Bastion{ @@ -362,6 +428,11 @@ func (r *StackitClusterReconciler) reconcileDelete(ctx context.Context, clusterS PublicIP: stackitCluster.Status.Bastion.PublicIP, SecurityGroupID: stackitCluster.Status.Bastion.SecurityGroupID, }); err != nil && !cloud.IsNotFound(err) { + log.Error(err, "Failed to delete bastion", "cluster", stackitCluster.Name) + if r.Recorder != nil { + r.Recorder.Eventf(stackitCluster, nil, corev1.EventTypeWarning, "FailedDeleteBastion", "Delete", + "Failed to delete bastion: %v", err) + } return ctrl.Result{}, err } if hasBastionStatus(stackitCluster.Status.Bastion) { diff --git a/controller/stackitmachine_controller_test.go b/controller/stackitmachine_controller_test.go index 2589cdb..6de4f44 100644 --- a/controller/stackitmachine_controller_test.go +++ b/controller/stackitmachine_controller_test.go @@ -255,7 +255,34 @@ var _ = Describe("StackitMachine Controller", func() { Expect(fakeCloud.ServerHasSecurityGroup(got.Status.InstanceID, securityGroups[0].ID)).To(BeTrue()) }) + It("records a warning event and logs when bastion node SSH access fails", func() { + recorder := events.NewFakeRecorder(10) + reconciler.Recorder = recorder + stackitCluster := &infrav1.StackitCluster{} + Expect(k8sClient.Get(ctx, types.NamespacedName{Name: clusterName, Namespace: namespace}, stackitCluster)).To(Succeed()) + stackitCluster.Spec.Bastion = validBastionSpec() + Expect(k8sClient.Update(ctx, stackitCluster)).To(Succeed()) + stackitCluster.Status.Ready = true + stackitCluster.Status.Bastion.SecurityGroupID = "bastion-sg" + Expect(k8sClient.Status().Update(ctx, stackitCluster)).To(Succeed()) + + fakeCloud.FailNextEnsureNodeSSH = fmt.Errorf("node ssh access timeout: %w", cloud.ErrTransient) + + result, err := reconciler.Reconcile(ctx, request) + Expect(err).NotTo(HaveOccurred()) + Expect(result.RequeueAfter).To(BeNumerically(">", 0)) + + got := &infrav1.StackitMachine{} + Expect(k8sClient.Get(ctx, stackitKey, got)).To(Succeed()) + expectCondition(got.Status.Conditions, infrav1.MachineReadyCondition, metav1.ConditionFalse, "BastionSSHAccessError") + + events := drainEvents(recorder) + Expect(events).To(ContainElement(ContainSubstring("FailedNodeSSHAccess"))) + }) + It("does not create a VM when availabilityZone is outside published failure domains", func() { + recorder := events.NewFakeRecorder(10) + reconciler.Recorder = recorder got := &infrav1.StackitMachine{} Expect(k8sClient.Get(ctx, stackitKey, got)).To(Succeed()) got.Spec.AvailabilityZone = "eu01-9" @@ -270,9 +297,14 @@ var _ = Describe("StackitMachine Controller", func() { Expect(got.Status.Ready).To(BeFalse()) expectCondition(got.Status.Conditions, infrav1.MachineInstanceReadyCondition, metav1.ConditionFalse, "InvalidFailureDomain") expectCondition(got.Status.Conditions, infrav1.MachineReadyCondition, metav1.ConditionFalse, "InvalidFailureDomain") + + events := drainEvents(recorder) + Expect(events).To(ContainElement(ContainSubstring("InvalidFailureDomain"))) }) It("requeues on unauthorized credentials and recovers once they are corrected", func() { + recorder := events.NewFakeRecorder(10) + reconciler.Recorder = recorder reconciler.CloudClientFactory = func(context.Context, cloud.Credentials) (cloud.Client, error) { return nil, fmt.Errorf("authenticate: %w", cloud.ErrUnauthorized) } @@ -287,6 +319,9 @@ var _ = Describe("StackitMachine Controller", func() { expectCondition(got.Status.Conditions, infrav1.MachineCredentialsReadyCondition, metav1.ConditionFalse, "CredentialsInvalid") expectCondition(got.Status.Conditions, infrav1.MachineReadyCondition, metav1.ConditionFalse, "CredentialsInvalid") + events := drainEvents(recorder) + Expect(events).To(ContainElement(ContainSubstring("InvalidCredentials"))) + By("correcting the credentials") reconciler.CloudClientFactory = func(context.Context, cloud.Credentials) (cloud.Client, error) { return fakeCloud, nil @@ -375,6 +410,8 @@ var _ = Describe("StackitMachine Controller", func() { }) It("returns an error when VM creation is rejected", func() { + recorder := events.NewFakeRecorder(10) + reconciler.Recorder = recorder fakeCloud.FailNextCreateServer = fmt.Errorf("create server rejected: %w", cloud.ErrInvalidInput) result, err := reconciler.Reconcile(ctx, request) @@ -386,6 +423,9 @@ var _ = Describe("StackitMachine Controller", func() { Expect(k8sClient.Get(ctx, stackitKey, got)).To(Succeed()) expectCondition(got.Status.Conditions, infrav1.MachineInstanceReadyCondition, metav1.ConditionFalse, "InstanceError") expectCondition(got.Status.Conditions, infrav1.MachineReadyCondition, metav1.ConditionFalse, "InstanceError") + + events := drainEvents(recorder) + Expect(events).To(ContainElement(ContainSubstring("FailedCreateInstance"))) }) It("deletes the VM and removes the finalizer", func() { @@ -406,6 +446,27 @@ var _ = Describe("StackitMachine Controller", func() { }).Should(BeTrue()) }) + It("records a warning event and logs when server deletion fails", func() { + _, err := reconciler.Reconcile(ctx, request) + Expect(err).NotTo(HaveOccurred()) + Expect(fakeCloud.ServerCount()).To(Equal(1)) + + recorder := events.NewFakeRecorder(10) + reconciler.Recorder = recorder + + got := &infrav1.StackitMachine{} + Expect(k8sClient.Get(ctx, stackitKey, got)).To(Succeed()) + Expect(k8sClient.Delete(ctx, got)).To(Succeed()) + + fakeCloud.FailNextDeleteServer = fmt.Errorf("delete server timeout: %w", cloud.ErrTransient) + + _, err = reconciler.Reconcile(ctx, request) + Expect(err).To(HaveOccurred()) + + events := drainEvents(recorder) + Expect(events).To(ContainElement(ContainSubstring("FailedDeleteInstance"))) + }) + // The hook observes API server state from inside the cloud call, so it proves // the ordering rather than only the end result. It("persists the finalizer before creating the server", func() { diff --git a/controller/stackitmachine_infrastructure.go b/controller/stackitmachine_infrastructure.go index 52ffd24..a055528 100644 --- a/controller/stackitmachine_infrastructure.go +++ b/controller/stackitmachine_infrastructure.go @@ -49,10 +49,15 @@ func (r *StackitMachineReconciler) reconcileNormal(ctx context.Context, machineS } if !machineScope.StackitCluster.Status.Ready { + log.Info("Waiting for StackitCluster to be ready", "machine", stackitMachine.Name, "cluster", machineScope.StackitCluster.Name) machineScope.SetNotReady("InfrastructureNotReady", "waiting for StackitCluster to be ready", infrav1.MachineReadyCondition) return ctrl.Result{}, nil } if err := validateMachineAvailabilityZone(machineScope); err != nil { + log.Error(err, "Invalid availability zone for machine", "machine", stackitMachine.Name, "availabilityZone", stackitMachine.Spec.AvailabilityZone) + if r.Recorder != nil { + r.Recorder.Eventf(stackitMachine, nil, corev1.EventTypeWarning, "InvalidFailureDomain", "Reconcile", "%v", err) + } machineScope.SetNotReady( "InvalidFailureDomain", err.Error(), @@ -65,6 +70,10 @@ func (r *StackitMachineReconciler) reconcileNormal(ctx context.Context, machineS bootstrapData, conditionStatus, reason, message := r.fetchBootstrapData(ctx, machineScope.Machine) machineScope.SetConditions(conditionStatus, reason, message, infrav1.MachineBootstrapReadyCondition) if conditionStatus != metav1.ConditionTrue { + log.Error(errors.New(message), "Bootstrap data is not ready", "machine", stackitMachine.Name, "reason", reason) + if r.Recorder != nil { + r.Recorder.Eventf(stackitMachine, nil, corev1.EventTypeWarning, reason, "Reconcile", "Bootstrap data error: %s", message) + } machineScope.SetConditions(metav1.ConditionFalse, reason, message, infrav1.MachineReadyCondition) if reason == util.BootstrapReasonInvalid { return ctrl.Result{}, nil @@ -74,6 +83,10 @@ func (r *StackitMachineReconciler) reconcileNormal(ctx context.Context, machineS cloudClient, err := util.BuildCloudClient(ctx, r.Client, r.CloudClientFactory, machineScope.StackitCluster) if err != nil { + log.Error(err, "Failed to build cloud client for machine", "machine", stackitMachine.Name) + if r.Recorder != nil { + r.Recorder.Eventf(stackitMachine, nil, corev1.EventTypeWarning, "InvalidCredentials", "Reconcile", "Failed to build cloud client: %v", err) + } return util.CredentialFailureResult( &stackitMachine.Status.Conditions, stackitMachine.Generation, @@ -102,6 +115,20 @@ func (r *StackitMachineReconciler) reconcileNormal(ctx context.Context, machineS return ctrl.Result{}, nil } if err != nil { + log.Error(err, "Failed to create server", + "machine", stackitMachine.Name, + "imageID", stackitMachine.Spec.ImageID, + "networkID", stackitMachine.Spec.Network.ID, + "region", machineScope.StackitCluster.Spec.Region, + ) + if r.Recorder != nil { + r.Recorder.Eventf( + stackitMachine, nil, corev1.EventTypeWarning, "FailedCreateInstance", "Create", + "Failed to create server %s (imageID: %s, networkID: %s, region: %s): %v", + stackitMachine.Name, stackitMachine.Spec.ImageID, stackitMachine.Spec.Network.ID, + machineScope.StackitCluster.Spec.Region, err, + ) + } machineScope.SetNotReady( "InstanceError", err.Error(), @@ -143,6 +170,13 @@ func (r *StackitMachineReconciler) reconcileNormal(ctx context.Context, machineS } if err := r.reconcileBastionNodeSSHAccess(ctx, cloudClient, machineScope, server); err != nil { + log.Error(err, "Failed to configure bastion SSH access for node", "machine", stackitMachine.Name) + if r.Recorder != nil { + r.Recorder.Eventf( + stackitMachine, nil, corev1.EventTypeWarning, "FailedNodeSSHAccess", "Reconcile", + "Failed to configure bastion SSH access for server %s: %v", server.ID, err, + ) + } machineScope.SetNotReady("BastionSSHAccessError", err.Error(), infrav1.MachineReadyCondition) return util.CloudFailureResult( &stackitMachine.Status.Conditions, @@ -174,6 +208,7 @@ func validateMachineAvailabilityZone(machineScope *scope.MachineScope) error { } func (r *StackitMachineReconciler) reconcileDelete(ctx context.Context, machineScope *scope.MachineScope) error { + log := logf.FromContext(ctx) stackitMachine := machineScope.StackitMachine // An empty status.instanceID is not proof that no server exists, so every @@ -185,6 +220,7 @@ func (r *StackitMachineReconciler) reconcileDelete(ctx context.Context, machineS // possible leak loud rather than stranding the Machine in Terminating. // Every other credentials problem is fixable and keeps retrying. if apierrors.IsNotFound(err) { + log.Error(err, "Credentials secret is gone; finalizing without cloud cleanup", "machine", stackitMachine.Name) if r.Recorder != nil { r.Recorder.Eventf(stackitMachine, nil, corev1.EventTypeWarning, "CleanupSkipped", "Delete", "Credentials Secret is gone; finalizing without cloud cleanup. "+ @@ -193,6 +229,11 @@ func (r *StackitMachineReconciler) reconcileDelete(ctx context.Context, machineS controllerutil.RemoveFinalizer(stackitMachine, infrav1.MachineFinalizer) return nil } + log.Error(err, "Failed to build cloud client for machine deletion", "machine", stackitMachine.Name) + if r.Recorder != nil { + r.Recorder.Eventf(stackitMachine, nil, corev1.EventTypeWarning, "InvalidCredentials", "Delete", + "Failed to build cloud client for deletion: %v", err) + } _, resultErr := util.CredentialFailureResult( &stackitMachine.Status.Conditions, stackitMachine.Generation, @@ -202,12 +243,23 @@ func (r *StackitMachineReconciler) reconcileDelete(ctx context.Context, machineS ) return resultErr } + instanceID, err := r.resolveServerForDeletion(ctx, cloudClient, machineScope) if err != nil { + log.Error(err, "Failed to resolve server for deletion", "machine", stackitMachine.Name) + if r.Recorder != nil { + r.Recorder.Eventf(stackitMachine, nil, corev1.EventTypeWarning, "FailedResolveInstance", "Delete", + "Failed to resolve server for deletion: %v", err) + } return err } if instanceID != "" { if err := cloudClient.DeleteServer(ctx, instanceID); err != nil && !cloud.IsNotFound(err) { + log.Error(err, "Failed to delete server", "machine", stackitMachine.Name, "instanceID", instanceID) + if r.Recorder != nil { + r.Recorder.Eventf(stackitMachine, nil, corev1.EventTypeWarning, "FailedDeleteInstance", "Delete", + "Failed to delete server %s: %v", instanceID, err) + } return err } machineScope.ClearInstance() From f8470679680bfb6f0f556d9ba778a3e4d5be12f9 Mon Sep 17 00:00:00 2001 From: Jan Larwig Date: Tue, 13 Oct 2026 12:43:54 +0200 Subject: [PATCH 2/2] refactor: use capi provided record util instead of custom recorder init Signed-off-by: Jan Larwig --- cmd/manager/main.go | 5 +- controller/controller_test_helpers_test.go | 14 --- controller/stackitcluster_bastion.go | 33 ++--- controller/stackitcluster_controller.go | 2 - controller/stackitcluster_controller_test.go | 69 +---------- controller/stackitcluster_infrastructure.go | 122 +++++++------------ controller/stackitmachine_controller.go | 2 - controller/stackitmachine_controller_test.go | 45 +------ controller/stackitmachine_infrastructure.go | 91 ++++++-------- 9 files changed, 107 insertions(+), 276 deletions(-) diff --git a/cmd/manager/main.go b/cmd/manager/main.go index ca4905e..1992b29 100644 --- a/cmd/manager/main.go +++ b/cmd/manager/main.go @@ -29,6 +29,7 @@ import ( utilruntime "k8s.io/apimachinery/pkg/util/runtime" clientgoscheme "k8s.io/client-go/kubernetes/scheme" clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta2" + "sigs.k8s.io/cluster-api/util/record" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/healthz" "sigs.k8s.io/controller-runtime/pkg/log/zap" @@ -182,11 +183,12 @@ func main() { os.Exit(1) } + record.InitFromRecorder(mgr.GetEventRecorderFor("cluster-api-provider-stackit")) + if err := (&controller.StackitClusterReconciler{ Client: mgr.GetClient(), Scheme: mgr.GetScheme(), CloudClientFactory: cloud.NewClient, - Recorder: mgr.GetEventRecorder("stackitcluster-controller"), }).SetupWithManager(mgr); err != nil { setupLog.Error(err, "Failed to create controller", "controller", "stackitcluster") os.Exit(1) @@ -195,7 +197,6 @@ func main() { Client: mgr.GetClient(), Scheme: mgr.GetScheme(), CloudClientFactory: cloud.NewClient, - Recorder: mgr.GetEventRecorder("stackitmachine-controller"), }).SetupWithManager(mgr); err != nil { setupLog.Error(err, "Failed to create controller", "controller", "stackitmachine") os.Exit(1) diff --git a/controller/controller_test_helpers_test.go b/controller/controller_test_helpers_test.go index dad7b4f..b8a77f9 100644 --- a/controller/controller_test_helpers_test.go +++ b/controller/controller_test_helpers_test.go @@ -18,7 +18,6 @@ import ( corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/client-go/tools/events" "sigs.k8s.io/controller-runtime/pkg/client" clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta2" @@ -204,16 +203,3 @@ func deleteIfExists(ctx context.Context, obj client.Object) { Expect(apierrors.IsNotFound(err)).To(BeTrue()) } } - -// drainEvents returns all events recorded so far. -func drainEvents(recorder *events.FakeRecorder) []string { - var out []string - for { - select { - case event := <-recorder.Events: - out = append(out, event) - default: - return out - } - } -} diff --git a/controller/stackitcluster_bastion.go b/controller/stackitcluster_bastion.go index 73dae55..e8c3d8b 100644 --- a/controller/stackitcluster_bastion.go +++ b/controller/stackitcluster_bastion.go @@ -22,6 +22,7 @@ import ( "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/cluster-api/util/record" ctrl "sigs.k8s.io/controller-runtime" logf "sigs.k8s.io/controller-runtime/pkg/log" @@ -63,9 +64,7 @@ func (r *StackitClusterReconciler) reconcileBastion( return ctrl.Result{}, false, err } clusterScope.ClearBastionStatus() - if r.Recorder != nil { - r.Recorder.Eventf(stackitCluster, nil, corev1.EventTypeNormal, "BastionDeleted", "Delete", "Deleted bastion") - } + record.Event(stackitCluster, "BastionDeleted", "Deleted bastion") } clusterScope.SetConditions( metav1.ConditionTrue, @@ -78,9 +77,7 @@ func (r *StackitClusterReconciler) reconcileBastion( if err := validateBastionSpec(stackitCluster.Spec.Bastion); err != nil { log.Error(err, "Invalid bastion spec", "cluster", stackitCluster.Name) - if r.Recorder != nil { - r.Recorder.Eventf(stackitCluster, nil, corev1.EventTypeWarning, "InvalidBastionSpec", "Reconcile", "Invalid bastion spec: %v", err) - } + record.Warnf(stackitCluster, "InvalidBastionSpec", "Invalid bastion spec: %v", err) clusterScope.SetNotReady( "InvalidBastionSpec", err.Error(), @@ -93,9 +90,7 @@ func (r *StackitClusterReconciler) reconcileBastion( cloudInit, err := r.resolveBastionCloudInit(ctx, stackitCluster) if err != nil { log.Error(err, "Failed to resolve bastion cloud-init", "cluster", stackitCluster.Name) - if r.Recorder != nil { - r.Recorder.Eventf(stackitCluster, nil, corev1.EventTypeWarning, "CloudInitRefError", "Reconcile", "Failed to resolve bastion cloudInitRef: %v", err) - } + record.Warnf(stackitCluster, "CloudInitRefError", "Failed to resolve bastion cloudInitRef: %v", err) clusterScope.SetNotReady( "CloudInitRefError", err.Error(), @@ -120,12 +115,10 @@ func (r *StackitClusterReconciler) reconcileBastion( infrav1.ClusterBastionReadyCondition, infrav1.ClusterReadyCondition, ) - if r.Recorder != nil { - r.Recorder.Eventf( - stackitCluster, nil, corev1.EventTypeNormal, "BastionRecreating", "Recreate", - "Recreating bastion because cloudInitRef content changed", - ) - } + record.Event( + stackitCluster, "BastionRecreating", + "Recreating bastion because cloudInitRef content changed", + ) return ctrl.Result{RequeueAfter: retryableErrorRequeueAfter}, false, nil } @@ -135,9 +128,9 @@ func (r *StackitClusterReconciler) reconcileBastion( return ctrl.Result{}, false, err } clusterScope.SetBastionStatus(bastion, bastionCloudInitHash(cloudInit)) - if !hadBastionStatus && r.Recorder != nil { - r.Recorder.Eventf( - stackitCluster, nil, corev1.EventTypeNormal, "BastionCreated", "Create", "Created bastion %s", bastion.ServerID, + if !hadBastionStatus { + record.Eventf( + stackitCluster, "BastionCreated", "Created bastion %s", bastion.ServerID, ) } if ready, reason, message, warn := serverStateCondition(bastion.ServerState, bastion.ServerPowerStatus); !ready { @@ -147,8 +140,8 @@ func (r *StackitClusterReconciler) reconcileBastion( } clusterScope.SetNotReady(reason, "bastion "+message, infrav1.ClusterBastionReadyCondition, infrav1.ClusterReadyCondition) // Only warn on entering the state, not on every requeue. - if warn && r.Recorder != nil && previousReason != reason { - r.Recorder.Eventf(stackitCluster, nil, corev1.EventTypeWarning, reason, "Reconcile", "Bastion server %s: %s", bastion.ServerID, message) + if warn && previousReason != reason { + record.Warnf(stackitCluster, reason, "Bastion server %s: %s", bastion.ServerID, message) } return ctrl.Result{RequeueAfter: 15 * time.Second}, false, nil } diff --git a/controller/stackitcluster_controller.go b/controller/stackitcluster_controller.go index b810616..a9e4a3c 100644 --- a/controller/stackitcluster_controller.go +++ b/controller/stackitcluster_controller.go @@ -27,7 +27,6 @@ import ( apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" - "k8s.io/client-go/tools/events" clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta2" clusterutil "sigs.k8s.io/cluster-api/util" ctrl "sigs.k8s.io/controller-runtime" @@ -53,7 +52,6 @@ type StackitClusterReconciler struct { // CloudClientFactory builds a cloud.Client from parsed credentials. It is // injected so tests can swap in the in-memory fake. CloudClientFactory cloud.Factory - Recorder events.EventRecorder } // +kubebuilder:rbac:groups=infrastructure.cluster.x-k8s.io,resources=stackitclusters,verbs=get;list;watch;create;update;patch;delete diff --git a/controller/stackitcluster_controller_test.go b/controller/stackitcluster_controller_test.go index 4ae794a..67a42f8 100644 --- a/controller/stackitcluster_controller_test.go +++ b/controller/stackitcluster_controller_test.go @@ -22,7 +22,6 @@ import ( "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" - "k8s.io/client-go/tools/events" clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta2" "sigs.k8s.io/controller-runtime/pkg/reconcile" @@ -140,7 +139,7 @@ var _ = Describe("StackitCluster Controller", func() { expectCondition(got.Status.Conditions, infrav1.ClusterBastionReadyCondition, metav1.ConditionTrue, "Available") }) - It("reports a failed bastion server with its own reason and warns", func() { + It("reports a failed bastion server with its own reason", func() { got := &infrav1.StackitCluster{} Expect(k8sClient.Get(ctx, stackitKey, got)).To(Succeed()) got.Spec.Bastion = validBastionSpec() @@ -150,8 +149,6 @@ var _ = Describe("StackitCluster Controller", func() { Expect(k8sClient.Get(ctx, stackitKey, got)).To(Succeed()) fakeCloud.SetServerState(got.Status.Bastion.ServerID, "ERROR", "") - recorder := events.NewFakeRecorder(10) - reconciler.Recorder = recorder result, err := reconciler.Reconcile(ctx, request) Expect(err).NotTo(HaveOccurred()) @@ -161,7 +158,6 @@ var _ = Describe("StackitCluster Controller", func() { expectCondition(got.Status.Conditions, infrav1.ClusterBastionReadyCondition, metav1.ConditionFalse, "InstanceFailed") condition := meta.FindStatusCondition(got.Status.Conditions, infrav1.ClusterBastionReadyCondition) Expect(condition.Message).To(Equal("bastion server failed (state ERROR)")) - Expect(drainEvents(recorder)).To(ContainElement(HavePrefix("Warning InstanceFailed"))) }) It("deletes existing bastion resources when bastion is disabled", func() { @@ -414,8 +410,6 @@ var _ = Describe("StackitCluster Controller", func() { }) It("marks the bastion not ready when cloud-init ref is missing", func() { - recorder := events.NewFakeRecorder(10) - reconciler.Recorder = recorder got := &infrav1.StackitCluster{} Expect(k8sClient.Get(ctx, stackitKey, got)).To(Succeed()) got.Spec.Bastion = validBastionSpec() @@ -434,9 +428,6 @@ var _ = Describe("StackitCluster Controller", func() { Expect(got.Status.Ready).To(BeFalse()) Expect(fakeCloud.ServerCount()).To(Equal(0)) expectCondition(got.Status.Conditions, infrav1.ClusterBastionReadyCondition, metav1.ConditionFalse, "CloudInitRefError") - - events := drainEvents(recorder) - Expect(events).To(ContainElement(ContainSubstring("CloudInitRefError"))) }) It("recreates the bastion and node SSH access when referenced cloud-init changes", func() { @@ -492,8 +483,6 @@ var _ = Describe("StackitCluster Controller", func() { }) It("marks the network not ready when the configured network does not exist", func() { - recorder := events.NewFakeRecorder(10) - reconciler.Recorder = recorder got := &infrav1.StackitCluster{} Expect(k8sClient.Get(ctx, stackitKey, got)).To(Succeed()) got.Spec.Network.ID = "99999999-9999-9999-9999-999999999999" @@ -507,14 +496,9 @@ var _ = Describe("StackitCluster Controller", func() { Expect(got.Status.Ready).To(BeFalse()) expectCondition(got.Status.Conditions, infrav1.ClusterNetworkReadyCondition, metav1.ConditionFalse, "NetworkNotFound") expectCondition(got.Status.Conditions, infrav1.ClusterReadyCondition, metav1.ConditionFalse, "NetworkNotFound") - - events := drainEvents(recorder) - Expect(events).To(ContainElement(ContainSubstring("FailedGetNetwork"))) }) - It("records a warning event and logs when API server load balancer creation fails", func() { - recorder := events.NewFakeRecorder(10) - reconciler.Recorder = recorder + It("marks conditions not ready when API server load balancer creation fails", func() { fakeCloud.FailNextEnsureLB = fmt.Errorf("load balancer creation rejected: %w", cloud.ErrInvalidInput) result, err := reconciler.Reconcile(ctx, request) @@ -525,14 +509,9 @@ var _ = Describe("StackitCluster Controller", func() { Expect(k8sClient.Get(ctx, stackitKey, got)).To(Succeed()) Expect(got.Status.Ready).To(BeFalse()) expectCondition(got.Status.Conditions, infrav1.ClusterLoadBalancerReadyCondition, metav1.ConditionFalse, "LoadBalancerError") - - events := drainEvents(recorder) - Expect(events).To(ContainElement(ContainSubstring("FailedCreateLoadBalancer"))) }) - It("records a warning event and logs when bastion creation fails", func() { - recorder := events.NewFakeRecorder(10) - reconciler.Recorder = recorder + It("marks conditions not ready when bastion creation fails", func() { got := &infrav1.StackitCluster{} Expect(k8sClient.Get(ctx, stackitKey, got)).To(Succeed()) got.Spec.Bastion = validBastionSpec() @@ -547,9 +526,6 @@ var _ = Describe("StackitCluster Controller", func() { Expect(k8sClient.Get(ctx, stackitKey, got)).To(Succeed()) Expect(got.Status.Ready).To(BeFalse()) expectCondition(got.Status.Conditions, infrav1.ClusterBastionReadyCondition, metav1.ConditionFalse, "BastionError") - - events := drainEvents(recorder) - Expect(events).To(ContainElement(ContainSubstring("FailedCreateBastion"))) }) It("requeues when network lookup returns a transient error", func() { @@ -568,8 +544,6 @@ var _ = Describe("StackitCluster Controller", func() { // The requeue is what lets a corrected Secret take effect: nothing else // enqueues the cluster once the credentials are rejected. It("requeues on unauthorized credentials and recovers once they are corrected", func() { - recorder := events.NewFakeRecorder(10) - reconciler.Recorder = recorder reconciler.CloudClientFactory = func(context.Context, cloud.Credentials) (cloud.Client, error) { return nil, fmt.Errorf("authenticate: %w", cloud.ErrUnauthorized) } @@ -584,9 +558,6 @@ var _ = Describe("StackitCluster Controller", func() { expectCondition(got.Status.Conditions, infrav1.ClusterCredentialsReadyCondition, metav1.ConditionFalse, "CredentialsInvalid") expectCondition(got.Status.Conditions, infrav1.ClusterReadyCondition, metav1.ConditionFalse, "CredentialsInvalid") - events := drainEvents(recorder) - Expect(events).To(ContainElement(ContainSubstring("InvalidCredentials"))) - By("correcting the credentials") reconciler.CloudClientFactory = func(context.Context, cloud.Credentials) (cloud.Client, error) { return fakeCloud, nil @@ -667,7 +638,7 @@ var _ = Describe("StackitCluster Controller", func() { }).Should(BeTrue()) }) - It("records a warning event and logs when API server load balancer deletion fails", func() { + It("returns an error when API server load balancer deletion fails", func() { _, err := reconciler.Reconcile(ctx, request) Expect(err).NotTo(HaveOccurred()) @@ -675,18 +646,12 @@ var _ = Describe("StackitCluster Controller", func() { Expect(k8sClient.Get(ctx, stackitKey, got)).To(Succeed()) Expect(got.Status.APIServerLoadBalancerID).NotTo(BeEmpty()) - recorder := events.NewFakeRecorder(10) - reconciler.Recorder = recorder - Expect(k8sClient.Delete(ctx, got)).To(Succeed()) fakeCloud.FailNextDeleteLB = fmt.Errorf("delete load balancer timeout: %w", cloud.ErrTransient) _, err = reconciler.Reconcile(ctx, request) Expect(err).To(HaveOccurred()) - - events := drainEvents(recorder) - Expect(events).To(ContainElement(ContainSubstring("FailedDeleteLoadBalancer"))) }) // GetNetwork is the first cloud call of the reconcile, so a hook there proves @@ -800,32 +765,6 @@ var _ = Describe("StackitCluster Controller", func() { expectCondition(got.Status.Conditions, infrav1.ClusterLoadBalancerReadyCondition, metav1.ConditionTrue, "Available") }) - It("only publishes the LoadBalancerReady event when the load balancer becomes available or its IP changes", func() { - recorder := events.NewFakeRecorder(10) - reconciler.Recorder = recorder - - _, err := reconciler.Reconcile(ctx, request) - Expect(err).NotTo(HaveOccurred()) - - Expect(drainEvents(recorder)).To(ContainElement(HavePrefix("Normal LoadBalancerReady"))) - - // A subsequent reconcile with target changes should not republish LoadBalancerReady. - createControlPlaneMachine(ctx, "cp-event-test-"+clusterName, clusterName, "10.0.0.99") - _, err = reconciler.Reconcile(ctx, request) - Expect(err).NotTo(HaveOccurred()) - - Expect(drainEvents(recorder)).NotTo(ContainElement(HavePrefix("Normal LoadBalancerReady"))) - - // If the load balancer IP changes, LoadBalancerReady should be published again. - got := &infrav1.StackitCluster{} - Expect(k8sClient.Get(ctx, stackitKey, got)).To(Succeed()) - fakeCloud.SetLoadBalancerIP(got.Status.APIServerLoadBalancerID, "203.0.113.99") - _, err = reconciler.Reconcile(ctx, request) - Expect(err).NotTo(HaveOccurred()) - - Expect(drainEvents(recorder)).To(ContainElement(Equal("Normal LoadBalancerReady API server load balancer is ready at 203.0.113.99"))) - }) - It("keeps the bootstrap placeholder while no control plane machine has an address", func() { // STACKIT rejects an empty target pool. createControlPlaneMachine(ctx, "cp-0-"+clusterName, clusterName, "") diff --git a/controller/stackitcluster_infrastructure.go b/controller/stackitcluster_infrastructure.go index a251e23..4e2eab2 100644 --- a/controller/stackitcluster_infrastructure.go +++ b/controller/stackitcluster_infrastructure.go @@ -17,12 +17,12 @@ import ( "net/netip" "time" - corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta2" "sigs.k8s.io/cluster-api/util/collections" + "sigs.k8s.io/cluster-api/util/record" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" logf "sigs.k8s.io/controller-runtime/pkg/log" @@ -53,9 +53,7 @@ func (r *StackitClusterReconciler) reconcileNormal(ctx context.Context, clusterS cloudClient, err := util.BuildCloudClient(ctx, r.Client, r.CloudClientFactory, stackitCluster) if err != nil { log.Error(err, "Failed to build cloud client for cluster", "cluster", stackitCluster.Name) - if r.Recorder != nil { - r.Recorder.Eventf(stackitCluster, nil, corev1.EventTypeWarning, "InvalidCredentials", "Reconcile", "Failed to build cloud client: %v", err) - } + record.Warnf(stackitCluster, "InvalidCredentials", "Failed to build cloud client: %v", err) stackitCluster.Status.Ready = false return util.CredentialFailureResult( &stackitCluster.Status.Conditions, @@ -79,13 +77,11 @@ func (r *StackitClusterReconciler) reconcileNormal(ctx context.Context, clusterS "networkID", stackitCluster.Spec.Network.ID, "region", stackitCluster.Spec.Region, ) - if r.Recorder != nil { - r.Recorder.Eventf( - stackitCluster, nil, corev1.EventTypeWarning, "FailedGetNetwork", "Reconcile", - "Failed to get network %s in region %s: %v", - stackitCluster.Spec.Network.ID, stackitCluster.Spec.Region, err, - ) - } + record.Warnf( + stackitCluster, "FailedGetNetwork", + "Failed to get network %s in region %s: %v", + stackitCluster.Spec.Network.ID, stackitCluster.Spec.Region, err, + ) stackitCluster.Status.Ready = false return util.CloudFailureResult( &stackitCluster.Status.Conditions, @@ -144,12 +140,10 @@ func (r *StackitClusterReconciler) reconcileNormal(ctx context.Context, clusterS "cluster", stackitCluster.Name, "region", stackitCluster.Spec.Region, ) - if r.Recorder != nil { - r.Recorder.Eventf( - stackitCluster, nil, corev1.EventTypeWarning, "FailedCreateLoadBalancer", "Create", - "Failed to create API server load balancer: %v", err, - ) - } + record.Warnf( + stackitCluster, "FailedCreateLoadBalancer", + "Failed to create API server load balancer: %v", err, + ) stackitCluster.Status.Ready = false return util.CloudFailureResult( &stackitCluster.Status.Conditions, @@ -165,9 +159,9 @@ func (r *StackitClusterReconciler) reconcileNormal(ctx context.Context, clusterS hadLoadBalancerID := stackitCluster.Status.APIServerLoadBalancerID != "" if loadBalancer != nil { stackitCluster.Status.APIServerLoadBalancerID = loadBalancer.ID - if !hadLoadBalancerID && loadBalancer.ID != "" && r.Recorder != nil { - r.Recorder.Eventf( - stackitCluster, nil, corev1.EventTypeNormal, "LoadBalancerCreated", "Create", + if !hadLoadBalancerID && loadBalancer.ID != "" { + record.Eventf( + stackitCluster, "LoadBalancerCreated", "Created API server load balancer %s", loadBalancer.ID, ) } @@ -195,9 +189,9 @@ func (r *StackitClusterReconciler) reconcileNormal(ctx context.Context, clusterS // out every machine while the API server is still needed for drain. if len(targets) > 0 { if err := cloudClient.SetAPIServerLoadBalancerTargets(ctx, loadBalancer.ID, defaultAPIServerPort, targets); err != nil { - if !cloud.IsRetryable(err) && r.Recorder != nil { - r.Recorder.Eventf( - stackitCluster, nil, corev1.EventTypeWarning, "LoadBalancerTargetError", "Update", + if !cloud.IsRetryable(err) { + record.Warnf( + stackitCluster, "LoadBalancerTargetError", "Cannot update API server load balancer target pool: %v", err, ) } @@ -225,12 +219,10 @@ func (r *StackitClusterReconciler) reconcileNormal(ctx context.Context, clusterS "", infrav1.ClusterLoadBalancerReadyCondition, ) - if r.Recorder != nil { - r.Recorder.Eventf( - stackitCluster, nil, corev1.EventTypeNormal, "LoadBalancerReady", "SetReady", - "API server load balancer is ready at %s", loadBalancer.IP, - ) - } + record.Eventf( + stackitCluster, "LoadBalancerReady", + "API server load balancer is ready at %s", loadBalancer.IP, + ) } } else if stackitCluster.Spec.ControlPlaneEndpoint.Host != "" { stackitCluster.Status.APIServerEndpoint = stackitCluster.Spec.ControlPlaneEndpoint @@ -242,9 +234,7 @@ func (r *StackitClusterReconciler) reconcileNormal(ctx context.Context, clusterS ) } else { log.Error(errors.New("controlPlaneEndpoint host is empty"), "API server load balancer is disabled and controlPlaneEndpoint is empty", "cluster", stackitCluster.Name) - if r.Recorder != nil { - r.Recorder.Eventf(stackitCluster, nil, corev1.EventTypeWarning, "EndpointMissing", "Reconcile", "apiServerLoadBalancer.enabled is false and controlPlaneEndpoint is empty") - } + record.Warn(stackitCluster, "EndpointMissing", "apiServerLoadBalancer.enabled is false and controlPlaneEndpoint is empty") clusterScope.SetNotReady( "EndpointMissing", "apiServerLoadBalancer.enabled is false and controlPlaneEndpoint is empty", @@ -258,12 +248,10 @@ func (r *StackitClusterReconciler) reconcileNormal(ctx context.Context, clusterS log.Error(err, "Failed to reconcile bastion", "cluster", stackitCluster.Name, ) - if r.Recorder != nil { - r.Recorder.Eventf( - stackitCluster, nil, corev1.EventTypeWarning, "FailedCreateBastion", "Create", - "Failed to reconcile bastion: %v", err, - ) - } + record.Warnf( + stackitCluster, "FailedCreateBastion", + "Failed to reconcile bastion: %v", err, + ) stackitCluster.Status.Ready = false return util.CloudFailureResult( &stackitCluster.Status.Conditions, @@ -340,10 +328,8 @@ func (r *StackitClusterReconciler) reconcileDelete(ctx context.Context, clusterS machines, err := collections.GetFilteredMachinesForCluster(ctx, r.Client, clusterScope.Cluster) if err != nil { log.Error(err, "Failed to list Machines for cluster during deletion", "cluster", clusterScope.Cluster.Name) - if r.Recorder != nil { - r.Recorder.Eventf(stackitCluster, nil, corev1.EventTypeWarning, "FailedListMachines", "Delete", - "Failed to list Machines for cluster %s: %v", clusterScope.Cluster.Name, err) - } + record.Warnf(stackitCluster, "FailedListMachines", + "Failed to list Machines for cluster %s: %v", clusterScope.Cluster.Name, err) return ctrl.Result{}, fmt.Errorf("list Machines for cluster %s: %w", clusterScope.Cluster.Name, err) } if machines.Len() > 0 { @@ -365,19 +351,15 @@ func (r *StackitClusterReconciler) reconcileDelete(ctx context.Context, clusterS // Every other credentials problem is fixable and keeps retrying. if apierrors.IsNotFound(err) { log.Error(err, "Credentials secret is gone; finalizing without cloud cleanup", "cluster", stackitCluster.Name) - if r.Recorder != nil { - r.Recorder.Eventf(stackitCluster, nil, corev1.EventTypeWarning, "CleanupSkipped", "Delete", - "Credentials Secret is gone; finalizing without cloud cleanup. "+ - "Any remaining STACKIT resources for this cluster must be removed manually: %v", err) - } + record.Warnf(stackitCluster, "CleanupSkipped", + "Credentials Secret is gone; finalizing without cloud cleanup. "+ + "Any remaining STACKIT resources for this cluster must be removed manually: %v", err) controllerutil.RemoveFinalizer(stackitCluster, infrav1.ClusterFinalizer) return ctrl.Result{}, nil } log.Error(err, "Failed to build cloud client for cluster deletion", "cluster", stackitCluster.Name) - if r.Recorder != nil { - r.Recorder.Eventf(stackitCluster, nil, corev1.EventTypeWarning, "InvalidCredentials", "Delete", - "Failed to build cloud client for deletion: %v", err) - } + record.Warnf(stackitCluster, "InvalidCredentials", + "Failed to build cloud client for deletion: %v", err) util.SetConditions( &stackitCluster.Status.Conditions, stackitCluster.Generation, @@ -391,35 +373,27 @@ func (r *StackitClusterReconciler) reconcileDelete(ctx context.Context, clusterS loadBalancerID, err := loadbalancerservice.ResolveID(ctx, cloudClient, stackitCluster) if err != nil { log.Error(err, "Failed to resolve API server load balancer for deletion", "cluster", stackitCluster.Name) - if r.Recorder != nil { - r.Recorder.Eventf(stackitCluster, nil, corev1.EventTypeWarning, "FailedResolveLoadBalancer", "Delete", - "Failed to resolve API server load balancer: %v", err) - } + record.Warnf(stackitCluster, "FailedResolveLoadBalancer", + "Failed to resolve API server load balancer: %v", err) return ctrl.Result{}, err } if loadBalancerID != "" { if err := cloudClient.DeleteAPIServerLoadBalancer(ctx, loadBalancerID); err != nil && !cloud.IsNotFound(err) { log.Error(err, "Failed to delete API server load balancer", "cluster", stackitCluster.Name, "loadBalancerID", loadBalancerID) - if r.Recorder != nil { - r.Recorder.Eventf(stackitCluster, nil, corev1.EventTypeWarning, "FailedDeleteLoadBalancer", "Delete", - "Failed to delete API server load balancer %s: %v", loadBalancerID, err) - } + record.Warnf(stackitCluster, "FailedDeleteLoadBalancer", + "Failed to delete API server load balancer %s: %v", loadBalancerID, err) return ctrl.Result{}, err } stackitCluster.Status.APIServerLoadBalancerID = "" - if r.Recorder != nil { - r.Recorder.Eventf( - stackitCluster, nil, corev1.EventTypeNormal, "LoadBalancerDeleted", "Delete", - "Deleted API server load balancer %s", loadBalancerID, - ) - } + record.Eventf( + stackitCluster, "LoadBalancerDeleted", + "Deleted API server load balancer %s", loadBalancerID, + ) } if err := cloudClient.DeleteNodeSSHAccess(ctx, bastionservice.NodeSSHAccessTags(stackitCluster)); err != nil && !cloud.IsNotFound(err) { log.Error(err, "Failed to delete bastion node SSH access", "cluster", stackitCluster.Name) - if r.Recorder != nil { - r.Recorder.Eventf(stackitCluster, nil, corev1.EventTypeWarning, "FailedDeleteNodeSSHAccess", "Delete", - "Failed to delete bastion node SSH access: %v", err) - } + record.Warnf(stackitCluster, "FailedDeleteNodeSSHAccess", + "Failed to delete bastion node SSH access: %v", err) return ctrl.Result{}, err } if err := cloudClient.DeleteBastion(ctx, bastionservice.Input(stackitCluster, nil), cloud.Bastion{ @@ -429,17 +403,13 @@ func (r *StackitClusterReconciler) reconcileDelete(ctx context.Context, clusterS SecurityGroupID: stackitCluster.Status.Bastion.SecurityGroupID, }); err != nil && !cloud.IsNotFound(err) { log.Error(err, "Failed to delete bastion", "cluster", stackitCluster.Name) - if r.Recorder != nil { - r.Recorder.Eventf(stackitCluster, nil, corev1.EventTypeWarning, "FailedDeleteBastion", "Delete", - "Failed to delete bastion: %v", err) - } + record.Warnf(stackitCluster, "FailedDeleteBastion", + "Failed to delete bastion: %v", err) return ctrl.Result{}, err } if hasBastionStatus(stackitCluster.Status.Bastion) { clusterScope.ClearBastionStatus() - if r.Recorder != nil { - r.Recorder.Eventf(stackitCluster, nil, corev1.EventTypeNormal, "BastionDeleted", "Delete", "Deleted bastion") - } + record.Event(stackitCluster, "BastionDeleted", "Deleted bastion") } controllerutil.RemoveFinalizer(stackitCluster, infrav1.ClusterFinalizer) return ctrl.Result{}, nil diff --git a/controller/stackitmachine_controller.go b/controller/stackitmachine_controller.go index cc91f21..30688dd 100644 --- a/controller/stackitmachine_controller.go +++ b/controller/stackitmachine_controller.go @@ -23,7 +23,6 @@ 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" clusterutil "sigs.k8s.io/cluster-api/util" ctrl "sigs.k8s.io/controller-runtime" @@ -44,7 +43,6 @@ type StackitMachineReconciler struct { // CloudClientFactory builds a cloud.Client from parsed credentials. CloudClientFactory cloud.Factory - Recorder events.EventRecorder } // +kubebuilder:rbac:groups=infrastructure.cluster.x-k8s.io,resources=stackitmachines,verbs=get;list;watch;create;update;patch;delete diff --git a/controller/stackitmachine_controller_test.go b/controller/stackitmachine_controller_test.go index 6de4f44..148ac44 100644 --- a/controller/stackitmachine_controller_test.go +++ b/controller/stackitmachine_controller_test.go @@ -22,7 +22,6 @@ import ( "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" - "k8s.io/client-go/tools/events" clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta2" "sigs.k8s.io/controller-runtime/pkg/reconcile" @@ -175,23 +174,19 @@ var _ = Describe("StackitMachine Controller", func() { "legacy status.ready must follow the Ready condition, not contradict it") }) - It("reports a stopped server with its own reason and warns once", func() { - recorder := events.NewFakeRecorder(10) - reconciler.Recorder = recorder + It("reports a stopped server with its own reason", func() { _, err := reconciler.Reconcile(ctx, request) Expect(err).NotTo(HaveOccurred()) - Expect(drainEvents(recorder)).To(ContainElement(HavePrefix("Normal InstanceCreated"))) got := &infrav1.StackitMachine{} Expect(k8sClient.Get(ctx, stackitKey, got)).To(Succeed()) - By("passing through the transitional STOPPING state without a warning") + By("passing through the transitional STOPPING state without failing") fakeCloud.SetServerState(got.Status.InstanceID, "STOPPING", "RUNNING") _, err = reconciler.Reconcile(ctx, request) Expect(err).NotTo(HaveOccurred()) - Expect(drainEvents(recorder)).NotTo(ContainElement(HavePrefix("Warning"))) - By("warning once the server is stopped") + By("marking not ready once the server is stopped") fakeCloud.SetServerState(got.Status.InstanceID, "INACTIVE", "STOPPED") result, err := reconciler.Reconcile(ctx, request) Expect(err).NotTo(HaveOccurred()) @@ -203,12 +198,10 @@ var _ = Describe("StackitMachine Controller", func() { expectCondition(got.Status.Conditions, infrav1.MachineInstanceReadyCondition, metav1.ConditionFalse, "InstanceStopped") condition := meta.FindStatusCondition(got.Status.Conditions, infrav1.MachineInstanceReadyCondition) Expect(condition.Message).To(Equal("server is stopped (state INACTIVE, power status STOPPED)")) - Expect(drainEvents(recorder)).To(ContainElement(HavePrefix("Warning InstanceStopped"))) - By("not warning again while the server stays stopped") + By("reconciling again while the server stays stopped") _, err = reconciler.Reconcile(ctx, request) Expect(err).NotTo(HaveOccurred()) - Expect(drainEvents(recorder)).NotTo(ContainElement(HavePrefix("Warning"))) }) It("reports an active but crashed server as not ready", func() { @@ -255,9 +248,7 @@ var _ = Describe("StackitMachine Controller", func() { Expect(fakeCloud.ServerHasSecurityGroup(got.Status.InstanceID, securityGroups[0].ID)).To(BeTrue()) }) - It("records a warning event and logs when bastion node SSH access fails", func() { - recorder := events.NewFakeRecorder(10) - reconciler.Recorder = recorder + It("marks conditions not ready when bastion node SSH access fails", func() { stackitCluster := &infrav1.StackitCluster{} Expect(k8sClient.Get(ctx, types.NamespacedName{Name: clusterName, Namespace: namespace}, stackitCluster)).To(Succeed()) stackitCluster.Spec.Bastion = validBastionSpec() @@ -275,14 +266,9 @@ var _ = Describe("StackitMachine Controller", func() { got := &infrav1.StackitMachine{} Expect(k8sClient.Get(ctx, stackitKey, got)).To(Succeed()) expectCondition(got.Status.Conditions, infrav1.MachineReadyCondition, metav1.ConditionFalse, "BastionSSHAccessError") - - events := drainEvents(recorder) - Expect(events).To(ContainElement(ContainSubstring("FailedNodeSSHAccess"))) }) It("does not create a VM when availabilityZone is outside published failure domains", func() { - recorder := events.NewFakeRecorder(10) - reconciler.Recorder = recorder got := &infrav1.StackitMachine{} Expect(k8sClient.Get(ctx, stackitKey, got)).To(Succeed()) got.Spec.AvailabilityZone = "eu01-9" @@ -297,14 +283,9 @@ var _ = Describe("StackitMachine Controller", func() { Expect(got.Status.Ready).To(BeFalse()) expectCondition(got.Status.Conditions, infrav1.MachineInstanceReadyCondition, metav1.ConditionFalse, "InvalidFailureDomain") expectCondition(got.Status.Conditions, infrav1.MachineReadyCondition, metav1.ConditionFalse, "InvalidFailureDomain") - - events := drainEvents(recorder) - Expect(events).To(ContainElement(ContainSubstring("InvalidFailureDomain"))) }) It("requeues on unauthorized credentials and recovers once they are corrected", func() { - recorder := events.NewFakeRecorder(10) - reconciler.Recorder = recorder reconciler.CloudClientFactory = func(context.Context, cloud.Credentials) (cloud.Client, error) { return nil, fmt.Errorf("authenticate: %w", cloud.ErrUnauthorized) } @@ -319,9 +300,6 @@ var _ = Describe("StackitMachine Controller", func() { expectCondition(got.Status.Conditions, infrav1.MachineCredentialsReadyCondition, metav1.ConditionFalse, "CredentialsInvalid") expectCondition(got.Status.Conditions, infrav1.MachineReadyCondition, metav1.ConditionFalse, "CredentialsInvalid") - events := drainEvents(recorder) - Expect(events).To(ContainElement(ContainSubstring("InvalidCredentials"))) - By("correcting the credentials") reconciler.CloudClientFactory = func(context.Context, cloud.Credentials) (cloud.Client, error) { return fakeCloud, nil @@ -410,8 +388,6 @@ var _ = Describe("StackitMachine Controller", func() { }) It("returns an error when VM creation is rejected", func() { - recorder := events.NewFakeRecorder(10) - reconciler.Recorder = recorder fakeCloud.FailNextCreateServer = fmt.Errorf("create server rejected: %w", cloud.ErrInvalidInput) result, err := reconciler.Reconcile(ctx, request) @@ -423,9 +399,6 @@ var _ = Describe("StackitMachine Controller", func() { Expect(k8sClient.Get(ctx, stackitKey, got)).To(Succeed()) expectCondition(got.Status.Conditions, infrav1.MachineInstanceReadyCondition, metav1.ConditionFalse, "InstanceError") expectCondition(got.Status.Conditions, infrav1.MachineReadyCondition, metav1.ConditionFalse, "InstanceError") - - events := drainEvents(recorder) - Expect(events).To(ContainElement(ContainSubstring("FailedCreateInstance"))) }) It("deletes the VM and removes the finalizer", func() { @@ -446,14 +419,11 @@ var _ = Describe("StackitMachine Controller", func() { }).Should(BeTrue()) }) - It("records a warning event and logs when server deletion fails", func() { + It("returns an error when server deletion fails", func() { _, err := reconciler.Reconcile(ctx, request) Expect(err).NotTo(HaveOccurred()) Expect(fakeCloud.ServerCount()).To(Equal(1)) - recorder := events.NewFakeRecorder(10) - reconciler.Recorder = recorder - got := &infrav1.StackitMachine{} Expect(k8sClient.Get(ctx, stackitKey, got)).To(Succeed()) Expect(k8sClient.Delete(ctx, got)).To(Succeed()) @@ -462,9 +432,6 @@ var _ = Describe("StackitMachine Controller", func() { _, err = reconciler.Reconcile(ctx, request) Expect(err).To(HaveOccurred()) - - events := drainEvents(recorder) - Expect(events).To(ContainElement(ContainSubstring("FailedDeleteInstance"))) }) // The hook observes API server state from inside the cloud call, so it proves diff --git a/controller/stackitmachine_infrastructure.go b/controller/stackitmachine_infrastructure.go index a055528..62bec6c 100644 --- a/controller/stackitmachine_infrastructure.go +++ b/controller/stackitmachine_infrastructure.go @@ -22,6 +22,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta2" + "sigs.k8s.io/cluster-api/util/record" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" logf "sigs.k8s.io/controller-runtime/pkg/log" @@ -55,9 +56,7 @@ func (r *StackitMachineReconciler) reconcileNormal(ctx context.Context, machineS } if err := validateMachineAvailabilityZone(machineScope); err != nil { log.Error(err, "Invalid availability zone for machine", "machine", stackitMachine.Name, "availabilityZone", stackitMachine.Spec.AvailabilityZone) - if r.Recorder != nil { - r.Recorder.Eventf(stackitMachine, nil, corev1.EventTypeWarning, "InvalidFailureDomain", "Reconcile", "%v", err) - } + record.Warnf(stackitMachine, "InvalidFailureDomain", "%v", err) machineScope.SetNotReady( "InvalidFailureDomain", err.Error(), @@ -71,9 +70,7 @@ func (r *StackitMachineReconciler) reconcileNormal(ctx context.Context, machineS machineScope.SetConditions(conditionStatus, reason, message, infrav1.MachineBootstrapReadyCondition) if conditionStatus != metav1.ConditionTrue { log.Error(errors.New(message), "Bootstrap data is not ready", "machine", stackitMachine.Name, "reason", reason) - if r.Recorder != nil { - r.Recorder.Eventf(stackitMachine, nil, corev1.EventTypeWarning, reason, "Reconcile", "Bootstrap data error: %s", message) - } + record.Warnf(stackitMachine, reason, "Bootstrap data error: %s", message) machineScope.SetConditions(metav1.ConditionFalse, reason, message, infrav1.MachineReadyCondition) if reason == util.BootstrapReasonInvalid { return ctrl.Result{}, nil @@ -84,9 +81,7 @@ func (r *StackitMachineReconciler) reconcileNormal(ctx context.Context, machineS cloudClient, err := util.BuildCloudClient(ctx, r.Client, r.CloudClientFactory, machineScope.StackitCluster) if err != nil { log.Error(err, "Failed to build cloud client for machine", "machine", stackitMachine.Name) - if r.Recorder != nil { - r.Recorder.Eventf(stackitMachine, nil, corev1.EventTypeWarning, "InvalidCredentials", "Reconcile", "Failed to build cloud client: %v", err) - } + record.Warnf(stackitMachine, "InvalidCredentials", "Failed to build cloud client: %v", err) return util.CredentialFailureResult( &stackitMachine.Status.Conditions, stackitMachine.Generation, @@ -106,12 +101,10 @@ func (r *StackitMachineReconciler) reconcileNormal(ctx context.Context, machineS infrav1.MachineInstanceReadyCondition, infrav1.MachineReadyCondition, ) - if r.Recorder != nil { - r.Recorder.Eventf( - stackitMachine, nil, corev1.EventTypeWarning, "InstanceNotFound", "Reconcile", - "Server %s no longer exists; the Machine must be replaced", stackitMachine.Status.InstanceID, - ) - } + record.Warnf( + stackitMachine, "InstanceNotFound", + "Server %s no longer exists; the Machine must be replaced", stackitMachine.Status.InstanceID, + ) return ctrl.Result{}, nil } if err != nil { @@ -121,14 +114,12 @@ func (r *StackitMachineReconciler) reconcileNormal(ctx context.Context, machineS "networkID", stackitMachine.Spec.Network.ID, "region", machineScope.StackitCluster.Spec.Region, ) - if r.Recorder != nil { - r.Recorder.Eventf( - stackitMachine, nil, corev1.EventTypeWarning, "FailedCreateInstance", "Create", - "Failed to create server %s (imageID: %s, networkID: %s, region: %s): %v", - stackitMachine.Name, stackitMachine.Spec.ImageID, stackitMachine.Spec.Network.ID, - machineScope.StackitCluster.Spec.Region, err, - ) - } + record.Warnf( + stackitMachine, "FailedCreateInstance", + "Failed to create server %s (imageID: %s, networkID: %s, region: %s): %v", + stackitMachine.Name, stackitMachine.Spec.ImageID, stackitMachine.Spec.Network.ID, + machineScope.StackitCluster.Spec.Region, err, + ) machineScope.SetNotReady( "InstanceError", err.Error(), @@ -146,9 +137,9 @@ func (r *StackitMachineReconciler) reconcileNormal(ctx context.Context, machineS infrav1.MachineReadyCondition, ) } - if created && r.Recorder != nil { - r.Recorder.Eventf( - stackitMachine, nil, corev1.EventTypeNormal, "InstanceCreated", "Create", "Created instance %s", server.ID, + if created { + record.Eventf( + stackitMachine, "InstanceCreated", "Created instance %s", server.ID, ) } @@ -163,20 +154,18 @@ func (r *StackitMachineReconciler) reconcileNormal(ctx context.Context, machineS } machineScope.SetNotReady(reason, message, infrav1.MachineInstanceReadyCondition, infrav1.MachineReadyCondition) // Only warn on entering the state, not on every requeue. - if warn && r.Recorder != nil && previousReason != reason { - r.Recorder.Eventf(stackitMachine, nil, corev1.EventTypeWarning, reason, "Reconcile", "Server %s: %s", server.ID, message) + if warn && previousReason != reason { + record.Warnf(stackitMachine, reason, "Server %s: %s", server.ID, message) } return ctrl.Result{RequeueAfter: 15 * time.Second}, nil } if err := r.reconcileBastionNodeSSHAccess(ctx, cloudClient, machineScope, server); err != nil { log.Error(err, "Failed to configure bastion SSH access for node", "machine", stackitMachine.Name) - if r.Recorder != nil { - r.Recorder.Eventf( - stackitMachine, nil, corev1.EventTypeWarning, "FailedNodeSSHAccess", "Reconcile", - "Failed to configure bastion SSH access for server %s: %v", server.ID, err, - ) - } + record.Warnf( + stackitMachine, "FailedNodeSSHAccess", + "Failed to configure bastion SSH access for server %s: %v", server.ID, err, + ) machineScope.SetNotReady("BastionSSHAccessError", err.Error(), infrav1.MachineReadyCondition) return util.CloudFailureResult( &stackitMachine.Status.Conditions, @@ -221,19 +210,15 @@ func (r *StackitMachineReconciler) reconcileDelete(ctx context.Context, machineS // Every other credentials problem is fixable and keeps retrying. if apierrors.IsNotFound(err) { log.Error(err, "Credentials secret is gone; finalizing without cloud cleanup", "machine", stackitMachine.Name) - if r.Recorder != nil { - r.Recorder.Eventf(stackitMachine, nil, corev1.EventTypeWarning, "CleanupSkipped", "Delete", - "Credentials Secret is gone; finalizing without cloud cleanup. "+ - "Any remaining STACKIT server for this machine must be removed manually: %v", err) - } + record.Warnf(stackitMachine, "CleanupSkipped", + "Credentials Secret is gone; finalizing without cloud cleanup. "+ + "Any remaining STACKIT server for this machine must be removed manually: %v", err) controllerutil.RemoveFinalizer(stackitMachine, infrav1.MachineFinalizer) return nil } log.Error(err, "Failed to build cloud client for machine deletion", "machine", stackitMachine.Name) - if r.Recorder != nil { - r.Recorder.Eventf(stackitMachine, nil, corev1.EventTypeWarning, "InvalidCredentials", "Delete", - "Failed to build cloud client for deletion: %v", err) - } + record.Warnf(stackitMachine, "InvalidCredentials", + "Failed to build cloud client for deletion: %v", err) _, resultErr := util.CredentialFailureResult( &stackitMachine.Status.Conditions, stackitMachine.Generation, @@ -247,27 +232,21 @@ func (r *StackitMachineReconciler) reconcileDelete(ctx context.Context, machineS instanceID, err := r.resolveServerForDeletion(ctx, cloudClient, machineScope) if err != nil { log.Error(err, "Failed to resolve server for deletion", "machine", stackitMachine.Name) - if r.Recorder != nil { - r.Recorder.Eventf(stackitMachine, nil, corev1.EventTypeWarning, "FailedResolveInstance", "Delete", - "Failed to resolve server for deletion: %v", err) - } + record.Warnf(stackitMachine, "FailedResolveInstance", + "Failed to resolve server for deletion: %v", err) return err } if instanceID != "" { if err := cloudClient.DeleteServer(ctx, instanceID); err != nil && !cloud.IsNotFound(err) { log.Error(err, "Failed to delete server", "machine", stackitMachine.Name, "instanceID", instanceID) - if r.Recorder != nil { - r.Recorder.Eventf(stackitMachine, nil, corev1.EventTypeWarning, "FailedDeleteInstance", "Delete", - "Failed to delete server %s: %v", instanceID, err) - } + record.Warnf(stackitMachine, "FailedDeleteInstance", + "Failed to delete server %s: %v", instanceID, err) return err } machineScope.ClearInstance() - if r.Recorder != nil { - r.Recorder.Eventf( - stackitMachine, nil, corev1.EventTypeNormal, "InstanceDeleted", "Delete", "Deleted instance %s", instanceID, - ) - } + record.Eventf( + stackitMachine, "InstanceDeleted", "Deleted instance %s", instanceID, + ) } controllerutil.RemoveFinalizer(stackitMachine, infrav1.MachineFinalizer) return nil