Skip to content

cnf ran ptp: make OC cases events-optional #1165

Open
klaskosk wants to merge 2 commits intorh-ecosystem-edge:mainfrom
klaskosk:ptp-event-optional-oc
Open

cnf ran ptp: make OC cases events-optional #1165
klaskosk wants to merge 2 commits intorh-ecosystem-edge:mainfrom
klaskosk:ptp-event-optional-oc

Conversation

@klaskosk
Copy link
Copy Markdown
Collaborator

@klaskosk klaskosk commented Feb 10, 2026

This commit uses the eventmetric package to fallback to metric
assertions whenever PTP events are not enabled.

Additionally, it flips the semantics of CurrentState. By default, the
CurrentState logs are excluded since they don't represent events being
sent to the consumer. They may be explicitly enabled when desired.

Closes: CNF-21255
Depends-on: #1123
Assisted-by: Cursor

Summary by CodeRabbit

  • New Features

    • Combined event+metric assertion framework for PTP state validation (node/interface, start time, timeout).
    • Runtime check to skip event operations when cluster event publishing is disabled.
    • Tests temporarily tighten Prometheus scrape interval during runs and restore it afterwards.
  • Bug Fixes

    • Improved error handling and clearer messages around event/config/version checks.
  • Refactor

    • Tests reworked to use metric-driven assertions for LOCKED, FREERUN, HOLDOVER flows.
  • Documentation

    • Event option renamed to control inclusion of current-state events (default: exclude).
  • Breaking Changes

    • Event-wait API/option semantics changed; callers must adopt the new assertion-based flow or updated options.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Feb 10, 2026

Note

Reviews paused

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds a concurrent event+metric assertion framework, gates consumer operations when PTP event publishing is disabled, renames an events option to include-current-state semantics, introduces ServiceMonitor update/restore utilities for 1s scraping, and migrates many PTP tests from direct event-pod waits to the new assertion-based checks.

Changes

Cohort / File(s) Summary
Consumer package
tests/cnf/ran/ptp/internal/consumer/bothversions.go, tests/cnf/ran/ptp/internal/consumer/common.go
Added AreEventsEnabled and early-skip when events are disabled; strengthened config/version nil-checks and error wrapping; prefer V2 API for PTP >= 4.19.
Event+Metric assertion framework
tests/cnf/ran/ptp/internal/eventmetric/eventmetric.go
New generic AssertConfig[V] and fluent API (NewAssertion, ForNode, WithStartTime, WithTimeout, WithEventOptions, WithMetricOptions, ExecuteAssertion) to run events and Prometheus metric checks concurrently and aggregate results.
Events package & docs
tests/cnf/ran/ptp/internal/events/events.go, tests/cnf/ran/ptp/internal/events/README.md
Renamed option from WithoutCurrentState to WithCurrentState (invert semantics to include current-state when true); updated extraction logic and docs; logging enhanced.
Test migrations — event/metric assertions
tests/cnf/ran/ptp/tests/ptp-event-consumer.go, tests/cnf/ran/ptp/tests/ptp-events-and-metrics.go, tests/cnf/ran/ptp/tests/ptp-interfaces.go, tests/cnf/ran/ptp/tests/ptp-node-reboot.go, tests/cnf/ran/ptp/tests/ptp-process-restart.go
Replaced direct consumer-pod lookups and WaitForEvent calls with eventmetric.NewAssertion-based assertions scoped by node/interface/time; added AreEventsEnabled checks to skip when appropriate; updated narratives to reflect combined event+metric validation.
Test option removal
tests/cnf/ran/ptp/tests/ptp-ntp-fallback.go
Removed prior WithoutCurrentState option usage at call sites to match the renamed WithCurrentState API (calls now use default behavior).
Metrics & ServiceMonitor
tests/cnf/ran/internal/ranparam/const.go, tests/cnf/ran/ptp/internal/metrics/servicemonitor.go, tests/cnf/ran/ptp/ptp_suite_test.go, tests/cnf/ran/ptp/internal/metrics/assert.go
Added PtpServiceMonitorName constant; new functions to update/restore PTP ServiceMonitor scrape interval; BeforeSuite/AfterSuite update and restore to set 1s scraping; added logging around metric assertions.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Suggested labels

lgtm

Suggested reviewers

  • yliu127
  • hhassid
🚥 Pre-merge checks | ✅ 3 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Merge Conflict Detection ⚠️ Warning ❌ Merge conflicts detected (24 files):

⚔️ go.mod (content)
⚔️ go.sum (content)
⚔️ tests/cnf/ran/internal/ranparam/const.go (content)
⚔️ tests/cnf/ran/ptp/internal/consumer/bothversions.go (content)
⚔️ tests/cnf/ran/ptp/internal/eventmetric/eventmetric.go (content)
⚔️ tests/cnf/ran/ptp/internal/events/README.md (content)
⚔️ tests/cnf/ran/ptp/internal/events/events.go (content)
⚔️ tests/cnf/ran/ptp/internal/metrics/assert.go (content)
⚔️ tests/cnf/ran/ptp/internal/profiles/parser.go (content)
⚔️ tests/cnf/ran/ptp/internal/profiles/profiles.go (content)
⚔️ tests/cnf/ran/ptp/ptp_suite_test.go (content)
⚔️ tests/cnf/ran/ptp/tests/ptp-event-consumer.go (content)
⚔️ tests/cnf/ran/ptp/tests/ptp-events-and-metrics.go (content)
⚔️ tests/cnf/ran/ptp/tests/ptp-interfaces.go (content)
⚔️ tests/cnf/ran/ptp/tests/ptp-node-reboot.go (content)
⚔️ tests/cnf/ran/ptp/tests/ptp-ntp-fallback.go (content)
⚔️ tests/cnf/ran/ptp/tests/ptp-process-restart.go (content)
⚔️ tests/system-tests/internal/reboot/reboot.go (content)
⚔️ vendor/github.com/rh-ecosystem-edge/eco-goinfra/pkg/pod/pod.go (content)
⚔️ vendor/github.com/rh-ecosystem-edge/eco-goinfra/pkg/schemes/argocd/argocdoperator/argocd_types.go (content)
⚔️ vendor/github.com/rh-ecosystem-edge/eco-goinfra/pkg/schemes/argocd/argocdoperator/zz_generated.deepcopy.go (content)
⚔️ vendor/github.com/rh-ecosystem-edge/eco-goinfra/pkg/schemes/argocd/argocdoperatorcommon/defaults.go (content)
⚔️ vendor/github.com/rh-ecosystem-edge/eco-goinfra/pkg/schemes/argocd/argocdoperatorcommon/keys.go (content)
⚔️ vendor/modules.txt (content)

These conflicts must be resolved before merging into main.
Resolve conflicts locally and push changes to this branch.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately reflects the main objective of the PR—making PTP OC test cases work when events are optional by updating them to use eventmetric assertions as fallback to metrics when events are disabled.
Docstring Coverage ✅ Passed Docstring coverage is 93.75% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@klaskosk klaskosk force-pushed the ptp-event-optional-oc branch 2 times, most recently from f05fd4e to a2c0c7c Compare February 12, 2026 22:25
@klaskosk
Copy link
Copy Markdown
Collaborator Author

@coderabbitai pause

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Feb 13, 2026

✅ Actions performed

Reviews paused.

@klaskosk klaskosk force-pushed the ptp-event-optional-oc branch 4 times, most recently from 70b5293 to 956787b Compare February 20, 2026 22:06
@klaskosk klaskosk force-pushed the ptp-event-optional-oc branch 3 times, most recently from b3b1f20 to 19c18e1 Compare February 24, 2026 22:53
This commit adds the ability to update the ServiceMonitor for PTP and
then restore it. By default, metrics are collected only every 30
seconds, which limits the resolution we have in these tests. After this
change, Prometheus will have second-by-second changes to the metrics.

There are small downsides in the form of CPU usage for Prometheus, but
this should not be an issue since we are measuring functionality, not
performance.

Assisted-by: Cursor
This commit uses the eventmetric package to fallback to metric
assertions whenever PTP events are not enabled.

Additionally, it flips the semantics of CurrentState. By default, the
CurrentState logs are excluded since they don't represent events being
sent to the consumer. They may be explicitly enabled when desired.

There are a few complexities still, such as some assertions needing to
be events-only due to the metric being too transient to capture.
Additionally, the tests will now configure the ServiceMonitor such that
Prometheus scrapes the metrics every second instead of the default 30
seconds. This improves the precision of metric assertions.

Assisted-by: Cursor
@klaskosk klaskosk force-pushed the ptp-event-optional-oc branch from 19c18e1 to 28b5fd2 Compare February 24, 2026 23:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant