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

Move github.com/cilium/ipam packages to main repo #25289

Merged
merged 3 commits into from May 12, 2023

Conversation

tklauser
Copy link
Member

@tklauser tklauser commented May 5, 2023

First 3 commits are preparatory to update the vendored package, allow use of Cilium's concurrency-safe PRNG type in the new package and allow k8s authors in file header copyright notices.

The fourth commit moves the packages. Quoting the commit message:

ipam, vendor: move github.com/cilium/ipam packages to main repo

Instead of keeping a separate repo for these packages move them to the
main cilium repo which makes this easier to maintain and iterate on.

The code was originally forked from
k8s.io/kubernetes/pkg/registry/core/service and has since been extended
and changed to fit Cilium's needs.

This is a copy of the github.com/cilium/ipam repo at commit.
https://github.com/cilium/ipam/commit/fd66eae7909b3d3962dfc8887805181462a7e384
This corresponds to the version that is currently vendored.
Additionally, the test files are copied and unnecessry BUILD files
(stemming from upstream k8s) were removed. Some modifications to the
lock and rand types used within the packages needed to be made:

 * Use mutex types from pkg/lock for deadlock detection
 * Use pkg/rand for concurrency safe PRNG
 * Change the license/copyright format in file headers to conform to Cilium
   project requirements.

Otherwise linters would complain about the use of the Go stdlib types.

@tklauser tklauser added release-note/misc This PR makes changes that have no direct user impact. sig/ipam IP address management, including cloud IPAM labels May 5, 2023
@tklauser
Copy link
Member Author

tklauser commented May 5, 2023

/test

@tklauser tklauser force-pushed the pr/tklauser/merge-cilium-ipam branch from 4574364 to 1d06ec9 Compare May 5, 2023 14:24
@tklauser
Copy link
Member Author

tklauser commented May 5, 2023

/test

@tklauser tklauser force-pushed the pr/tklauser/merge-cilium-ipam branch 6 times, most recently from 2e90303 to 7c2cc13 Compare May 8, 2023 09:52
@tklauser
Copy link
Member Author

tklauser commented May 8, 2023

/test

@tklauser
Copy link
Member Author

/test

@tklauser tklauser marked this pull request as ready for review May 10, 2023 10:22
@tklauser tklauser requested review from a team as code owners May 10, 2023 10:22
Copy link
Member

@gandro gandro left a comment

Choose a reason for hiding this comment

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

Thanks for splitting it into multiple commits. Very easy to review!

@tklauser
Copy link
Member Author

tklauser commented May 10, 2023

/test-runtime

#25291 https://jenkins.cilium.io/job/Cilium-PR-Runtime-net-next/6575/

Job 'Cilium-PR-K8s-1.26-kernel-net-next' failed:

Click to show.

Test Name

K8sDatapathServicesTest Checks N/S loadbalancing With host policy Tests NodePort

Failure Output

FAIL: Can not connect to service "tftp://192.168.56.11:30013/hello" from outside cluster (9/10)

Jenkins URL: https://jenkins.cilium.io/job/Cilium-PR-K8s-1.26-kernel-net-next/2192/

If it is a flake and a GitHub issue doesn't already exist to track it, comment /mlh new-flake Cilium-PR-K8s-1.26-kernel-net-next so I can create one.

Then please upload the Jenkins artifacts to that issue.

@tklauser
Copy link
Member Author

/test-1.26-net-next

Copy link
Member

@rolinh rolinh left a comment

Choose a reason for hiding this comment

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

Vendor changes lgtm 🎉

@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 May 12, 2023
In order to be able to assign the return value of NewSafeRand to a typed
variable or a struct member, the type needs to be exported. This will be
needed in a successive commit.

Signed-off-by: Tobias Klauser <tobias@cilium.io>
We want to retain the k8s author copyright notice for these files.

Signed-off-by: Tobias Klauser <tobias@cilium.io>
Instead of keeping a separate repo for these packages move them to the
main cilium repo which makes this easier to maintain and iterate on.

The code was originally forked from
k8s.io/kubernetes/pkg/registry/core/service and has since been extended
and changed to fit Cilium's needs.

This is a copy of the github.com/cilium/ipam repo at commit
cilium/ipam@fd66eae
This corresponds to the version that is currently vendored.
Additionally, the test files are copied and unnecessry BUILD files
(stemming from upstream k8s) were removed. Some modifications to the
lock and rand types used within the packages needed to be made:

 * Use mutex types from pkg/lock for deadlock detection
 * Use pkg/rand for concurrency safe PRNG
 * Change the license/copyright format in file headers to conform to Cilium
   project requirements.

Otherwise linters would complain about the use of the Go stdlib types.

Signed-off-by: Tobias Klauser <tobias@cilium.io>
@tklauser tklauser force-pushed the pr/tklauser/merge-cilium-ipam branch from ef38d1d to 60a769e Compare May 12, 2023 15:58
@tklauser
Copy link
Member Author

/test

@tklauser tklauser removed the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label May 12, 2023
@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 May 12, 2023
@tklauser tklauser merged commit fd7b531 into main May 12, 2023
58 checks passed
@tklauser tklauser deleted the pr/tklauser/merge-cilium-ipam branch May 12, 2023 19:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/misc This PR makes changes that have no direct user impact. sig/ipam IP address management, including cloud IPAM
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants