From 6c37593dc207656d25de2b9ce5b28fda360b10ae Mon Sep 17 00:00:00 2001 From: lukasmetzner Date: Mon, 17 Aug 2026 12:07:18 +0200 Subject: [PATCH] refactor: use WarnEventLogf --- hcloud/instances.go | 15 +++--- hcloud/instances_util.go | 6 ++- hcloud/routes.go | 8 +-- internal/hcops/load_balancer.go | 30 +++++------ internal/utils/eventlog_test.go | 93 +++++++++++++++++++++++++++++++++ 5 files changed, 124 insertions(+), 28 deletions(-) create mode 100644 internal/utils/eventlog_test.go diff --git a/hcloud/instances.go b/hcloud/instances.go index 3bb89510f..9198624f0 100644 --- a/hcloud/instances.go +++ b/hcloud/instances.go @@ -34,6 +34,7 @@ import ( "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/legacydatacenter" "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/metrics" "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/providerid" + "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/utils" "github.com/hetznercloud/hcloud-go/v2/hcloud" ) @@ -135,9 +136,9 @@ func (i *instances) lookupServer( } if cloudServer != nil && hrobotServer != nil { - i.recorder.Eventf( + utils.WarnEventLogf( + i.recorder, node, - corev1.EventTypeWarning, "InstanceLookupFailed", "Node %s could not be uniquely associated with a Cloud or Robot server, as a server with this name exists in both APIs", node.Name, @@ -314,19 +315,19 @@ func robotNodeAddresses( } if warnMsg != "" { - recorder.Event(node, corev1.EventTypeWarning, MisconfiguredInternalIP, warnMsg) - klog.Warning(warnMsg) + utils.WarnEventLogf(recorder, node, MisconfiguredInternalIP, "%s", warnMsg) continue } for _, address := range addresses { if currentAddress.Address == address.Address { - warnMsg := fmt.Sprintf( + utils.WarnEventLogf( + recorder, + node, + MisconfiguredInternalIP, "Configured InternalIP already exists as an ExternalIP. As a result, %s is not added as an InternalIP", currentAddress.Address, ) - recorder.Event(node, corev1.EventTypeWarning, MisconfiguredInternalIP, warnMsg) - klog.Warning(warnMsg) continue OUTER } } diff --git a/hcloud/instances_util.go b/hcloud/instances_util.go index a86f0b447..dbbb37554 100644 --- a/hcloud/instances_util.go +++ b/hcloud/instances_util.go @@ -25,6 +25,8 @@ import ( hrobotmodels "github.com/syself/hrobot-go/models" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/runtime" + + "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/utils" ) type MockEventRecorder struct{} @@ -88,9 +90,9 @@ func getRobotServerByID(i *instances, id int, node *corev1.Node) (*hrobotmodels. // the name in the Robot API is updated. As the node no longer exists in the cluster with the old name, // we need to return nil here. if server.Name != node.Name { - i.recorder.Eventf( + utils.WarnEventLogf( + i.recorder, node, - corev1.EventTypeWarning, "PossibleNodeDeletion", "Might be deleted by node-lifecycle-manager due to name mismatch; Node name %q differs from Robot name %q", node.ObjectMeta.Name, diff --git a/hcloud/routes.go b/hcloud/routes.go index 7406bf623..ac7d09d22 100644 --- a/hcloud/routes.go +++ b/hcloud/routes.go @@ -18,6 +18,7 @@ import ( "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/cache" "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/metrics" "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/providerid" + "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/utils" "github.com/hetznercloud/hcloud-go/v2/hcloud" ) @@ -324,12 +325,13 @@ func (r *routes) warnCIDRMismatch(cidr *net.IPNet, node *corev1.Node) { destPrefixLen, _ := cidr.Mask.Size() if !r.clusterCIDR.Contains(cidr.IP) || destPrefixLen < clusterPrefixLen { - warnMsg := fmt.Sprintf( + utils.WarnEventLogf( + r.recorder, + node, + "ClusterCIDRMisconfigured", "route CIDR %s is not contained within cluster CIDR %s", cidr.String(), r.clusterCIDR.String(), ) - klog.Warning(warnMsg) - r.recorder.Event(node, corev1.EventTypeWarning, "ClusterCIDRMisconfigured", warnMsg) } } diff --git a/internal/hcops/load_balancer.go b/internal/hcops/load_balancer.go index b43b478e6..db04c7820 100644 --- a/internal/hcops/load_balancer.go +++ b/internal/hcops/load_balancer.go @@ -681,9 +681,9 @@ func (l *LoadBalancerOps) ReconcileHCLBTargets( if errors.As(err, new(*providerid.UnkownPrefixError)) { // ProviderID has unknown prefix, cluster might have non-hccm nodes that can not be added to the // Load Balancer. Emitting an event and ignoring that Node in this reconciliation loop. - l.Recorder.Eventf( + utils.WarnEventLogf( + l.Recorder, node, - corev1.EventTypeWarning, "UnknownProviderIDPrefix", "Node could not be added to Load Balancer for service %s because the provider ID does not match any known format", svc.Name, @@ -731,14 +731,15 @@ func (l *LoadBalancerOps) ReconcileHCLBTargets( // Check if InternalIP is set at Node object internalIP := getNodeInternalIP(node) if internalIP == "" { - warnMsg := fmt.Sprintf( + utils.WarnEventLogf( + l.Recorder, + svc, + "InternalIPNotConfigured", "%s: load balancer %s has set `use-private-ip: true`, but no InternalIP found for node %s. Continuing with ExternalIP.", op, svc.Name, node.Name, ) - klog.Warning(warnMsg) - l.Recorder.Eventf(svc, corev1.EventTypeWarning, "InternalIPNotConfigured", "%s", warnMsg) continue } @@ -761,13 +762,14 @@ func (l *LoadBalancerOps) ReconcileHCLBTargets( internalIP := getNodeInternalIP(node) if internalIP == "" { - warnMsg := fmt.Sprintf( + utils.WarnEventLogf( + l.Recorder, + svc, + "InternalIPNotConfigured", "no InternalIP found for Robot node %s (id=%d), cannot add as LB target without Robot credentials; skipping", node.Name, id, ) - klog.Warning(warnMsg) - l.Recorder.Eventf(svc, corev1.EventTypeWarning, "InternalIPNotConfigured", "%s", warnMsg) continue } @@ -989,18 +991,14 @@ func (l *LoadBalancerOps) ReconcileHCLBServices( ) if port.Protocol != "" && port.Protocol != corev1.ProtocolTCP { - warnMsg := fmt.Sprintf( + utils.WarnEventLogf( + l.Recorder, + svc, + "UnsupportedProtocolConfigured", "configured unsupported Hetzner Cloud load balancer protocol %s for service with name %s", port.Protocol, svc.Name, ) - l.Recorder.Event( - svc, - corev1.EventTypeWarning, - "UnsupportedProtocolConfigured", - warnMsg, - ) - klog.Warning(warnMsg) continue } diff --git a/internal/utils/eventlog_test.go b/internal/utils/eventlog_test.go new file mode 100644 index 000000000..262bdf9d7 --- /dev/null +++ b/internal/utils/eventlog_test.go @@ -0,0 +1,93 @@ +package utils + +import ( + "bytes" + "testing" + + "github.com/stretchr/testify/assert" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/tools/record" + "k8s.io/klog/v2" +) + +func captureKlog(t *testing.T) *bytes.Buffer { + t.Helper() + + state := klog.CaptureState() + t.Cleanup(state.Restore) + + var buf bytes.Buffer + klog.LogToStderr(false) + klog.SetOutput(&buf) + + return &buf +} + +func TestWarnEventLogf(t *testing.T) { + tests := []struct { + name string + reason string + msg string + args []any + expected string + }{ + { + name: "without args", + reason: "InternalIPNotConfigured", + msg: "no InternalIP found for node", + expected: "no InternalIP found for node", + }, + { + name: "with args", + reason: "UnsupportedProtocolConfigured", + msg: "unsupported protocol %s for service %s", + args: []any{corev1.ProtocolUDP, "my-service"}, + expected: "unsupported protocol UDP for service my-service", + }, + { + name: "verbs in args are not expanded again", + reason: "LoadBalancerTypeUnconfigured", + msg: "set it with the annotation %q", + args: []any{"load-balancer.hetzner.cloud/type=%s"}, + expected: `set it with the annotation "load-balancer.hetzner.cloud/type=%s"`, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + logs := captureKlog(t) + recorder := record.NewFakeRecorder(1) + + WarnEventLogf(recorder, &corev1.Node{}, tt.reason, tt.msg, tt.args...) + + assert.Equal(t, "Warning "+tt.reason+" "+tt.expected, <-recorder.Events) + + klog.Flush() + // klog prefix `W` for warning + assert.Regexp(t, `^W\d`, logs.String()) + assert.Contains(t, logs.String(), tt.expected) + }) + } +} + +func TestWarnEventLogfEventObject(t *testing.T) { + captureKlog(t) + + recorder := record.NewFakeRecorder(1) + recorder.IncludeObject = true + + node := &corev1.Node{ + TypeMeta: metav1.TypeMeta{Kind: "Node", APIVersion: "v1"}, + ObjectMeta: metav1.ObjectMeta{Name: "my-node"}, + } + + WarnEventLogf(recorder, node, "ServerNotFound", "no server with id %d was found in Robot", 42) + + // FakeRecorder adds `involvedObject` section + assert.Equal( + t, + "Warning ServerNotFound no server with id 42 was found in Robot involvedObject{kind=Node,apiVersion=v1}", + <-recorder.Events, + ) +}