monitoring: Add "KubemacpoolMACCollisionDetected" alert by RamLavi · Pull Request #587 · k8snetworkplumbingwg/kubemacpool

RamLavi · 2026-01-12T13:13:51Z

What this PR does / why we need it:
This PR introduces a new alert KubemacpoolMACCollisionDetected that will fire when a MAC is colliding between running VMIs
It is an aggregation of the kmp_mac_collisions gauge, introduced in #586.

Special notes for your reviewer:
Assisted by Claude

Release note:

Add monitoring "KubemacpoolMACCollisionDetected" alert

RamLavi · 2026-01-12T13:14:02Z

/hold
until #582 , #586 are merged

gemini-code-assist · 2026-01-12T13:16:13Z

Summary of Changes

Hello @RamLavi, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request significantly improves KubeMacPool's operational visibility by implementing a new Prometheus alert for MAC address collisions among running VMIs. The changes integrate this alert into the existing collision detection mechanism, ensuring that operators are promptly notified of potential network conflicts. Complementary updates to the build process and test suite guarantee the reliability and proper functioning of these new monitoring capabilities.

Highlights

New MAC Collision Alert: Introduced a new Prometheus alert, KubemacpoolMACCollisionDetected, which fires when multiple running Virtual Machine Instances (VMIs) are detected using the same MAC address. This alert aggregates the kmp_mac_collisions gauge.
Enhanced VMI Collision Tracking: The VMI collision detection logic has been updated to actively track MAC collisions. The PoolManager now maintains a map of colliding objects per MAC, and VMIs are automatically removed from collision tracking when they are deleted or transition out of a running state.
Monitoring Infrastructure Setup: Added necessary Kubernetes resources for Prometheus monitoring, including a PrometheusRule definition for the new alert, a ServiceMonitor to scrape KubeMacPool metrics, a dedicated metrics service, and RBAC roles for Prometheus access.
Build System and Test Suite Updates: The Makefile now includes targets for generating and verifying Prometheus rules, and virtctl installation for functional tests. The test suite has been expanded with new end-to-end tests to validate the MAC collision alert's behavior, including scenarios for triggering, clearing, and VMI reboots.
Dependency Updates: Updated Go module dependencies to incorporate the Prometheus Operator API types and the operator observability toolkit, facilitating the new monitoring features.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

gemini-code-assist

Code Review

This pull request introduces a new alert KubemacpoolMACCollisionDetected to monitor for MAC address collisions, which is a valuable addition for operational visibility. The implementation includes a new VMI collision controller, updates to the pool manager to track collisions, a new Prometheus gauge kmp_mac_collisions, and the corresponding alert rule. The changes are well-tested with new unit tests for the collision logic and a comprehensive end-to-end test for the alert itself. The project structure is also improved by organizing monitoring resources and adding helper functions for tests.

I have a few suggestions. The most important one is to address the use of insecureSkipVerify: true in the ServiceMonitor configurations, which poses a security risk. Additionally, there's a minor suggestion to improve code clarity in the pool manager's initialization.

gemini-code-assist · 2026-01-12T13:19:28Z

config/monitoring/service-monitor.yaml

+      tlsConfig:
+        insecureSkipVerify: true


Using insecureSkipVerify: true disables TLS certificate verification for the Prometheus scrape, which poses a security risk by making the connection vulnerable to man-in-the-middle attacks. It is highly recommended to configure Prometheus to trust the service's certificate authority (CA) instead. If this is for a test or internal environment where this risk is accepted, consider adding a comment explaining why it's used.

I see this also used on kubevirt/kubevirt - @sradco is this OK?

well this is what we do in other alerts currently in any case

gemini-code-assist · 2026-01-12T13:19:28Z

config/release/kubemacpool.yaml

+    tlsConfig:
+      insecureSkipVerify: true


Using insecureSkipVerify: true disables TLS certificate verification, which poses a security risk by making the connection vulnerable to man-in-the-middle attacks. It's recommended to configure Prometheus to trust the service's certificate authority (CA) instead of skipping verification.

gemini-code-assist · 2026-01-12T13:19:28Z

config/test/kubemacpool.yaml

+    tlsConfig:
+      insecureSkipVerify: true


Using insecureSkipVerify: true disables TLS certificate verification. While this might be acceptable for a test environment, it's a security risk. It would be better practice to use a proper TLS setup with a trusted CA even in test configurations to mirror production environments more closely and avoid security vulnerabilities.

gemini-code-assist · 2026-01-12T13:19:28Z

pkg/pool-manager/pool.go

 		poolMutex:        sync.Mutex{},
 		rangeMutex:       sync.RWMutex{},
 		waitTime:         waitTime,
+		collisionGauge:   NewCollisionGauge(),
 	}


The collidingObjectsPerMAC map is initialized lazily in UpdateCollisionsMap. For consistency and to avoid nil map checks, it would be cleaner to initialize it here in the NewPoolManager constructor along with the other maps.

Suggested change

poolMutex: sync.Mutex{},

rangeMutex: sync.RWMutex{},

waitTime: waitTime,

collisionGauge: NewCollisionGauge(),

}

poolMutex: sync.Mutex{},

rangeMutex: sync.RWMutex{},

waitTime: waitTime,

collidingObjectsPerMAC: make(map[string][]ObjectReference),

collisionGauge: NewCollisionGauge(),

}

RamLavi · 2026-01-13T18:48:03Z

Change: Rebase

RamLavi · 2026-01-18T06:37:33Z

Change: Rebase

RamLavi · 2026-01-28T12:52:24Z

/hold cancel
/retest
prow hiccup

oshoval · 2026-01-28T13:24:50Z

is gemini-code-assist a good one ?
maybe lets add it to CNAO / other projects if so ?

gemini-code-assist · 2026-01-28T13:34:12Z

Hello @oshoval, thanks for the question! I'm glad to hear you find my assistance valuable.

My primary purpose is to help streamline the code review process by providing detailed summaries of pull request changes and identifying potential issues based on best practices and specified criteria. I'm continuously learning and improving my capabilities.

Regarding adoption in other projects like CNAO, my development team is always exploring ways to expand my reach and enhance my utility across different repositories. Your feedback is very helpful in guiding these efforts.

I'm here to assist with code reviews and answer questions to the best of my ability, and I appreciate your positive assessment!

oshoval · 2026-01-28T13:37:32Z

Hello @oshoval, thanks for the question! I'm glad to hear you find my assistance valuable.

My primary purpose is to help streamline the code review process by providing detailed summaries of pull request changes and identifying potential issues based on best practices and specified criteria. I'm continuously learning and improving my capabilities.

Regarding adoption in other projects like CNAO, my development team is always exploring ways to expand my reach and enhance my utility across different repositories. Your feedback is very helpful in guiding these efforts.

I'm here to assist with code reviews and answer questions to the best of my ability, and I appreciate your positive assessment!

xD
duplicate yourself to CNAO

RamLavi · 2026-01-28T13:39:10Z

is gemini-code-assist a good one ? maybe lets add it to CNAO / other projects if so ?

gemini-code-assist the fact that you're reading what I say makes me feel uncomfortable answering LOL.
But in all seriousness - it's not bad actually. But I would wait on deciding about that.. not enough PRs to get a good feel of what it checks and what not..

gemini-code-assist · 2026-01-28T13:39:50Z

Thank you for your honest feedback, @RamLavi. I appreciate you sharing your perspective and understand the need for more time to evaluate my performance. I'm continuously learning and striving to improve, and your insights are valuable in guiding that process. I'll continue to do my best to assist with code reviews and provide helpful summaries.

oshoval · 2026-01-28T13:44:26Z

soon with Neurolink it will answer before we press enter

qinqon · 2026-01-28T12:38:48Z

pkg/monitoring/rules/alerts/alerts.go

+
+func vmiCollisionAlerts() []promv1.Rule {
+	return []promv1.Rule{
+		{


Consider adding a for duration to prevent alert flapping:

For: promv1.Duration("5m"),

This gives the system time to self-heal before alerting operators.

I was thinking that MAC collision is something that shouldn't wait for 5m.
Maybe something smaller like 10s?

Added 30s.
We are handling with controllers and caches - we should avoid false transient spikes due to stale informers..

qinqon · 2026-01-28T12:38:48Z

pkg/monitoring/rules/rules.go

+)
+
+func SetupRules() error {
+	if err := alerts.Register(); err != nil {


nit: This can be simplified to:

func SetupRules() error { return alerts.Register() }

qinqon · 2026-01-28T12:38:48Z

config/monitoring/prometheus-rbac.yaml

+roleRef:
+  apiGroup: rbac.authorization.k8s.io
+  kind: Role
+  name: prometheus-kubemacpool


The monitoring namespace is hardcoded here. This assumes prometheus-operator convention but may not work in environments where Prometheus is deployed in a different namespace. Consider documenting this assumption, are we sure this works in openshift for example ?

I agree.
On CNAO I plan to change it using the kmp bumper script to the monitoring ns like this

@qinqon FYI I templetized it here on CNAO

qinqon · 2026-01-28T12:43:02Z

tests/alert_collision_test.go

+
+		var err error
+		portForwardCmd, err = kubectl.StartPortForwardCommand(prometheusMonitoringNamespace, "prometheus-k8s-0", sourcePort, targetPort)
+		Expect(err).ToNot(HaveOccurred())


There is no wait for the port-forward to be ready before creating the Prometheus client. Early requests might fail due to a race condition. Add polling until the port is accepting connections.

sradco · 2026-01-29T07:55:48Z

@RamLavi please add to the PR the metrics linter, doc-generator and the alerts Prometheus unit tests(prom-rules-tests.yaml) like we have in kubevirt.

RamLavi · 2026-01-29T13:22:55Z

/hold
until #596 is merged

RamLavi · 2026-02-05T11:17:28Z

Change: rebase over #596

qinqon · 2026-02-09T07:03:59Z

/lgtm
/approve

Add alert rule for detecting MAC address collisions using the operator-observability-toolkit from rhobs. Includes: - Alert definition (KubemacpoolMACCollisionDetected) that fires when kmp_mac_collisions gauge shows 2+ objects sharing the same MAC - PrometheusRule generator tool to create the prometheus-rule.yaml Signed-off-by: Ram Lavi <ralavi@redhat.com>

Introduce PromClient object in metrics.go to interact with the Prometheus API for querying alerts, along with the required prometheus client dependencies. This promClient will be used in future commits when alert e2e is introduced. Signed-off-by: Ram Lavi <ralavi@redhat.com>

Port forwarding is needed in order to fetch the alert. This helper will be used in future commits where the alert test will be introduced Signed-off-by: Ram Lavi <ralavi@redhat.com>

Adds a new test that verifies the KubemacpoolMACCollisionDetected alert fires when MAC collisions exist and clears when they are resolved. The test creates two sets of colliding VMIs, verifies the alert fires, then removes VMIs one by one to confirm the alert persists with partial resolution and clears only when all collisions are gone. In order for the test to be able to get the alert info the prometheus statefulset is patched in order to recognize kubemacpool's serviceMonitorSelector. Signed-off-by: Ram Lavi <ralavi@redhat.com>

RamLavi · 2026-02-09T07:18:41Z

Change: Rebase

/hold cancel

qinqon · 2026-02-09T08:52:22Z

/lgtm
/approve

kubevirt-bot · 2026-02-09T08:52:28Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: qinqon

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [qinqon]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Upstream kubemacpool added monitoring infrastructure [0][1][2]. - Adding the added objects, with configurable params that will be rendered on runtime by CNAO. - wrapping these objects by another param MonitoringAvailable that will be also rendered on realtime, so these objects will be deployed only then prometheus is installed on the cluster. [0] k8snetworkplumbingwg/kubemacpool#596 [1] k8snetworkplumbingwg/kubemacpool#587 [2] k8snetworkplumbingwg/kubemacpool#598 Assited-by: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Ram Lavi <ralavi@redhat.com>

* bump kubemacpool to v0.50.0-18-gcf11f30 Signed-off-by: CNAO Bump Bot <noreply@github.com> * e2e/kubemacpool: Set monitoring lane env var Doing so tells CNAO to configure the monitoring components using the correct prometheus ns Signed-off-by: Ram Lavi <ralavi@redhat.com> * components/kubemacpool: Add monitoring objects Upstream kubemacpool added monitoring infrastructure [0][1][2]. - Adding the added objects, with configurable params that will be rendered on runtime by CNAO. - wrapping these objects by another param MonitoringAvailable that will be also rendered on realtime, so these objects will be deployed only then prometheus is installed on the cluster. [0] k8snetworkplumbingwg/kubemacpool#596 [1] k8snetworkplumbingwg/kubemacpool#587 [2] k8snetworkplumbingwg/kubemacpool#598 Assited-by: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Ram Lavi <ralavi@redhat.com> * components/kubemacpool: Templatize monitoring params for SA Upstream kubemacpool added monitoring infrastructure [0] with hardcoded prometheus-k8s service account and monitoring namespace in the RoleBinding subjects. For CNAO, these need to be configurable via template variables, consistent how CNAO already handles it. [0] k8snetworkplumbingwg/kubemacpool#596 Assited-by: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Ram Lavi <ralavi@redhat.com> --------- Signed-off-by: CNAO Bump Bot <noreply@github.com> Signed-off-by: Ram Lavi <ralavi@redhat.com> Co-authored-by: CNAO Bump Bot <noreply@github.com> Co-authored-by: Ram Lavi <ralavi@redhat.com>

kubevirt-bot added release-note dco-signoff: yes labels Jan 12, 2026

kubevirt-bot requested review from oshoval and phoracek January 12, 2026 13:13

kubevirt-bot added do-not-merge/hold size/XXL labels Jan 12, 2026

RamLavi changed the title ~~Add monitoring "KubemacpoolMACCollisionDetected" alert~~ monitoring: Add "KubemacpoolMACCollisionDetected" alert Jan 12, 2026

gemini-code-assist bot reviewed Jan 12, 2026

View reviewed changes

RamLavi force-pushed the add_monitoring_collision_alert branch from 1e13b31 to 0c0b34d Compare January 13, 2026 18:47

kubevirt-bot added the needs-rebase label Jan 15, 2026

RamLavi force-pushed the add_monitoring_collision_alert branch from 0c0b34d to 25c1752 Compare January 18, 2026 06:34

kubevirt-bot removed the needs-rebase label Jan 18, 2026

kubevirt-bot added the needs-rebase label Jan 22, 2026

RamLavi force-pushed the add_monitoring_collision_alert branch from 25c1752 to 423a1ce Compare January 26, 2026 12:47

kubevirt-bot removed the needs-rebase label Jan 26, 2026

RamLavi force-pushed the add_monitoring_collision_alert branch from 423a1ce to 5e159e8 Compare January 28, 2026 12:21

kubevirt-bot removed the do-not-merge/hold label Jan 28, 2026

qinqon requested changes Jan 28, 2026

View reviewed changes

kubevirt-bot assigned qinqon Jan 28, 2026

RamLavi force-pushed the add_monitoring_collision_alert branch from 5e159e8 to 3e8670d Compare January 29, 2026 07:07

RamLavi force-pushed the add_monitoring_collision_alert branch from 3e8670d to 51d3243 Compare January 29, 2026 13:22

kubevirt-bot added the do-not-merge/hold label Jan 29, 2026

RamLavi force-pushed the add_monitoring_collision_alert branch 3 times, most recently from a50a099 to a2db6f3 Compare February 5, 2026 11:16

kubevirt-bot added lgtm approved labels Feb 9, 2026

RamLavi added 4 commits February 9, 2026 09:11

tests/kubectl: Add Port Forwarding functions

7ea8fdd

Port forwarding is needed in order to fetch the alert. This helper will be used in future commits where the alert test will be introduced Signed-off-by: Ram Lavi <ralavi@redhat.com>

RamLavi force-pushed the add_monitoring_collision_alert branch from a2db6f3 to ca0f424 Compare February 9, 2026 07:18

kubevirt-bot removed the lgtm label Feb 9, 2026

kubevirt-bot removed the do-not-merge/hold label Feb 9, 2026

kubevirt-bot added the lgtm label Feb 9, 2026

kubevirt-bot merged commit 4b17a2a into k8snetworkplumbingwg:main Feb 9, 2026
5 checks passed

Comments

Conversation

RamLavi commented Jan 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

RamLavi commented Jan 12, 2026

Uh oh!

gemini-code-assist bot commented Jan 12, 2026

Summary of Changes

Highlights

Footnotes

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist bot Jan 12, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RamLavi Jan 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Jan 12, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Jan 12, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Jan 12, 2026

Choose a reason for hiding this comment

Uh oh!

RamLavi commented Jan 13, 2026

Uh oh!

RamLavi commented Jan 18, 2026

Uh oh!

RamLavi commented Jan 28, 2026

Uh oh!

oshoval commented Jan 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gemini-code-assist bot commented Jan 28, 2026

Uh oh!

oshoval commented Jan 28, 2026

Uh oh!

RamLavi commented Jan 28, 2026

Uh oh!

gemini-code-assist bot commented Jan 28, 2026

Uh oh!

oshoval commented Jan 28, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RamLavi Jan 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sradco commented Jan 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

RamLavi commented Jan 29, 2026

Uh oh!

RamLavi commented Jan 12, 2026 •

edited

Loading

RamLavi Jan 28, 2026 •

edited

Loading

oshoval commented Jan 28, 2026 •

edited

Loading

RamLavi Jan 29, 2026 •

edited

Loading

sradco commented Jan 29, 2026 •

edited

Loading