Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

IPAM: Extend CiliumNodes CRD to Track Used and Released IPv6 Addresses #31143

Merged
merged 1 commit into from
Apr 29, 2024
Merged

IPAM: Extend CiliumNodes CRD to Track Used and Released IPv6 Addresses #31143

merged 1 commit into from
Apr 29, 2024

Conversation

danehans
Copy link
Contributor

@danehans danehans commented Mar 4, 2024 •
edited
Loading

Previously, IPAM API types were specific to managing an IPv4 address pool. This icommit updates the API types to support separate IPAM pool maintainers for IPv4 and IPv6.

  • pkg/ipam/node.go: Updates the Node type to support IPv6 allocation statistics.
  • pkg/ipam/stats/stats.go: Updates the InterfaceStats type support IPv6 interface statistics.
  • pkg/ipam/types/types.go: Updates IPAMSpec, IPAMStatus, and Subnet type to support IPv6 allocation statistics.
  • ciliumnodes.yaml: Regenerated due to newly added fields of IPAM API types.

Supports: #19251

Adds `IPv6Pool` field to the spec of CiliumNodes CRD to list of IPv6 addresses available to the node for allocation.
Adds `IPv6Used` field to the status of CiliumNodes CRD to list all IPv6 addresses from `ciliumnodes.spec.ipam.ipv6pool` which have been allocated and are in use.

@danehans danehans requested review from a team as code owners March 4, 2024 20:28
@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Mar 4, 2024
@danehans danehans force-pushed the issue_19251_ipam_spec_type_refactor branch 2 times, most recently from 8466901 to b616ac9 Compare March 4, 2024 21:44
@danehans danehans requested a review from a team as a code owner March 4, 2024 21:44
@danehans danehans force-pushed the issue_19251_ipam_spec_type_refactor branch from b616ac9 to 76ff3d8 Compare March 4, 2024 21:45
@danehans danehans changed the title IPAM: Updates API Types for IPv6 Support IPAM: Updates API Types for IPv6 Allocation Statistics Mar 4, 2024
@danehans danehans force-pushed the issue_19251_ipam_spec_type_refactor branch 2 times, most recently from 70cae72 to ffda490 Compare March 4, 2024 23:41
@danehans danehans changed the title IPAM: Updates API Types for IPv6 Allocation Statistics IPAM: Updates API Types for IPv6 Allocation Mar 5, 2024
@danehans
Copy link
Contributor Author

Marking as a draft until #30684 is merged.

@danehans danehans marked this pull request as draft March 25, 2024 17:29
@danehans danehans force-pushed the issue_19251_ipam_spec_type_refactor branch from ffda490 to 0755129 Compare April 1, 2024 17:55
@danehans danehans marked this pull request as ready for review April 1, 2024 17:58
@danehans
Copy link
Contributor Author

danehans commented Apr 1, 2024

Draft was removed since #30684 has merged.

@danehans
IPAM: Updates API Types for IPv6 Allocation Statistics
2eef811 
Previously, IPAM API types were specific to managing an IPv4 address pool.
This icommit updates the API types to support separate IPAM pool maintainers for
IPv4 and IPv6.

- `pkg/ipam/node.go`: Updates the `Node` type to support IPv6 allocation statistics.
- `pkg/ipam/stats/stats.go`: Updates the `InterfaceStats` type support IPv6 interface statistics.
- `pkg/ipam/types/types.go`: Updates `IPAMSpec`, `IPAMStatus`, and `Subnet` type to support IPv6 allocation statistics.
- `ciliumnodes.yaml`: Regenerated due to newly added fields of IPAM API types.

Supports: cilium#19251

Signed-off-by: Daneyon Hansen <daneyon.hansen@solo.io>
@danehans danehans force-pushed the issue_19251_ipam_spec_type_refactor branch from 0755129 to 2eef811 Compare April 5, 2024 17:07
@danehans
Copy link
Contributor Author

danehans commented Apr 5, 2024

/test

@danehans
Copy link
Contributor Author

danehans commented Apr 6, 2024

/ci-clustermesh

@danehans
Copy link
Contributor Author

danehans commented Apr 7, 2024

/ci-gateway-api

@danehans
Copy link
Contributor Author

danehans commented Apr 7, 2024

/ci-ingress

@danehans
Copy link
Contributor Author

@joestringer @tommyp1ckles @christarazi is there anything else needed for this PR to be merged?

@christarazi
Copy link
Member

Looks like the release-note label needs to be set properly. The mergability job specifies the reason for the blockage.

@joestringer
Copy link
Member

Indeed. So I'd suggest two things here:
(1) I believe you should be able to add a label to this PR like release-note/*. For this change, since it's modifying a CRD, but doesn't appear to be a dedicated feature by itself, I would guess that release-note/minor is a reasonable level. Minor is for anything user-facing but not necessarily a major thing we want to call out in the upcoming release. misc is for anything not user-facing. There's also a ci release note label for CI-only changes.
(2) Maybe add a user-facing release note in the PR description under a section like the below. At least for me, I don't know what the current PR title "Updates API Types for IPv6 Allocation" really means if I was to try to use this change as a user. Maybe something like "Extend the CiliumNodes CRD to track used and released IPv6 addresses"? . If this is actually just for things managed by Cilium then maybe this is not necessary, we could just resolve (1) by setting release-note/misc label.

```release-note
...
```

@danehans danehans changed the title IPAM: Updates API Types for IPv6 Allocation IPAM: Extend CiliumNodes CRD to Track Used and Released IPv6 Addresses Apr 29, 2024
@danehans danehans added the release-note/minor This PR changes functionality that users may find relevant to operating Cilium. label Apr 29, 2024
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Apr 29, 2024
@joestringer joestringer added this pull request to the merge queue Apr 29, 2024
@maintainer-s-little-helper maintainer-s-little-helper bot added ready-to-merge This PR has passed all tests and received consensus from code owners to merge. labels Apr 29, 2024
@danehans
Copy link
Contributor Author

@christarazi @joestringer thanks for the guidance here. I have updated the PR description to include a release note and have added the release-note/minor label. PTAL when you have a moment.

@christarazi christarazi added sig/k8s Impacts the kubernetes API, or kubernetes -> cilium internals translation layers. sig/ipam IP address management, including cloud IPAM area/ipam Impacts IP address management functionality. labels Apr 29, 2024
Merged via the queue into cilium:main with commit 93a6d3c Apr 29, 2024
62 checks passed
@danehans danehans deleted the issue_19251_ipam_spec_type_refactor branch May 2, 2024 17:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@christarazi christarazi christarazi approved these changes

@tommyp1ckles tommyp1ckles tommyp1ckles approved these changes

Assignees
No one assigned
Labels
area/ipam Impacts IP address management functionality. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/minor This PR changes functionality that users may find relevant to operating Cilium. sig/ipam IP address management, including cloud IPAM sig/k8s Impacts the kubernetes API, or kubernetes -> cilium internals translation layers.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@danehans @christarazi @joestringer @tommyp1ckles