Skip to content

Conversation

@emy
Copy link
Collaborator

@emy emy commented Sep 22, 2025

Is this a BUG FIX or a FEATURE ?:

Uncomment only one, leave it on its own line:

/kind bug

/kind enhancement

What this PR does / why we need it:
As discussed with @qinqon we're introducing an example test for per interface ipv4 forwarding.

Special notes for your reviewer:

Release note:

NONE

Summary by CodeRabbit

  • Documentation
    • Added a new example manifest demonstrating per-interface IPv4 forwarding using a NodeNetworkConfigurationPolicy. The example targets interface eth1, sets it up, enables IPv4 with DHCP, and turns on forwarding.
  • Tests
    • Expanded example coverage by adding a corresponding test entry for the new per-interface IPv4 forwarding example.

@kubevirt-bot kubevirt-bot added release-note-none Denotes a PR that doesn't merit a release note. dco-signoff: yes Indicates the PR's author has DCO signed all their commits. labels Sep 22, 2025
@coderabbitai
Copy link

coderabbitai bot commented Sep 22, 2025

Walkthrough

Adds a new example manifest for a NodeNetworkConfigurationPolicy enabling IPv4 forwarding on eth1, and registers this example in the test examples list. No other files or behaviors are modified.

Changes

Cohort / File(s) Summary
New example manifest
docs/examples/per-interface-ipv4-forwarding.yaml
Adds a NodeNetworkConfigurationPolicy example named per-interface-ipv4-forwarding configuring eth1 (ethernet, up) with IPv4 DHCP enabled and forwarding true.
Test examples registry update
test/doc/examples.go
Adds an ExampleSpec entry: name "Per interface IPv4 forwarding", file "per-interface-ipv4-forwarding.yaml", policy "per-interface-ipv4-forwarding", ifaceNames ["eth1"].

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

I hop through YAML fields so bright,
eth1 wakes, its flags alight—
DHCP whispers, routes take wing,
Forward, packets! Spring to spring.
In tests we note this tidy feat,
A carrot checkmark: crisp and neat. 🥕

Pre-merge checks and finishing touches

✅ 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 "NMT-1521: Add per interface ipv4 forwarding test." clearly and concisely summarizes the primary change—adding a per‑interface IPv4 forwarding test/example—and includes the tracking ticket; this matches the added example manifest and test registration in the PR.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

  • Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
  • Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.


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.

@kubevirt-bot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign cybertron for approval. For more information see the Code Review Process.

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

Needs approval from an approver in each of these files:

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

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c85fc6e and c656ecb.

📒 Files selected for processing (2)
  • docs/examples/per-interface-ipv4-forwarding.yaml (1 hunks)
  • test/doc/examples.go (1 hunks)

@nmstate nmstate deleted a comment from coderabbitai bot Sep 24, 2025
@qinqon
Copy link
Member

qinqon commented Sep 24, 2025

/gemini review

Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a new example and a corresponding test for configuring per-interface IPv4 forwarding. The changes are clear and well-contained. My review includes one suggestion for the new example YAML file to improve its conciseness by removing a redundant property, which will help align it with best practices for users who might copy this example.

@qinqon
Copy link
Member

qinqon commented Sep 26, 2025

@emy looks like like the upgrade test fail at the test ?

@mkowalski
Copy link
Member

But it's not only that failing... I see also

   Error from server (NotFound): nodenetworkconfigurationpolicies.nmstate.io "dns" not found
  [FAILED] 
  Unexpected error:
      <*errors.errorString | 0xc000698ec0>: 
      ./cluster/kubectl.shwait nncp dns --for condition=Available --timeout 3m : selecting podman as container runtime
      Error from server (NotFound): nodenetworkconfigurationpolicies.nmstate.io "dns" not found
       exit status 1
      {
          s: "./cluster/kubectl.shwait nncp dns --for condition=Available --timeout 3m : selecting podman as container runtime\nError from server (NotFound): nodenetworkconfigurationpolicies.nmstate.io \"dns\" not found\n exit status 1",
      }
  occurred 

as well as

   Error from server (NotFound): nodenetworkconfigurationpolicies.nmstate.io "linux-bridge-vlan" not found
  [FAILED] 
  Unexpected error:
      <*errors.errorString | 0xc000bbcd00>: 
      ./cluster/kubectl.shwait nncp linux-bridge-vlan --for condition=Available --timeout 3m : selecting podman as container runtime
      Error from server (NotFound): nodenetworkconfigurationpolicies.nmstate.io "linux-bridge-vlan" not found
       exit status 1
      {
          s: "./cluster/kubectl.shwait nncp linux-bridge-vlan --for condition=Available --timeout 3m : selecting podman as container runtime\nError from server (NotFound): nodenetworkconfigurationpolicies.nmstate.io \"linux-bridge-vlan\" not found\n exit status 1",
      } 

@mkowalski
Copy link
Member

/retest

@kubevirt-bot
Copy link
Collaborator

@emy: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
pull-kubernetes-nmstate-e2e-upgrade-k8s c656ecb link false /test pull-kubernetes-nmstate-e2e-upgrade-k8s

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dco-signoff: yes Indicates the PR's author has DCO signed all their commits. release-note-none Denotes a PR that doesn't merit a release note. size/S

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants