Skip to content

fix(cache): updating the k8s service endpoints to handle new random suffix appended to the name#72

Merged
Jacobbrewer1 merged 4 commits intomainfrom
fix/cache-service-endpoints
May 7, 2025
Merged

fix(cache): updating the k8s service endpoints to handle new random suffix appended to the name#72
Jacobbrewer1 merged 4 commits intomainfrom
fix/cache-service-endpoints

Conversation

@Jacobbrewer1
Copy link
Owner

Describe your changes

This pull request refactors the ServiceEndpointHashBucket implementation to improve its handling of Kubernetes EndpointSlices and updates related tests. Additionally, it removes redundant compile-time interface checks for clarity.

Refactoring and Enhancements for EndpointSlices:

  • Updated Start method in ServiceEndpointHashBucket to use List instead of Get for fetching EndpointSlices, applying a label selector for filtering, and handling cases where no slices are found. It also combines hosts from multiple slices into a unified set. (cache/service_endpoint_hash_bucket.go, cache/service_endpoint_hash_bucket.goL87-L93)
  • Modified onEndpointUpdate method to match EndpointSlices using GenerateName prefix instead of Name for better compatibility with Kubernetes naming conventions. (cache/service_endpoint_hash_bucket.go, cache/service_endpoint_hash_bucket.goL146-R156)

Test Updates for EndpointSlices:

  • Enhanced Test_Lifecycle to include GenerateName, labels, and annotations in the mock EndpointSlice, ensuring tests align with the new implementation. (cache/service_endpoint_hash_bucket_test.go, cache/service_endpoint_hash_bucket_test.goL27-R43)
  • Updated Test_onEndpointUpdate to use GenerateName for EndpointSlice matching in test cases. (cache/service_endpoint_hash_bucket_test.go, [1] [2]

Codebase Simplification:

  • Removed redundant compile-time interface checks in both ServiceEndpointHashBucket and dedupeHandler to reduce verbosity without impacting functionality. (cache/service_endpoint_hash_bucket.go, [1]; logging/dedupe_handler.go, [2]

… random suffix appended to the name on the services
@Jacobbrewer1 Jacobbrewer1 changed the title fix(cache): updating the handling for the k8s service endpoints to handle the new random suffix appended to the name on the services fix(cache): updating the k8s service endpoints to handle new random suffix appended to the name May 7, 2025
@Jacobbrewer1 Jacobbrewer1 marked this pull request as ready for review May 7, 2025 08:02
Copilot AI review requested due to automatic review settings May 7, 2025 08:02
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 refactors the ServiceEndpointHashBucket to improve the handling of Kubernetes EndpointSlices by switching from Get to List operations and updating the matching logic from using Name to GenerateName, as well as updating corresponding tests and cleaning up redundant comments.

  • Refactors the Start method to use a List operation with proper error handling
  • Updates onEndpointUpdate to match EndpointSlices using the GenerateName prefix
  • Enhances test cases to include GenerateName, labels, and annotations for EndpointSlices and removes redundant compile-time interface check comments

Reviewed Changes

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

File Description
logging/dedupe_handler.go Removed redundant compile-time interface check comments for clarity
cache/service_endpoint_hash_bucket_test.go Updated tests to use GenerateName and added extra metadata to EndpointSlice test fixtures
cache/service_endpoint_hash_bucket.go Switched from Get to List for fetching EndpointSlices and updated matching logic using GenerateName
Comments suppressed due to low confidence (1)

cache/service_endpoint_hash_bucket.go:156

  • Consider handling cases where GetGenerateName() might return an empty string. If GenerateName is not set, falling back to using the Name field would ensure robust matching of EndpointSlices.
if !strings.HasPrefix(coreNewEndpoints.GetGenerateName(), sb.appName) || coreNewEndpoints.GetNamespace() != sb.appNamespace {

@Jacobbrewer1 Jacobbrewer1 requested a review from Copilot May 7, 2025 08:24
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 refactors the EndpointSlice handling in the cache package for improved compatibility with Kubernetes naming conventions and simplifies the code by removing redundant compile-time interface checks.

  • Refactored ServiceEndpointHashBucket to list EndpointSlices using a label selector and handle missing slices.
  • Updated onEndpointUpdate to use the GenerateName prefix for EndpointSlice matching.
  • Enhanced test coverage with additional test cases that include GenerateName, labels, and annotations.

Reviewed Changes

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

File Description
logging/dedupe_handler.go Removed compile-time interface check comments to simplify the file.
cache/service_endpoint_hash_bucket.go Updated Start method to use List with a label selector; revised onEndpointUpdate to incorporate GenerateName matching logic.
cache/service_endpoint_hash_bucket_test.go Updated tests to reflect changes in EndpointSlice handling, including test cases for scenarios with and without GenerateName.
Comments suppressed due to low confidence (2)

cache/service_endpoint_hash_bucket.go:90

  • [nitpick] Verify that the new List approach with a label selector reliably captures all relevant EndpointSlices. Consider adding a brief comment discussing any potential edge cases where endpoints may be filtered unintentionally.
endpointSliceList, err := sb.kubeClient.DiscoveryV1().EndpointSlices(sb.appNamespace).List(ctx, metav1.ListOptions{ LabelSelector: fmt.Sprintf("%s=%s", k8sNameLabel, sb.appName), })

cache/service_endpoint_hash_bucket.go:166

  • [nitpick] Consider adding an inline comment to clarify the intent behind matching EndpointSlices using the GenerateName prefix, improving readability for future maintainers.
generateName := coreNewEndpoints.GetGenerateName()

@Jacobbrewer1 Jacobbrewer1 merged commit c8809a6 into main May 7, 2025
5 checks passed
@Jacobbrewer1 Jacobbrewer1 deleted the fix/cache-service-endpoints branch May 7, 2025 08:25
@github-actions
Copy link

github-actions bot commented Jun 7, 2025

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 7, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants