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.

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

Fix: Correctly assign unique memory addresses for EndpointSlice ports in reconcileByAddressType #335

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

kvaps
Copy link
Member

@kvaps kvaps commented Dec 6, 2024

This PR resolves an issue where EndpointPort fields in reconcileByAddressType pointed to the same memory address across iterations, causing duplicate values in desiredPorts. The fix ensures unique memory addresses are used for each port by creating separate variables for Name, Protocol, and TargetPort on each loop iteration.

E1205 10:40:39.191901       1 kubevirteps_controller.go:535] Failed to create EndpointSlice  in namespace tenant-freemont: EndpointSlice.discovery.k8s.io "a1a08215a57704dc8a52248f3121d731m969n" is invalid: [ports[1].name: Duplicate value: "serve", ports[2].name: Duplicate value: "serve", ports[3].name: Duplicate value: "serve", ports[4].name: Duplicate value: "serve"]

@kubevirt-bot kubevirt-bot added the dco-signoff: no Indicates the PR's author has not DCO signed all their commits. label Dec 6, 2024
@kubevirt-bot kubevirt-bot requested review from qinqon and rmohr December 6, 2024 11:02
@kvaps kvaps changed the title Fix duplicate value in ports Fix EndpointSlice duplication issue in finalize logic Dec 6, 2024
@kvaps kvaps force-pushed the fix-duplicate-ports branch from bb09519 to 9a340de Compare December 6, 2024 11:03
@kubevirt-bot kubevirt-bot added dco-signoff: yes Indicates the PR's author has DCO signed all their commits. and removed dco-signoff: no Indicates the PR's author has not DCO signed all their commits. labels Dec 6, 2024
@kvaps kvaps marked this pull request as draft December 6, 2024 12:54
@kubevirt-bot kubevirt-bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 6, 2024
@kvaps kvaps changed the title Fix EndpointSlice duplication issue in finalize logic Fix: Correctly assign unique memory addresses for EndpointSlice ports in reconcileByAddressType Dec 6, 2024
@kvaps kvaps force-pushed the fix-duplicate-ports branch 2 times, most recently from 4a20974 to 06f89d8 Compare December 6, 2024 13:56
@kvaps kvaps marked this pull request as ready for review December 6, 2024 13:56
@kubevirt-bot kubevirt-bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 6, 2024
@kubevirt-bot kubevirt-bot requested a review from mfranczy December 6, 2024 13:56
@kubevirt-bot kubevirt-bot added size/M and removed size/S labels Dec 6, 2024
@kvaps
Copy link
Member Author

kvaps commented Dec 6, 2024

/assign @qinqon

@kvaps kvaps force-pushed the fix-duplicate-ports branch from 06f89d8 to e26a55c Compare December 9, 2024 10:09
@kubevirt-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please ask for approval from qinqon. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

kvaps added a commit to aenix-io/cozystack that referenced this pull request Dec 9, 2024
Upstream fixes:

- kubevirt/cloud-provider-kubevirt#335
- kubevirt/cloud-provider-kubevirt#336

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

## Release Notes

- **New Features**
  - Incremented Kubernetes chart version to 0.14.1.
- Introduced a new cloud provider controller for managing EndpointSlices
in KubeVirt, enhancing responsiveness to service changes.

- **Improvements**
- Updated Docker image tag for kubevirt-cloud-provider to use the latest
version.
- Enhanced handling of EndpointSlices for LoadBalancer services,
improving service management.

- **Bug Fixes**
- Improved error handling and logging for service retrieval and
EndpointSlice management.

- **Documentation**
- Updated version mappings in the versions map file for clarity and
tracking.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

Signed-off-by: Andrei Kvapil <[email protected]>
@kubevirt-bot
Copy link
Contributor

@klinch0: changing LGTM is restricted to collaborators

In response to this:

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dco-signoff: yes Indicates the PR's author has DCO signed all their commits. size/M
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants