-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Migrate to range-over-func style iteration in libcalicio-go/lib/set #11456
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.
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
base: master
Are you sure you want to change the base?
Conversation
Add support for Go 1.22+ range-over-func iteration to the Set types (Typed and Adaptive) in libcalico-go/lib/set alongside the existing Iter() method. Changes: - Add All() iter.Seq[T] method to Set[T] interface - Implement All() for Typed[T] with direct map iteration - Implement All() for Adaptive[T] with intelligent handling: - Map-backed sets (>16 items): direct iteration - Array-backed sets (≤16 items): snapshot for safe mutation - Add tests for All() method The new All() method provides modern Go iteration using iter.Seq while maintaining the same safe-mutation-during-iteration guarantees as the existing Iter() method. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Convert codebase from callback-based Set.Iter() to Go 1.22+ range-over-func iteration using Set.All(). This modernizes iteration patterns throughout the codebase for improved readability and idiomatic Go style. The conversion handles three patterns: - Simple iteration: callback returning nil → for-range loop - Early termination: return set.StopIteration → break - Removal during iteration: return set.RemoveItem → Discard() + continue 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Convert the three remaining set.Iter callback-based iterations to modern for-range loops using set.All() in the BPF endpoint manager. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this 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 migrates the set iteration mechanism in libcalico-go/lib/set from callback-based Iter() to Go's modern range-over-func style using a new All() method that returns iter.Seq[T]. This modernizes the codebase to use idiomatic Go 1.23+ iteration patterns while maintaining backward compatibility by keeping the existing Iter() method.
Key changes include:
- Adding
All() iter.Seq[T]method to theSet[T]interface and all implementations (Typed, Adaptive, Empty) - Migrating all internal and external usage from
s.Iter(func(item T) error {...})tofor item := range s.All() {...} - Converting
StopIterationerror returns tobreakstatements andRemoveItemreturns to explicits.Discard(item)calls - Adding comprehensive test coverage for the new iterator including early termination, deletion during iteration, and concurrent modification scenarios
Reviewed changes
Copilot reviewed 64 out of 64 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| libcalico-go/lib/set/interface.go | Adds All() iter.Seq[T] method to Set interface |
| libcalico-go/lib/set/set.go | Implements All() for Typed set, updates internal methods to use new iteration |
| libcalico-go/lib/set/adaptive.go | Implements All() for Adaptive set with snapshot-based iteration for array-backed sets |
| libcalico-go/lib/set/union.go | Migrates union iteration to use All() with proper early termination handling |
| libcalico-go/lib/set/diff.go | Migrates difference iteration to use All() with error handling preserved |
| libcalico-go/lib/set/set_test.go | Adds comprehensive tests for All() method behavior |
| libcalico-go/lib/set/adaptive_test.go | Adds tests for Adaptive set's All() implementation |
| libcalico-go/lib/backend/syncersv1/dedupebuffer/dedupe_buffer.go | Migrates deduplication buffer to use range-over-func iteration |
| hack/cmd/deps/deps.go | Updates dependency calculation to use new iteration style |
| goldmane/pkg/storage/*.go | Migrates goldmane storage layer to use range-over-func iteration |
| felix/**/*.go | Comprehensive migration of Felix components (wireguard, route tables, nftables, iptables, BPF, dataplane, etc.) |
| calicoctl/calicoctl/commands/**/*.go | Migrates calicoctl command implementations to new iteration style |
The error return value was only needed for Set.Iter() compatibility. Now that we use for-range with All(), it can be removed. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Description
Modernise libcalicio-go's set type with range-over-func iterators and migrate the codebase to use them.
Related issues/PRs
Todos
Release Note
Reminder for the reviewer
Make sure that this PR has the correct labels and milestone set.
Every PR needs one
docs-*label.docs-pr-required: This change requires a change to the documentation that has not been completed yet.docs-completed: This change has all necessary documentation completed.docs-not-required: This change has no user-facing impact and requires no docs.Every PR needs one
release-note-*label.release-note-required: This PR has user-facing changes. Most PRs should have this label.release-note-not-required: This PR has no user-facing changes.Other optional labels:
cherry-pick-candidate: This PR should be cherry-picked to an earlier release. For bug fixes only.needs-operator-pr: This PR is related to install and requires a corresponding change to the operator.