Skip to content

Conversation

@komer3
Copy link
Contributor

@komer3 komer3 commented Aug 20, 2025

  • Add computeStableIPv6PodCIDR() to derive a stable /112 from a node’s /64 using mnemonic subprefix :0:c::/112.
  • allocateIPv6CIDR(): prefer stable subprefix when mask=112 and base is /64; otherwise fall back to start-of-range masking.
  • Update tests in cloud/nodeipam/ipam/cloud_allocator_test.go to expect 2300:5800:2:1:0:c::/112.
  • Document behavior and correct example flag to --node-cidr-mask-size-ipv6=112 in docs/configuration/nodeipam.md.
  • Add/adjust debug logs for clarity; no change to IPv4 behavior.

General:

  • Have you removed all sensitive information, including but not limited to access keys and passwords?
  • Have you checked to ensure there aren't other open or closed Pull Requests for the same bug/feature/question?

Pull Request Guidelines:

  1. Does your submission pass tests?
  2. Have you added tests?
  3. Are you addressing a single feature in this PR?
  4. Are your commits atomic, addressing one change per commit?
  5. Are you following the conventions of the language?
  6. Have you saved your large formatting changes for a different PR, so we can focus on your work?
  7. Have you explained your rationale for why this feature is needed?
  8. Have you linked your PR to an open issue

…pdate tests/docs

- Add computeStableIPv6PodCIDR() to derive a stable /112 from a node’s /64
  using mnemonic subprefix :0:c::/112.
- allocateIPv6CIDR(): prefer stable subprefix when mask=112 and base is /64;
  otherwise fall back to start-of-range masking.
- Update tests in cloud/nodeipam/ipam/cloud_allocator_test.go to expect
  2300:5800:2:1:0:c::/112.
- Document behavior and correct example flag to --node-cidr-mask-size-ipv6=112
  in docs/configuration/nodeipam.md.
- Add/adjust debug logs for clarity; no change to IPv4 behavior.
@github-actions github-actions bot added improvement for improvements in existing functionality in the changelog. new-feature for new features in the changelog. labels Aug 20, 2025
@codecov
Copy link

codecov bot commented Aug 20, 2025

Codecov Report

❌ Patch coverage is 80.00000% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 71.88%. Comparing base (df34350) to head (590d969).
⚠️ Report is 9 commits behind head on main.

Files with missing lines Patch % Lines
cloud/nodeipam/ipam/cloud_allocator.go 80.00% 6 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #439      +/-   ##
==========================================
+ Coverage   71.87%   71.88%   +0.01%     
==========================================
  Files          19       19              
  Lines        3598     3625      +27     
==========================================
+ Hits         2586     2606      +20     
- Misses        773      779       +6     
- Partials      239      240       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@komer3 komer3 requested a review from Copilot August 20, 2025 19:50
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces deterministic IPv6 PodCIDR allocation using a stable mnemonic subprefix :0:c::/112 within a node's /64 IPv6 range. The change ensures consistent PodCIDR assignment across restarts while maintaining backward compatibility.

  • Implements computeStableIPv6PodCIDR() function to derive stable /112 subnets from /64 base ranges
  • Updates allocation logic to prefer stable subprefix when conditions are met, falling back to original behavior otherwise
  • Corrects documentation example and updates test expectations to reflect the new deterministic behavior

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
cloud/nodeipam/ipam/cloud_allocator.go Adds stable IPv6 PodCIDR computation logic and updates allocation method
cloud/nodeipam/ipam/cloud_allocator_test.go Updates test expectations to use new deterministic IPv6 addresses
docs/configuration/nodeipam.md Documents new behavior and corrects configuration example

This comment was marked as outdated.

@AshleyDumaine AshleyDumaine requested a review from Copilot August 21, 2025 14:49
Copy link
Contributor

@AshleyDumaine AshleyDumaine left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR implements deterministic IPv6 PodCIDR allocation using a stable mnemonic subprefix :0:c::/112 to derive /112 subnets from node /64 IPv6 ranges. This ensures consistent IPv6 pod networking across node restarts and deployments.

  • Adds getIPv6PodCIDR() function to compute stable /112 PodCIDRs using mnemonic subprefix :0:c::/112
  • Updates allocateIPv6CIDR() to prefer stable allocation when conditions are met, with fallback to original behavior
  • Updates all test expectations and documentation to reflect the new stable IPv6 allocation behavior

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
cloud/nodeipam/ipam/cloud_allocator.go Core implementation adding stable IPv6 PodCIDR computation with mnemonic subprefix
cloud/nodeipam/ipam/cloud_allocator_test.go New test for stable IPv6 function and updated expected values across existing tests
docs/configuration/nodeipam.md Documentation updates explaining stable IPv6 behavior and correcting example flag value

… slice. In go, when passing non pointer var that are slices, maps, or channels, it just makes a copy of the obj header and not the underlying data. Since this passed ip can be used in the fallback logic, we need to make sure we copy it so there is no bug in future.
@komer3 komer3 merged commit ad082a6 into main Aug 21, 2025
11 of 12 checks passed
@komer3 komer3 deleted the nodeipam/ipv6-stable-112-subprefix branch August 21, 2025 20:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

improvement for improvements in existing functionality in the changelog. new-feature for new features in the changelog.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants