Skip to content

feat: add collection utility functions #66

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

Merged
merged 5 commits into from
Jul 14, 2025
Merged

feat: add collection utility functions #66

merged 5 commits into from
Jul 14, 2025

Conversation

Diaphteiros
Copy link
Contributor

@Diaphteiros Diaphteiros commented Jun 23, 2025

What this PR does / why we need it:
When implementing preemptive scheduling, I needed a few utility functions which I put into this repo.
Mostly related to collections.

See release notes for more details.

Which issue(s) this PR fixes:
None

Special notes for your reviewer:
Prerequisite for openmcp-project/openmcp-operator#67

As requested, I split this PR into smaller ones, the new ones are:

Release note:

The `pkg/collections/maps` package now has a `GetAny` functions to get an arbitrary element from a map.
The `pkg/collections` package now has projection (`ProjectSlice`, `ProjectMapToSlice`, `ProjectMapToMap`) and aggregation (`AggregateSlice`, `AggregateMap`) functions to perform operations for each entry of a slice or map.
The `pkg/controller` package has a new `RemoveFinalizerWithPrefix` function that removes the first finalizer from the object with the given prefix.

@Diaphteiros Diaphteiros changed the title add utility functions for scheduler feat: add utility functions for scheduler Jun 23, 2025
@Diaphteiros Diaphteiros marked this pull request as draft June 23, 2025 11:50
@Diaphteiros Diaphteiros marked this pull request as ready for review June 23, 2025 15:47
reshnm
reshnm previously approved these changes Jun 30, 2025
Copy link

@Copilot 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 adds several utility functions and enhancements to support preemptive scheduling, testing, and general collection operations.

  • Introduces UID injection support in testing environments (WithUIDs and InjectUIDOnObjectCreation).
  • Implements a retrying Kubernetes client wrapper with configurable backoff, timeout, and attempt parameters.
  • Adds generic projection, aggregation, and map‐related helpers (e.g., ProjectSlice, GetAny) plus a RemoveFinalizersWithPrefix utility.

Reviewed Changes

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

Show a summary per file
File Description
pkg/testing/environment.go Added WithUIDs to inject random UIDs on object creation.
pkg/testing/complex_environment.go Extended complex builder for per-cluster UID injection; added InjectUIDOnObjectCreation.
pkg/retry/retry.go New Client wrapper implementing retry logic for Kubernetes API calls.
pkg/retry/retry_test.go Comprehensive tests for the retrying client behavior.
pkg/collections/utils.go Added projection and aggregation helpers for slices and maps.
pkg/collections/maps/utils.go Added GetAny to return an arbitrary key-value pair.
pkg/controller/utils.go Introduced RemoveFinalizersWithPrefix to strip object finalizers by prefix.
pkg/controller/utils_test.go Converted tests to Ginkgo/Gomega style and added coverage for new utilities.
pkg/clusters/cluster.go Added Retry() method returning the retrying client.
docs/libs/retry.md Documentation for the retrying client.
docs/README.md Added retry docs to the table of contents.
Comments suppressed due to low confidence (5)

pkg/retry/retry.go:61

  • [nitpick] The getter MaxRetries doesn’t align with the setter WithMaxAttempts. Consider renaming one of them for consistency (e.g., MaxAttempts).
func (rc *Client) MaxRetries() int {

pkg/controller/utils.go:69

  • Ranging over an integer is invalid. Replace with an index-based loop, e.g., for i := 0; i < length; i++, or iterate directly over the slice with for i := range fins.
	for i := range length {

pkg/controller/utils.go:70

  • The code uses strings.HasPrefix but the strings package isn’t imported. Add import "strings" at the top of the file.
		if (removeAll || !found) && strings.HasPrefix(fins[i], prefix) {

pkg/retry/retry.go:152

  • Ranging over an integer is invalid. Use a classic for-loop like for i := 0; i < argCountVariadic; i++ to index into the variadic arguments.
		for i := range argCountVariadic {

pkg/testing/complex_environment.go:432

  • This function signature references context.Context but the context package isn’t imported. Add import "context" to the imports.
func InjectUIDOnObjectCreation(additionalLogic func(ctx context.Context, client client.WithWatch, obj client.Object, opts ...client.CreateOption) error) func(ctx context.Context, client client.WithWatch, obj client.Object, opts ...client.CreateOption) error {

@maximiliantech
Copy link
Member

@Diaphteiros Could you have a look on the review from Copilot please? There might be some things that need your attention. And also please don't forget to update the PR description 🙏

@Diaphteiros
Copy link
Contributor Author

@maximiliantech CoPilot actually noticed one function that I forgot to rename, the other suggestions were humbug.
I adapted the PR description.

@Diaphteiros
Copy link
Contributor Author

I added another small function and adapted the Release Notes in the PR info.

@Diaphteiros Diaphteiros changed the title feat: add utility functions for scheduler feat: add collection utility functions Jul 11, 2025
reshnm
reshnm previously approved these changes Jul 14, 2025
@Diaphteiros Diaphteiros requested a review from robertgraeff July 14, 2025 14:20
@Diaphteiros Diaphteiros merged commit 09bfaf2 into main Jul 14, 2025
7 checks passed
@Diaphteiros Diaphteiros deleted the scheduler branch July 14, 2025 14:52
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.

4 participants