diff --git a/hcloud/cloud.go b/hcloud/cloud.go index 4df6642d0..50f91250a 100644 --- a/hcloud/cloud.go +++ b/hcloud/cloud.go @@ -35,6 +35,7 @@ import ( "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/cache" "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/config" + "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/events" "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/hcops" "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/instance" "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/metrics" @@ -60,7 +61,7 @@ type cloud struct { serverCache *cache.Cache[hcloud.Server] lbTypeCache *cache.Cache[hcloud.LoadBalancerType] cfg config.HCCMConfiguration - recorder record.EventRecorder + recorder *events.Recorder networkID int64 cidr string nodeLister corelisters.NodeLister @@ -181,7 +182,7 @@ func (c *cloud) Initialize(clientBuilder cloudprovider.ControllerClientBuilder, }() recorder := eventBroadcaster.NewRecorder(scheme.Scheme, corev1.EventSource{Component: "hcloud-cloud-controller-manager"}) - c.recorder = recorder + c.recorder = events.NewRecorder(recorder) } func (c *cloud) Instances() (cloudprovider.Instances, bool) { diff --git a/hcloud/load_balancers.go b/hcloud/load_balancers.go index b7ba486fc..c1dfcec25 100644 --- a/hcloud/load_balancers.go +++ b/hcloud/load_balancers.go @@ -8,10 +8,10 @@ import ( corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/labels" - "k8s.io/client-go/tools/record" "k8s.io/klog/v2" "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/config" + "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/events" "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/hcops" "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/lbspec" "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/metrics" @@ -41,10 +41,10 @@ type LoadBalancerOps interface { type loadBalancers struct { lbOps LoadBalancerOps cfg *config.LoadBalancerConfiguration - recorder record.EventRecorder + recorder *events.Recorder } -func newLoadBalancers(lbOps LoadBalancerOps, lbCfg *config.LoadBalancerConfiguration, recorder record.EventRecorder) *loadBalancers { +func newLoadBalancers(lbOps LoadBalancerOps, lbCfg *config.LoadBalancerConfiguration, recorder *events.Recorder) *loadBalancers { return &loadBalancers{ lbOps: lbOps, cfg: lbCfg, diff --git a/hcloud/load_balancers_test.go b/hcloud/load_balancers_test.go index 15552f81c..850dd68ac 100644 --- a/hcloud/load_balancers_test.go +++ b/hcloud/load_balancers_test.go @@ -14,6 +14,7 @@ import ( "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/annotation" "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/config" + "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/events" "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/hcops" "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/lbspec" "github.com/hetznercloud/hcloud-go/v2/hcloud" @@ -989,7 +990,7 @@ func TestLoadBalancer_matchNodeSelector(t *testing.T) { for _, c := range cases { t.Run(c.name, func(t *testing.T) { - spec, err := lbspec.Resolve(record.NewFakeRecorder(10), c.service, config.LoadBalancerConfiguration{}) + spec, err := lbspec.Resolve(events.NewRecorder(record.NewFakeRecorder(10)), c.service, config.LoadBalancerConfiguration{}) if err != nil { t.Fatal(err) } diff --git a/hcloud/testing.go b/hcloud/testing.go index b976f40de..82d74e8ac 100644 --- a/hcloud/testing.go +++ b/hcloud/testing.go @@ -11,6 +11,7 @@ import ( "k8s.io/client-go/tools/record" "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/config" + "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/events" "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/hcops" "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/lbspec" "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/mocks" @@ -83,13 +84,13 @@ func (tt *LoadBalancerTestCase) run(t *testing.T) { // The error is ignored on purpose: a test case may set an invalid // annotation to check how the cloud provider reports it. - tt.Spec, _ = lbspec.Resolve(&record.FakeRecorder{}, tt.Service, *lbConfig) + tt.Spec, _ = lbspec.Resolve(events.NewRecorder(&record.FakeRecorder{}), tt.Service, *lbConfig) if tt.Mock != nil { tt.Mock(t, tt) } - tt.LoadBalancers = newLoadBalancers(tt.LBOps, lbConfig, &record.FakeRecorder{}) + tt.LoadBalancers = newLoadBalancers(tt.LBOps, lbConfig, events.NewRecorder(&record.FakeRecorder{})) tt.Perform(t, tt) tt.LBOps.AssertExpectations(t) diff --git a/internal/events/events.go b/internal/events/events.go new file mode 100644 index 000000000..3dc10f163 --- /dev/null +++ b/internal/events/events.go @@ -0,0 +1,31 @@ +package events + +type Reason string + +// Instances controller event reasons. +const ( + PossibleNodeDeletion Reason = "PossibleNodeDeletion" + InstanceLookupFailed Reason = "InstanceLookupFailed" + IgnoredExternalIPv6 Reason = "IgnoredExternalIPv6" + InvalidExternalIPv6 Reason = "InvalidExternalIPv6" + MisconfiguredInternalIP Reason = "MisconfiguredInternalIP" +) + +// Route controller event reasons. +const ( + ClusterCIDRMisconfigured Reason = "ClusterCIDRMisconfigured" +) + +// Load balancer controller event reasons. +const ( + PrimaryIPChangeUnsupported Reason = "PrimaryIPChangeUnsupported" + UnknownProviderIDPrefix Reason = "UnknownProviderIDPrefix" + InternalIPNotConfigured Reason = "InternalIPNotConfigured" + ExternalIPTargetReplaced Reason = "ExternalIPTargetReplaced" + ServerNotFound Reason = "ServerNotFound" + MaxTargetsReached Reason = "MaxTargetsReached" + UnsupportedProtocolConfigured Reason = "UnsupportedProtocolConfigured" + LoadBalancerTypeUnconfigured Reason = "LoadBalancerTypeUnconfigured" + LoadBalancerTypeDeprecated Reason = "LoadBalancerTypeDeprecated" + InvalidLoadBalancerAnnotation Reason = "InvalidLoadBalancerAnnotation" +) diff --git a/internal/events/recorder.go b/internal/events/recorder.go new file mode 100644 index 000000000..df92fb457 --- /dev/null +++ b/internal/events/recorder.go @@ -0,0 +1,69 @@ +package events + +import ( + "fmt" + + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/client-go/kubernetes/scheme" + "k8s.io/client-go/tools/record" + "k8s.io/klog/v2" +) + +type Recorder struct { + recorder record.EventRecorder +} + +func NewRecorder(recorder record.EventRecorder) *Recorder { + return &Recorder{recorder: recorder} +} + +// Warnf emits a warning event and a structured info log message. +func (r *Recorder) Warnf(obj runtime.Object, reason Reason, format string, args ...any) { + r.emit(obj, corev1.EventTypeWarning, reason, fmt.Sprintf(format, args...)) +} + +// Normalf emits a normal event and a structured info log message. +func (r *Recorder) Normalf(obj runtime.Object, reason Reason, format string, args ...any) { + r.emit(obj, corev1.EventTypeNormal, reason, fmt.Sprintf(format, args...)) +} + +// Errorf emits a warning event and a structured error log message. +func (r *Recorder) Errorf(obj runtime.Object, reason Reason, format string, args ...any) { + msg := fmt.Sprintf(format, args...) + r.recorder.Event(obj, corev1.EventTypeWarning, string(reason), msg) + // Depth 1 skips Warnf/Normalf so file:line points at the caller. + klog.ErrorSDepth(1, nil, msg, logKeysAndValues(obj, corev1.EventTypeWarning, reason)...) +} + +func (r *Recorder) emit(obj runtime.Object, eventType string, reason Reason, msg string) { + r.recorder.Event(obj, eventType, string(reason), msg) + // Depth 2 skips emit and Warnf/Normalf so file:line points at the caller. + klog.InfoSDepth(2, msg, logKeysAndValues(obj, eventType, reason)...) +} + +func logKeysAndValues(obj runtime.Object, eventType string, reason Reason) []any { + return []any{ + "kind", objKind(obj), + "object", objRef(obj), + "type", eventType, + "reason", string(reason), + } +} + +func objRef(obj runtime.Object) any { + if m, ok := obj.(klog.KMetadata); ok { + return klog.KObj(m) + } + return fmt.Sprintf("%T", obj) +} + +func objKind(obj runtime.Object) string { + if gvk := obj.GetObjectKind().GroupVersionKind(); gvk.Kind != "" { + return gvk.Kind + } + if gvks, _, err := scheme.Scheme.ObjectKinds(obj); err == nil && len(gvks) > 0 { + return gvks[0].Kind + } + return fmt.Sprintf("%T", obj) +} diff --git a/internal/events/recorder_test.go b/internal/events/recorder_test.go new file mode 100644 index 000000000..e86c7ecd2 --- /dev/null +++ b/internal/events/recorder_test.go @@ -0,0 +1,146 @@ +package events + +import ( + "testing" + + "github.com/stretchr/testify/assert" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/client-go/tools/record" + "k8s.io/klog/v2" + + "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/testsupport" +) + +type unregisteredObject struct { + metav1.TypeMeta +} + +func (o *unregisteredObject) DeepCopyObject() runtime.Object { + return &unregisteredObject{TypeMeta: o.TypeMeta} +} + +func TestRecorder(t *testing.T) { + node := &corev1.Node{ObjectMeta: metav1.ObjectMeta{Name: "my-node"}} + svc := &corev1.Service{ObjectMeta: metav1.ObjectMeta{Name: "my-svc", Namespace: "default"}} + + tests := []struct { + name string + emit func(r *Recorder) + expectedEvent string + expectedLevel string + expectedLog string + }{ + { + name: "warning", + emit: func(r *Recorder) { + r.Warnf(node, PossibleNodeDeletion, "node %s might be deleted", node.Name) + }, + expectedEvent: "Warning PossibleNodeDeletion node my-node might be deleted", + expectedLevel: "I", + expectedLog: `"node my-node might be deleted" kind="Node" object="my-node" type="Warning" reason="PossibleNodeDeletion"`, + }, + { + name: "normal", + emit: func(r *Recorder) { + r.Normalf(svc, LoadBalancerTypeDeprecated, "type %s is deprecated", "lb11") + }, + expectedEvent: "Normal LoadBalancerTypeDeprecated type lb11 is deprecated", + expectedLevel: "I", + expectedLog: `"type lb11 is deprecated" kind="Service" object="default/my-svc" type="Normal" reason="LoadBalancerTypeDeprecated"`, + }, + { + name: "error", + emit: func(r *Recorder) { + r.Errorf(svc, InvalidLoadBalancerAnnotation, "Invalid Load Balancer annotation %s", "foo") + }, + expectedEvent: "Warning InvalidLoadBalancerAnnotation Invalid Load Balancer annotation foo", + expectedLevel: "E", + expectedLog: `"Invalid Load Balancer annotation foo" kind="Service" object="default/my-svc" type="Warning" reason="InvalidLoadBalancerAnnotation"`, + }, + { + name: "without args", + emit: func(r *Recorder) { + r.Warnf(svc, InternalIPNotConfigured, "no InternalIP found") + }, + expectedEvent: "Warning InternalIPNotConfigured no InternalIP found", + expectedLevel: "I", + expectedLog: `"no InternalIP found" kind="Service" object="default/my-svc" type="Warning" reason="InternalIPNotConfigured"`, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + logs := testsupport.CaptureKlog(t) + fakeRecorder := record.NewFakeRecorder(1) + + tt.emit(NewRecorder(fakeRecorder)) + + testsupport.AssertEvent(t, fakeRecorder, tt.expectedEvent) + assert.Regexp(t, `^`+tt.expectedLevel+`\d`, logs.String()) + assert.Contains(t, logs.String(), tt.expectedLog) + assert.Regexp(t, `recorder_test\.go:\d+\] `, logs.String()) + }) + } +} + +func TestObjKind(t *testing.T) { + tests := []struct { + name string + obj runtime.Object + expected string + }{ + { + name: "from type meta", + obj: &corev1.Node{TypeMeta: metav1.TypeMeta{Kind: "CustomNode"}}, + expected: "CustomNode", + }, + { + name: "from scheme", + obj: &corev1.Service{}, + expected: "Service", + }, + { + name: "unregistered type", + obj: &unregisteredObject{}, + expected: "*events.unregisteredObject", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.expected, objKind(tt.obj)) + }) + } +} + +func TestObjRef(t *testing.T) { + tests := []struct { + name string + obj runtime.Object + expected any + }{ + { + name: "cluster scoped", + obj: &corev1.Node{ObjectMeta: metav1.ObjectMeta{Name: "my-node"}}, + expected: klog.KRef("", "my-node"), + }, + { + name: "namespaced", + obj: &corev1.Service{ObjectMeta: metav1.ObjectMeta{Name: "my-svc", Namespace: "default"}}, + expected: klog.KRef("default", "my-svc"), + }, + { + name: "without metadata", + obj: &unregisteredObject{}, + expected: "*events.unregisteredObject", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.expected, objRef(tt.obj)) + }) + } +} diff --git a/internal/hcops/load_balancer.go b/internal/hcops/load_balancer.go index c8200496d..09dec7b4f 100644 --- a/internal/hcops/load_balancer.go +++ b/internal/hcops/load_balancer.go @@ -10,16 +10,15 @@ import ( hrobot "github.com/syself/hrobot-go" corev1 "k8s.io/api/core/v1" - "k8s.io/client-go/tools/record" "k8s.io/klog/v2" "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/annotation" "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/cache" "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/config" + "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/events" "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/lbspec" "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" "github.com/hetznercloud/hcloud-go/v2/hcloud/exp/deprecationutil" ) @@ -37,7 +36,7 @@ type LoadBalancerOps struct { RetryDelay time.Duration NetworkID int64 Cfg config.HCCMConfiguration - Recorder record.EventRecorder + Recorder *events.Recorder } // GetByK8SServiceUID tries to find a Load Balancer by its Kubernetes service @@ -243,10 +242,9 @@ func (l *LoadBalancerOps) reconcilePrimaryIPs( spec lbspec.Spec, ) { if spec.PrimaryIPv4 != 0 && spec.PrimaryIPv4 != lb.PublicNet.IPv4.ID { - utils.WarnEventLogf( - l.Recorder, + l.Recorder.Warnf( svc, - "PrimaryIPChangeUnsupported", + events.PrimaryIPChangeUnsupported, "Load Balancer has Primary IPv4 %d, changing it to %d is not supported", lb.PublicNet.IPv4.ID, spec.PrimaryIPv4, @@ -254,10 +252,9 @@ func (l *LoadBalancerOps) reconcilePrimaryIPs( } if spec.PrimaryIPv6 != 0 && spec.PrimaryIPv6 != lb.PublicNet.IPv6.ID { - utils.WarnEventLogf( - l.Recorder, + l.Recorder.Warnf( svc, - "PrimaryIPChangeUnsupported", + events.PrimaryIPChangeUnsupported, "Load Balancer has Primary IPv6 %d, changing it to %d is not supported", lb.PublicNet.IPv6.ID, spec.PrimaryIPv6, @@ -554,10 +551,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. - utils.WarnEventLogf( - l.Recorder, + l.Recorder.Warnf( node, - "UnknownProviderIDPrefix", + events.UnknownProviderIDPrefix, "Node could not be added to Load Balancer for service %s because the provider ID does not match any known format", svc.Name, ) @@ -607,10 +603,9 @@ func (l *LoadBalancerOps) ReconcileHCLBTargets( // Check if InternalIP is set at Node object internalIP := getNodeInternalIP(node) if internalIP == "" { - utils.WarnEventLogf( - l.Recorder, + l.Recorder.Warnf( svc, - "InternalIPNotConfigured", + events.InternalIPNotConfigured, "%s: load balancer %s has set `use-private-ip: true`, but no InternalIP found for node %s. Continuing with ExternalIP.", op, svc.Name, @@ -640,10 +635,9 @@ func (l *LoadBalancerOps) ReconcileHCLBTargets( internalIP := getNodeInternalIP(node) if internalIP == "" { - utils.WarnEventLogf( - l.Recorder, + l.Recorder.Warnf( svc, - "InternalIPNotConfigured", + events.InternalIPNotConfigured, "no InternalIP found for Robot node %s (id=%d), cannot add as LB target without Robot credentials; skipping", node.Name, id, @@ -711,10 +705,9 @@ func (l *LoadBalancerOps) ReconcileHCLBTargets( } if id, replaced := replacedExternalIPs[ip]; replaced { - utils.WarnEventLogf( - l.Recorder, + l.Recorder.Warnf( svc, - "ExternalIPTargetReplaced", + events.ExternalIPTargetReplaced, "removing ExternalIP target %s of node %s, because `use-private-ip: true` is set and its InternalIP %s is used instead", ip, k8sNodes[int64(id)].Name, @@ -759,7 +752,7 @@ func (l *LoadBalancerOps) ReconcileHCLBTargets( node := k8sNodes[id] if numberOfTargets >= lb.LoadBalancerType.MaxTargets { - l.emitMaxTargetsReachedError(node, svc, op) + l.emitMaxTargetsReachedError(node, svc) continue } @@ -768,7 +761,7 @@ func (l *LoadBalancerOps) ReconcileHCLBTargets( a, _, err := l.LBClient.AddServerTarget(ctx, lb, opts) if err != nil { if hcloud.IsError(err, hcloud.ErrorCodeResourceLimitExceeded) { - l.emitMaxTargetsReachedError(node, svc, op) + l.emitMaxTargetsReachedError(node, svc) // Continue loop so that error is emitted for each node continue } @@ -794,13 +787,12 @@ func (l *LoadBalancerOps) ReconcileHCLBTargets( continue } if ip == "" { - l.Recorder.Eventf(node, corev1.EventTypeWarning, "ServerNotFound", "No server with id %d was found in Robot", id) - klog.InfoS("k8s node found but no corresponding server in robot", "id", id) + l.Recorder.Warnf(node, events.ServerNotFound, "No server with id %d was found in Robot", id) continue } if numberOfTargets >= lb.LoadBalancerType.MaxTargets { - l.emitMaxTargetsReachedError(node, svc, op) + l.emitMaxTargetsReachedError(node, svc) continue } @@ -811,7 +803,7 @@ func (l *LoadBalancerOps) ReconcileHCLBTargets( a, _, err := l.LBClient.AddIPTarget(ctx, lb, opts) if err != nil { if hcloud.IsError(err, hcloud.ErrorCodeResourceLimitExceeded) { - l.emitMaxTargetsReachedError(node, svc, op) + l.emitMaxTargetsReachedError(node, svc) continue } return changed, fmt.Errorf("%s: target %s: %w", op, node.Name, err) @@ -827,12 +819,13 @@ func (l *LoadBalancerOps) ReconcileHCLBTargets( return changed, nil } -//nolint:unparam // op might get set to different values in the future -func (l *LoadBalancerOps) emitMaxTargetsReachedError(node *corev1.Node, svc *corev1.Service, op string) { - l.Recorder.Eventf(node, corev1.EventTypeWarning, "MaxTargetsReached", +func (l *LoadBalancerOps) emitMaxTargetsReachedError(node *corev1.Node, svc *corev1.Service) { + l.Recorder.Warnf( + node, + events.MaxTargetsReached, "Node could not be added to Load Balancer for service %s because the max number of targets has been reached", - svc.ObjectMeta.Name) - klog.InfoS("cannot add server target because max number of targets have been reached", "op", op, "service", svc.ObjectMeta.Name, "targetName", node.Name) + svc.ObjectMeta.Name, + ) } // ReconcileHCLBServices synchronizes services exposed by the Hetzner Cloud @@ -868,10 +861,9 @@ func (l *LoadBalancerOps) ReconcileHCLBServices( var action *hcloud.Action if port.Protocol != "" && port.Protocol != corev1.ProtocolTCP { - utils.WarnEventLogf( - l.Recorder, + l.Recorder.Warnf( svc, - "UnsupportedProtocolConfigured", + events.UnsupportedProtocolConfigured, "configured unsupported Hetzner Cloud load balancer protocol %s for service with name %s", port.Protocol, svc.Name, @@ -997,10 +989,9 @@ func (l *LoadBalancerOps) verifyType( ctx = cache.SetSubsystem(ctx, loadBalancerSubsystem) if spec.TypeUnset { - utils.WarnEventLogf( - l.Recorder, + l.Recorder.Warnf( svc, - "LoadBalancerTypeUnconfigured", + events.LoadBalancerTypeUnconfigured, "Load Balancer Type unconfigured: this will be required in the future, set it with the annotation %q or cluster-wide with the environment variable %q", annotation.LBType, config.HcloudLoadBalancersType, @@ -1021,10 +1012,9 @@ func (l *LoadBalancerOps) verifyType( return nil, errors.New(msg) } if msg != "" { - utils.WarnEventLogf( - l.Recorder, + l.Recorder.Warnf( svc, - "LoadBalancerTypeDeprecated", + events.LoadBalancerTypeDeprecated, "%s", msg, ) } diff --git a/internal/hcops/load_balancer_internal_test.go b/internal/hcops/load_balancer_internal_test.go index cb420f3cf..648a85164 100644 --- a/internal/hcops/load_balancer_internal_test.go +++ b/internal/hcops/load_balancer_internal_test.go @@ -17,6 +17,7 @@ import ( "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/annotation" "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/config" + "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/events" "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/lbspec" "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/mocks" "github.com/hetznercloud/hcloud-go/v2/hcloud" @@ -456,7 +457,7 @@ func TestBuildServiceOpts(t *testing.T) { } maps.Copy(svc.Annotations, tt.serviceAnnotations) - spec, err := lbspec.Resolve(record.NewFakeRecorder(10), svc, tt.cfg) + spec, err := lbspec.Resolve(events.NewRecorder(record.NewFakeRecorder(10)), svc, tt.cfg) require.NoError(t, err) lbOps := &LoadBalancerOps{ diff --git a/internal/hcops/load_balancer_test.go b/internal/hcops/load_balancer_test.go index bb85be4da..0cafa99c4 100644 --- a/internal/hcops/load_balancer_test.go +++ b/internal/hcops/load_balancer_test.go @@ -19,8 +19,10 @@ import ( "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/annotation" "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/config" + "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/events" "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/hcops" "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/lbspec" + "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/testsupport" "github.com/hetznercloud/hcloud-go/v2/hcloud" ) @@ -909,17 +911,14 @@ func TestLoadBalancerOps_ReconcileHCLB(t *testing.T) { }, }, mock: func(_ *testing.T, tt *LBReconcilementTestCase) { - tt.fx.LBOps.Recorder = record.NewFakeRecorder(10) + tt.fx.FakeRecorder = record.NewFakeRecorder(10) + tt.fx.LBOps.Recorder = events.NewRecorder(tt.fx.FakeRecorder) }, perform: func(t *testing.T, tt *LBReconcilementTestCase) { changed, err := tt.fx.LBOps.ReconcileHCLB(tt.fx.Ctx, tt.initialLB, tt.service, tt.spec) assert.NoError(t, err) assert.False(t, changed) - select { - case event := <-tt.fx.LBOps.Recorder.(*record.FakeRecorder).Events: - t.Errorf("unexpected event: %s", event) - default: - } + testsupport.AssertEvents(t, tt.fx.FakeRecorder) }, }, { @@ -941,17 +940,14 @@ func TestLoadBalancerOps_ReconcileHCLB(t *testing.T) { }, }, mock: func(_ *testing.T, tt *LBReconcilementTestCase) { - tt.fx.LBOps.Recorder = record.NewFakeRecorder(10) + tt.fx.FakeRecorder = record.NewFakeRecorder(10) + tt.fx.LBOps.Recorder = events.NewRecorder(tt.fx.FakeRecorder) }, perform: func(t *testing.T, tt *LBReconcilementTestCase) { changed, err := tt.fx.LBOps.ReconcileHCLB(tt.fx.Ctx, tt.initialLB, tt.service, tt.spec) assert.NoError(t, err) assert.False(t, changed) - select { - case event := <-tt.fx.LBOps.Recorder.(*record.FakeRecorder).Events: - t.Errorf("unexpected event: %s", event) - default: - } + testsupport.AssertEvents(t, tt.fx.FakeRecorder) }, }, { @@ -973,23 +969,18 @@ func TestLoadBalancerOps_ReconcileHCLB(t *testing.T) { }, }, mock: func(_ *testing.T, tt *LBReconcilementTestCase) { - tt.fx.LBOps.Recorder = record.NewFakeRecorder(10) + tt.fx.FakeRecorder = record.NewFakeRecorder(10) + tt.fx.LBOps.Recorder = events.NewRecorder(tt.fx.FakeRecorder) }, perform: func(t *testing.T, tt *LBReconcilementTestCase) { changed, err := tt.fx.LBOps.ReconcileHCLB(tt.fx.Ctx, tt.initialLB, tt.service, tt.spec) assert.NoError(t, err) assert.False(t, changed) - - events := tt.fx.LBOps.Recorder.(*record.FakeRecorder).Events - assert.Equal( + testsupport.AssertEvents( t, + tt.fx.FakeRecorder, "Warning PrimaryIPChangeUnsupported Load Balancer has Primary IPv4 4711, changing it to 4713 is not supported", - <-events, - ) - assert.Equal( - t, "Warning PrimaryIPChangeUnsupported Load Balancer has Primary IPv6 4712, changing it to 4714 is not supported", - <-events, ) }, }, @@ -1894,19 +1885,17 @@ func TestLoadBalancerOps_ReconcileHCLBTargets(t *testing.T) { action = tt.fx.MockAddIPTarget(tt.initialLB, optsIP, nil) tt.fx.ActionClient.On("WaitFor", tt.fx.Ctx, action).Return(nil) - tt.fx.LBOps.Recorder = record.NewFakeRecorder(10) + tt.fx.FakeRecorder = record.NewFakeRecorder(10) + tt.fx.LBOps.Recorder = events.NewRecorder(tt.fx.FakeRecorder) }, perform: func(t *testing.T, tt *LBReconcilementTestCase) { changed, err := tt.fx.LBOps.ReconcileHCLBTargets(tt.fx.Ctx, tt.initialLB, tt.service, tt.spec, tt.k8sNodes) assert.NoError(t, err) assert.True(t, changed) - - events := tt.fx.LBOps.Recorder.(*record.FakeRecorder).Events - assert.Len(t, events, 1) - assert.Equal( + testsupport.AssertEvent( t, + tt.fx.FakeRecorder, "Warning ExternalIPTargetReplaced removing ExternalIP target 1.2.3.4 of node robot-3, because `use-private-ip: true` is set and its InternalIP 10.0.1.10 is used instead", - <-events, ) }, }, diff --git a/internal/hcops/testing.go b/internal/hcops/testing.go index 363d1e4e8..15c095e9e 100644 --- a/internal/hcops/testing.go +++ b/internal/hcops/testing.go @@ -14,6 +14,7 @@ import ( "k8s.io/client-go/tools/record" "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/cache" + "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/events" "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/lbspec" "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/mocks" "github.com/hetznercloud/hcloud-go/v2/hcloud" @@ -38,6 +39,7 @@ type LoadBalancerOpsFixture struct { ActionClient *mocks.ActionClient NetworkClient *mocks.NetworkClient RobotClient *mocks.RobotClient + FakeRecorder *record.FakeRecorder LBOps *LoadBalancerOps @@ -52,6 +54,7 @@ func NewLoadBalancerOpsFixture(t *testing.T) *LoadBalancerOpsFixture { CertClient: &mocks.CertificateClient{}, NetworkClient: &mocks.NetworkClient{}, RobotClient: &mocks.RobotClient{}, + FakeRecorder: &record.FakeRecorder{}, T: t, } @@ -68,7 +71,7 @@ func NewLoadBalancerOpsFixture(t *testing.T) *LoadBalancerOpsFixture { NetworkClient: fx.NetworkClient, RobotClient: fx.RobotClient, LBTypeCache: newLBTypeCacheFixture(t), - Recorder: &record.FakeRecorder{}, + Recorder: events.NewRecorder(fx.FakeRecorder), } return fx diff --git a/internal/instance/instances.go b/internal/instance/instances.go index cbfb2c1df..5d4dfd13e 100644 --- a/internal/instance/instances.go +++ b/internal/instance/instances.go @@ -27,35 +27,31 @@ import ( hrobot "github.com/syself/hrobot-go" hrobotmodels "github.com/syself/hrobot-go/models" corev1 "k8s.io/api/core/v1" - "k8s.io/client-go/tools/record" cloudprovider "k8s.io/cloud-provider" "k8s.io/klog/v2" "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/annotation" "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/cache" "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/config" + "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/events" "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" ) const ( - ProvidedBy = "instance.hetzner.cloud/provided-by" - providedByCloud = "cloud" - providedByRobot = "robot" - MisconfiguredInternalIP = "MisconfiguredInternalIP" - IgnoredExternalIPv6 = "IgnoredExternalIPv6" - InvalidExternalIPv6 = "InvalidExternalIPv6" - instancesV2Subsystem = "instances_v2" + ProvidedBy = "instance.hetzner.cloud/provided-by" + providedByCloud = "cloud" + providedByRobot = "robot" + instancesV2Subsystem = "instances_v2" ) type instances struct { client *hcloud.Client robotClient hrobot.RobotClient serverCache *cache.Cache[hcloud.Server] - recorder record.EventRecorder + recorder *events.Recorder networkID int64 cfg config.HCCMConfiguration } @@ -71,7 +67,7 @@ func New( client *hcloud.Client, robotClient hrobot.RobotClient, serverCache *cache.Cache[hcloud.Server], - recorder record.EventRecorder, + recorder *events.Recorder, networkID int64, cfg config.HCCMConfiguration, ) cloudprovider.InstancesV2 { @@ -156,10 +152,9 @@ func (i *instances) getServer( } if cloudServer != nil && hrobotServer != nil { - utils.WarnEventLogf( - i.recorder, + i.recorder.Warnf( node, - "InstanceLookupFailed", + events.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, ) @@ -245,7 +240,7 @@ func hcloudNodeAddresses( server *hcloud.Server, node *corev1.Node, cfg config.HCCMConfiguration, - recorder record.EventRecorder, + recorder *events.Recorder, ) ([]corev1.NodeAddress, error) { var addresses []corev1.NodeAddress addresses = append( @@ -299,7 +294,7 @@ func robotNodeAddresses( server *hrobotmodels.Server, node *corev1.Node, cfg config.HCCMConfiguration, - recorder record.EventRecorder, + recorder *events.Recorder, ) ([]corev1.NodeAddress, error) { family := cfg.Instance.AddressFamily dualStack := family == config.AddressFamilyDualStack @@ -334,14 +329,13 @@ func externalIPv6( ipv6 bool, subnet string, node *corev1.Node, - recorder record.EventRecorder, + recorder *events.Recorder, ) (string, error) { if !ipv6 { if _, ok := node.GetAnnotations()[string(annotation.ExternalIPv6)]; ok { - utils.WarnEventLogf( - recorder, + recorder.Warnf( node, - IgnoredExternalIPv6, + events.IgnoredExternalIPv6, "The annotation %s is set, but IPv6 is not enabled for Node addresses. As a result, it is ignored", annotation.ExternalIPv6, ) @@ -387,11 +381,10 @@ func externalIPv6( // invalidExternalIPv6 reports err to the user as well, who would otherwise only see the Node fail // to initialize: the cloud node controller logs the error without emitting an event. -func invalidExternalIPv6(recorder record.EventRecorder, node *corev1.Node, err error) error { - utils.WarnEventLogf( - recorder, +func invalidExternalIPv6(recorder *events.Recorder, node *corev1.Node, err error) error { + recorder.Warnf( node, - InvalidExternalIPv6, + events.InvalidExternalIPv6, "Invalid Node annotation: %v. As a result, the Node is not initialized", err, ) @@ -417,7 +410,7 @@ func appendForwardedInternalIPs( addresses []corev1.NodeAddress, node *corev1.Node, family config.AddressFamily, - recorder record.EventRecorder, + recorder *events.Recorder, ) []corev1.NodeAddress { for _, internalIP := range node.Status.Addresses { if internalIP.Type != corev1.NodeInternalIP { @@ -431,26 +424,23 @@ func appendForwardedInternalIPs( switch { case isIPv4 && family == config.AddressFamilyIPv6: - utils.WarnEventLogf( - recorder, + recorder.Warnf( node, - MisconfiguredInternalIP, + events.MisconfiguredInternalIP, "Configured InternalIP is IPv4 even though IPv6 only is configured. As a result, %s is not added as an InternalIP", internalIP.Address, ) case !isIPv4 && family == config.AddressFamilyIPv4: - utils.WarnEventLogf( - recorder, + recorder.Warnf( node, - MisconfiguredInternalIP, + events.MisconfiguredInternalIP, "Configured InternalIP is IPv6 even though IPv4 only is configured. As a result, %s is not added as an InternalIP", internalIP.Address, ) case isKnown: - utils.WarnEventLogf( - recorder, + recorder.Warnf( node, - MisconfiguredInternalIP, + events.MisconfiguredInternalIP, "Configured InternalIP already exists as an ExternalIP. As a result, %s is not added as an InternalIP", internalIP.Address, ) @@ -473,7 +463,7 @@ type genericServer interface { type hcloudServer struct { *hcloud.Server - recorder record.EventRecorder + recorder *events.Recorder } func (s hcloudServer) IsShutdown() (bool, error) { @@ -508,7 +498,7 @@ func (s hcloudServer) Metadata(networkID int64, node *corev1.Node, cfg config.HC type robotServer struct { *hrobotmodels.Server robotClient hrobot.RobotClient - recorder record.EventRecorder + recorder *events.Recorder } func (s robotServer) IsShutdown() (bool, error) { diff --git a/internal/instance/instances_test.go b/internal/instance/instances_test.go index 30627f675..79bd64860 100644 --- a/internal/instance/instances_test.go +++ b/internal/instance/instances_test.go @@ -34,7 +34,9 @@ import ( "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/annotation" "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/config" + "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/events" "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/testenv" + "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/testsupport" "github.com/hetznercloud/hcloud-go/v2/hcloud" "github.com/hetznercloud/hcloud-go/v2/hcloud/schema" ) @@ -466,25 +468,6 @@ func TestInstances_InstanceMetadataRobotServerInvalidExternalIPv6(t *testing.T) assert.Nil(t, metadata) } -// assertEvent asserts that recorder recorded expected, or nothing at all if expected is empty. -func assertEvent(t *testing.T, recorder *record.FakeRecorder, expected string) { - t.Helper() - - close(recorder.Events) - - var events []string - for event := range recorder.Events { - events = append(events, event) - } - - if expected == "" { - assert.Empty(t, events) - return - } - - assert.Equal(t, []string{expected}, events) -} - func TestExternalIPv6(t *testing.T) { const subnet = "2001:db8:1234::" @@ -603,9 +586,9 @@ func TestExternalIPv6(t *testing.T) { recorder := record.NewFakeRecorder(10) - hostAddress, err := externalIPv6(test.ipv6Configured, test.ipv6Subnet, node, recorder) + hostAddress, err := externalIPv6(test.ipv6Configured, test.ipv6Subnet, node, events.NewRecorder(recorder)) - assertEvent(t, recorder, test.expectedEvent) + testsupport.AssertEvent(t, recorder, test.expectedEvent) if test.expectedErr != "" { assert.EqualError(t, err, test.expectedErr) @@ -831,9 +814,9 @@ func TestNodeAddresses(t *testing.T) { recorder := record.NewFakeRecorder(10) - addresses, err := hcloudNodeAddresses(test.privateNetwork, test.server, node, cfg, recorder) + addresses, err := hcloudNodeAddresses(test.privateNetwork, test.server, node, cfg, events.NewRecorder(recorder)) - assertEvent(t, recorder, test.expectedEvent) + testsupport.AssertEvent(t, recorder, test.expectedEvent) if test.expectedErr != "" { assert.EqualError(t, err, test.expectedErr) @@ -1029,9 +1012,9 @@ func TestNodeAddressesRobotServer(t *testing.T) { recorder := record.NewFakeRecorder(10) - addresses, err := robotNodeAddresses(test.server, node, cfg, recorder) + addresses, err := robotNodeAddresses(test.server, node, cfg, events.NewRecorder(recorder)) - assertEvent(t, recorder, test.expectedEvent) + testsupport.AssertEvent(t, recorder, test.expectedEvent) if test.expectedErr != "" { assert.EqualError(t, err, test.expectedErr) diff --git a/internal/instance/instances_util.go b/internal/instance/instances_util.go index 903010c9c..1bd458c90 100644 --- a/internal/instance/instances_util.go +++ b/internal/instance/instances_util.go @@ -25,7 +25,7 @@ import ( hrobotmodels "github.com/syself/hrobot-go/models" corev1 "k8s.io/api/core/v1" - "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/utils" + "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/events" ) func getRobotServerByName(c hrobot.RobotClient, node *corev1.Node) (server *hrobotmodels.Server, err error) { @@ -69,10 +69,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 { - utils.WarnEventLogf( - i.recorder, + i.recorder.Warnf( node, - "PossibleNodeDeletion", + events.PossibleNodeDeletion, "Might be deleted by node-lifecycle-manager due to name mismatch; Node name %q differs from Robot name %q", node.ObjectMeta.Name, server.Name, diff --git a/internal/instance/instances_util_test.go b/internal/instance/instances_util_test.go index f2d92a493..f53a149a1 100644 --- a/internal/instance/instances_util_test.go +++ b/internal/instance/instances_util_test.go @@ -10,7 +10,9 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/tools/record" + "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/events" "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/mocks" + "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/testsupport" ) func TestGetRobotServerByID(t *testing.T) { @@ -39,7 +41,7 @@ func TestGetRobotServerByID(t *testing.T) { robotClientMock.On("ServerGet").Return(&hrobotmodels.Server{ServerNumber: 1, Name: "foobar"}, nil) inst := &instances{ - recorder: recorder, + recorder: events.NewRecorder(recorder), robotClient: robotClientMock, } @@ -54,12 +56,11 @@ func TestGetRobotServerByID(t *testing.T) { if tt.expectedEvent != "" { require.Nil(t, server) - event := <-recorder.Events - assert.Equal(t, tt.expectedEvent, event) } else { assert.Equal(t, "foobar", server.Name) - assert.Empty(t, recorder.Events) } + + testsupport.AssertEvent(t, recorder, tt.expectedEvent) }) } } diff --git a/internal/lbspec/solver.go b/internal/lbspec/solver.go index d65f68037..c91d0f3b1 100644 --- a/internal/lbspec/solver.go +++ b/internal/lbspec/solver.go @@ -7,12 +7,11 @@ import ( corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/labels" - "k8s.io/client-go/tools/record" cloudprovider "k8s.io/cloud-provider" - "k8s.io/klog/v2" "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/annotation" "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/config" + "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/events" "github.com/hetznercloud/hcloud-go/v2/hcloud" ) @@ -32,7 +31,7 @@ func Name(svc *corev1.Service) string { // All invalid annotations are reported together in the returned error. The // returned Spec is still fully populated, with the offending settings left at // their fallback, so callers must not use it when err is non-nil. -func Resolve(recorder record.EventRecorder, svc *corev1.Service, cfg config.LoadBalancerConfiguration) (Spec, error) { +func Resolve(recorder *events.Recorder, svc *corev1.Service, cfg config.LoadBalancerConfiguration) (Spec, error) { var errs []error var spec Spec @@ -79,16 +78,9 @@ func Resolve(recorder record.EventRecorder, svc *corev1.Service, cfg config.Load } for _, err := range errs { - klog.ErrorS( - err, - "invalid Load Balancer annotation", - "service", - klog.KObj(svc), - ) - recorder.Eventf( + recorder.Errorf( svc, - corev1.EventTypeWarning, - "InvalidLoadBalancerAnnotation", + events.InvalidLoadBalancerAnnotation, "Invalid Load Balancer annotation %v", err, ) diff --git a/internal/lbspec/solver_test.go b/internal/lbspec/solver_test.go index 87be623bc..6fd13a9db 100644 --- a/internal/lbspec/solver_test.go +++ b/internal/lbspec/solver_test.go @@ -15,6 +15,7 @@ import ( "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/annotation" "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/config" + "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/events" "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/lbspec" "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/testsupport" "github.com/hetznercloud/hcloud-go/v2/hcloud" @@ -346,7 +347,7 @@ func TestResolve(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - spec, err := lbspec.Resolve(record.NewFakeRecorder(10), service("some-uid", tt.annotations), tt.cfg) + spec, err := lbspec.Resolve(events.NewRecorder(record.NewFakeRecorder(10)), service("some-uid", tt.annotations), tt.cfg) require.NoError(t, err) tt.check(t, spec) }) @@ -365,7 +366,7 @@ func TestResolveReportsEveryInvalidAnnotation(t *testing.T) { logs := testsupport.CaptureKlog(t) recorder := record.NewFakeRecorder(10) - _, err := lbspec.Resolve(recorder, svc, config.LoadBalancerConfiguration{}) + _, err := lbspec.Resolve(events.NewRecorder(recorder), svc, config.LoadBalancerConfiguration{}) // The error only counts the invalid annotations, the details are reported // through the logs and the Kubernetes events. @@ -431,7 +432,7 @@ func TestResolveErrors(t *testing.T) { logs := testsupport.CaptureKlog(t) recorder := record.NewFakeRecorder(10) - _, err := lbspec.Resolve(recorder, service("some-uid", tt.annotations), config.LoadBalancerConfiguration{}) + _, err := lbspec.Resolve(events.NewRecorder(recorder), service("some-uid", tt.annotations), config.LoadBalancerConfiguration{}) require.EqualError(t, err, "1 Load Balancer annotation(s) are invalid") diff --git a/internal/routes/routes.go b/internal/routes/routes.go index af6564b8f..e834a05e4 100644 --- a/internal/routes/routes.go +++ b/internal/routes/routes.go @@ -11,14 +11,13 @@ import ( apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/types" corelisters "k8s.io/client-go/listers/core/v1" - "k8s.io/client-go/tools/record" cloudprovider "k8s.io/cloud-provider" "k8s.io/klog/v2" "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/cache" + "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/events" "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" ) @@ -34,11 +33,11 @@ type routes struct { network *hcloud.Network serverCache *cache.Cache[hcloud.Server] clusterCIDR *net.IPNet - recorder record.EventRecorder + recorder *events.Recorder nodeLister corelisters.NodeLister } -func New(client *hcloud.Client, networkID int64, clusterCIDR string, recorder record.EventRecorder, nodeLister corelisters.NodeLister, serverCache *cache.Cache[hcloud.Server]) (cloudprovider.Routes, error) { +func New(client *hcloud.Client, networkID int64, clusterCIDR string, recorder *events.Recorder, nodeLister corelisters.NodeLister, serverCache *cache.Cache[hcloud.Server]) (cloudprovider.Routes, error) { const op = "hcloud/newRoutes" metrics.OperationCalled.WithLabelValues(op).Inc() @@ -325,10 +324,9 @@ func (r *routes) warnCIDRMismatch(cidr *net.IPNet, node *corev1.Node) { destPrefixLen, _ := cidr.Mask.Size() if !r.clusterCIDR.Contains(cidr.IP) || destPrefixLen < clusterPrefixLen { - utils.WarnEventLogf( - r.recorder, + r.recorder.Warnf( node, - "ClusterCIDRMisconfigured", + events.ClusterCIDRMisconfigured, "route CIDR %s is not contained within cluster CIDR %s", cidr.String(), r.clusterCIDR.String(), diff --git a/internal/testenv/testenv.go b/internal/testenv/testenv.go index 0d2939717..2d35c340d 100644 --- a/internal/testenv/testenv.go +++ b/internal/testenv/testenv.go @@ -13,6 +13,7 @@ import ( "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/cache" "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/config" + "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/events" "github.com/hetznercloud/hcloud-go/v2/hcloud" ) @@ -24,7 +25,7 @@ type TestEnv struct { Client *hcloud.Client RobotClient hrobot.RobotClient ServerCache *cache.Cache[hcloud.Server] - Recorder record.EventRecorder + Recorder *events.Recorder Cfg config.HCCMConfiguration } @@ -53,7 +54,7 @@ func New() TestEnv { robotClient := hrobot.NewBasicAuthClient("", "") robotClient.SetBaseURL(server.URL + "/robot") serverCache := cache.NewServerCache(client, cache.ModeOne, 10*time.Second) - recorder := record.NewBroadcaster().NewRecorder(scheme.Scheme, corev1.EventSource{Component: "hcloud-cloud-controller-manager"}) + recorder := events.NewRecorder(record.NewBroadcaster().NewRecorder(scheme.Scheme, corev1.EventSource{Component: "hcloud-cloud-controller-manager"})) cfg := config.HCCMConfiguration{} cfg.Instance.AddressFamily = config.AddressFamilyIPv4 diff --git a/internal/testsupport/events.go b/internal/testsupport/events.go new file mode 100644 index 000000000..b2fa785d9 --- /dev/null +++ b/internal/testsupport/events.go @@ -0,0 +1,34 @@ +package testsupport + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "k8s.io/client-go/tools/record" +) + +// AssertEvent asserts that recorder recorded expected, or nothing at all if expected is empty. +func AssertEvent(t *testing.T, recorder *record.FakeRecorder, expected string) { + t.Helper() + + if expected == "" { + AssertEvents(t, recorder) + return + } + + AssertEvents(t, recorder, expected) +} + +// AssertEvents asserts that recorder recorded exactly the expected events in order. +func AssertEvents(t *testing.T, recorder *record.FakeRecorder, expected ...string) { + t.Helper() + + close(recorder.Events) + + var events []string + for event := range recorder.Events { + events = append(events, event) + } + + assert.Equal(t, expected, events) +} diff --git a/internal/utils/eventlog.go b/internal/utils/eventlog.go deleted file mode 100644 index 088ef3558..000000000 --- a/internal/utils/eventlog.go +++ /dev/null @@ -1,16 +0,0 @@ -package utils - -import ( - "fmt" - - corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/runtime" - "k8s.io/client-go/tools/record" - "k8s.io/klog/v2" -) - -func WarnEventLogf(recorder record.EventRecorder, obj runtime.Object, reason string, msg string, args ...any) { - msgf := fmt.Sprintf(msg, args...) - recorder.Event(obj, corev1.EventTypeWarning, reason, msgf) - klog.Warning(msgf) -} diff --git a/internal/utils/eventlog_test.go b/internal/utils/eventlog_test.go deleted file mode 100644 index 099450f42..000000000 --- a/internal/utils/eventlog_test.go +++ /dev/null @@ -1,79 +0,0 @@ -package utils - -import ( - "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" - - "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/testsupport" -) - -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 := testsupport.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 prefix `W` for warning - assert.Regexp(t, `^W\d`, logs.String()) - assert.Contains(t, logs.String(), tt.expected) - }) - } -} - -func TestWarnEventLogfEventObject(t *testing.T) { - testsupport.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, - ) -}