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

ipam: deepcopy interface resource correctly. #26998

Merged

Conversation

tommyp1ckles
Copy link
Contributor

@tommyp1ckles tommyp1ckles commented Jul 21, 2023

ipam: deepcopy interface resource correctly.

Currently only used by azure ipam test mocks, was resulting in race
condition prone code where underlying interface addresses where being
shared across resources.

This ensures that the revision interface is correctly deepcopied such
that the underlying resource is also safely copied.

This seems at least partially related to flake #26617.

Addresses: #26617

Signed-off-by: Tom Hadlaw tom.hadlaw@isovalent.com

@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 Jul 21, 2023
@tommyp1ckles tommyp1ckles force-pushed the pr/tp/fix-false-deepcopy-causing-races branch 2 times, most recently from 0b52cfe to 0ebd5f1 Compare July 26, 2023 22:37
@tommyp1ckles tommyp1ckles added the release-note/ci This PR makes changes to the CI. label Jul 26, 2023
@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 Jul 26, 2023
@tommyp1ckles tommyp1ckles added the area/ipam Impacts IP address management functionality. label Jul 26, 2023
@tommyp1ckles tommyp1ckles marked this pull request as ready for review July 26, 2023 22:38
@tommyp1ckles tommyp1ckles requested review from a team as code owners July 26, 2023 22:38
@tommyp1ckles tommyp1ckles marked this pull request as draft July 26, 2023 22:39
@tommyp1ckles tommyp1ckles force-pushed the pr/tp/fix-false-deepcopy-causing-races branch from 0ebd5f1 to 39e1697 Compare August 1, 2023 04:24
@tommyp1ckles tommyp1ckles marked this pull request as ready for review August 1, 2023 04:25
@christarazi christarazi force-pushed the pr/tp/fix-false-deepcopy-causing-races branch from 39e1697 to 5976a48 Compare August 2, 2023 16:55
@christarazi
Copy link
Member

/test

@tommyp1ckles tommyp1ckles force-pushed the pr/tp/fix-false-deepcopy-causing-races branch from 5976a48 to 4b4e8d0 Compare August 10, 2023 04:38
@tommyp1ckles
Copy link
Contributor Author

/test

@tommyp1ckles
Copy link
Contributor Author

failures are flake this is partially trying to address

@tommyp1ckles
Copy link
Contributor Author

/test

1 similar comment
@tommyp1ckles
Copy link
Contributor Author

/test

@tommyp1ckles tommyp1ckles force-pushed the pr/tp/fix-false-deepcopy-causing-races branch from 4b4e8d0 to 5383cea Compare August 15, 2023 05:22
@tommyp1ckles tommyp1ckles force-pushed the pr/tp/fix-false-deepcopy-causing-races branch from 5383cea to c4b8d3e Compare August 30, 2023 05:50
@tommyp1ckles
Copy link
Contributor Author

/test

@tommyp1ckles
Copy link
Contributor Author

Think this is still desired, if tests pass following the last fix for the referenced test.

@tommyp1ckles tommyp1ckles force-pushed the pr/tp/fix-false-deepcopy-causing-races branch from c4b8d3e to d2c24d0 Compare September 16, 2023 04:10
@christarazi
Copy link
Member

@hemanthmalla Could you take a look on behalf of cilium/azure?

Copy link
Member

@hemanthmalla hemanthmalla left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM for Azure

@tommyp1ckles tommyp1ckles added this pull request to the merge queue Mar 15, 2024
@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Mar 15, 2024
Merged via the queue into cilium:main with commit 1ca6141 Mar 15, 2024
62 checks passed
@tommyp1ckles tommyp1ckles deleted the pr/tp/fix-false-deepcopy-causing-races branch March 15, 2024 19:53
@tommyp1ckles tommyp1ckles added needs-backport/1.13 This PR / issue needs backporting to the v1.13 branch needs-backport/1.14 This PR / issue needs backporting to the v1.14 branch needs-backport/1.15 This PR / issue needs backporting to the v1.15 branch labels Mar 15, 2024
@gandro gandro mentioned this pull request Mar 19, 2024
21 tasks
@gandro gandro added backport-pending/1.15 The backport for Cilium 1.15.x for this PR is in progress. and removed needs-backport/1.15 This PR / issue needs backporting to the v1.15 branch labels Mar 19, 2024
@gandro gandro mentioned this pull request Mar 19, 2024
10 tasks
@gandro gandro added backport-pending/1.14 The backport for Cilium 1.14.x for this PR is in progress. and removed needs-backport/1.14 This PR / issue needs backporting to the v1.14 branch labels Mar 19, 2024
@gandro gandro mentioned this pull request Mar 19, 2024
8 tasks
@gandro gandro added backport-pending/1.13 The backport for Cilium 1.13.x for this PR is in progress. and removed needs-backport/1.13 This PR / issue needs backporting to the v1.13 branch labels Mar 19, 2024
@github-actions github-actions bot added backport-done/1.14 The backport for Cilium 1.14.x for this PR is done. backport-done/1.13 The backport for Cilium 1.13.x for this PR is done. backport-done/1.15 The backport for Cilium 1.15.x for this PR is done. and removed backport-pending/1.14 The backport for Cilium 1.14.x for this PR is in progress. backport-pending/1.13 The backport for Cilium 1.13.x for this PR is in progress. backport-pending/1.15 The backport for Cilium 1.15.x for this PR is in progress. labels Mar 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/ipam Impacts IP address management functionality. backport-done/1.13 The backport for Cilium 1.13.x for this PR is done. backport-done/1.14 The backport for Cilium 1.14.x for this PR is done. backport-done/1.15 The backport for Cilium 1.15.x for this PR is done. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/ci This PR makes changes to the CI.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants