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

feat: add route-controller to linode ccm #184

Merged
merged 6 commits into from
Mar 13, 2024
Merged

Conversation

rahulait
Copy link
Collaborator

@rahulait rahulait commented Mar 1, 2024

What type of PR is this?
/kind feature

What this PR does / why we need it:
Cloud-controller-manager provides interfaces to implement functionality of managing routes in cluster. For k8s clusters provisioned within VPC, we need to automatically add/remove podCIDRs allocated to nodes to the linode's interface configs so that the traffic can be routed within the VPC. This PR implements the Routes interfaces.

This PR also makes sure that if a node is a part of VPC, then VPC ip is shown as the node internal ip.

root@rah6-control-plane-bjwwd:~# k get nodes -o wide
NAME                       STATUS   ROLES           AGE    VERSION   INTERNAL-IP   EXTERNAL-IP      OS-IMAGE             KERNEL-VERSION      CONTAINER-RUNTIME
rah6-control-plane-bjwwd   Ready    control-plane   14m    v1.29.2   10.0.0.2      172.232.1.230    Ubuntu 22.04.4 LTS   5.15.0-94-generic   containerd://1.7.2
rah6-md-0-zrzwr            Ready    <none>          12m    v1.29.2   10.0.0.4      172.233.209.90   Ubuntu 22.04.4 LTS   5.15.0-94-generic   containerd://1.7.2

root@rah6-control-plane-bjwwd:~# k get node rah6-control-plane-bjwwd -o yaml | yq .status.addresses
- address: rah6-control-plane-bjwwd
  type: Hostname
- address: 172.232.1.230
  type: ExternalIP
- address: 10.0.0.2
  type: InternalIP
- address: 192.168.132.88
  type: InternalIP
root@rah6-control-plane-bjwwd:~#

root@rah6-control-plane-bjwwd:~# k get node rah6-control-plane-bjwwd -o yaml | yq .spec.providerID
linode://55914087
root@rah6-control-plane-bjwwd:~# k get node rah6-control-plane-bjwwd -o yaml | yq .spec.podCIDRs
- 10.192.0.0/24
root@rah6-control-plane-bjwwd:~# curl -s --header "Authorization: Bearer $LINODE_API_TOKEN" -X GET https://api.linode.com/v4/linode/instances/55914087/configs | yq .data[0].interfaces[].ip_ranges
null
["10.192.0.0/24"]
root@rah6-control-plane-bjwwd:~#

root@rah6-control-plane-bjwwd:~# k get node rah6-md-0-zrzwr -o yaml | yq .status.addresses
- address: rah6-md-0-zrzwr
  type: Hostname
- address: 172.233.209.90
  type: ExternalIP
- address: 10.0.0.4
  type: InternalIP
- address: 192.168.132.220
  type: InternalIP
root@rah6-control-plane-bjwwd:~# k get node rah6-md-0-zrzwr -o yaml | yq .spec.podCIDRs
- 10.192.3.0/24
root@rah6-control-plane-bjwwd:~# k get node rah6-md-0-zrzwr -o yaml | yq .spec.providerID
linode://55914178
root@rah6-control-plane-bjwwd:~# curl -s --header "Authorization: Bearer $LINODE_API_TOKEN" -X GET https://api.linode.com/v4/linode/instances/55914178/configs | yq .data[0].interfaces[].ip_ranges
null
["10.192.3.0/24"]
root@rah6-control-plane-bjwwd:~#

Key points:

  • Implements routes interface
  • Disabled by default and can be enabled with certain helm flags
  • Uses cache to avoid hitting linode API too many times
  • VPC ip set as node internal ip

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes # #183

Special notes for your reviewer:

TODO:
Add unittests

General:

  • Have you removed all sensitive information, including but not limited to access keys and passwords?
  • Have you checked to ensure there aren't other open or closed Pull Requests for the same bug/feature/question?

Pull Request Guidelines:

  1. Does your submission pass tests?
  2. Have you added tests?
  3. Are you addressing a single feature in this PR?
  4. Are your commits atomic, addressing one change per commit?
  5. Are you following the conventions of the language?
  6. Have you saved your large formatting changes for a different PR, so we can focus on your work?
  7. Have you explained your rationale for why this feature is needed?
  8. Have you linked your PR to an open issue

@rahulait
Copy link
Collaborator Author

rahulait commented Mar 2, 2024

Hitting linode/linodego#462 with linodego. It doesn't affect the PR as the interface also gets deleted when a node is deleted and we don't need DeleteRoute() call to always work, but it would be nice if linodego is fixed.

@rahulait rahulait marked this pull request as ready for review March 2, 2024 06:49
cloud/linode/route_controller.go Outdated Show resolved Hide resolved
cloud/linode/route_controller.go Outdated Show resolved Hide resolved
cloud/linode/route_controller.go Outdated Show resolved Hide resolved
cloud/linode/route_controller.go Show resolved Hide resolved
cloud/linode/route_controller.go Outdated Show resolved Hide resolved
cloud/linode/route_controller.go Outdated Show resolved Hide resolved
cloud/linode/route_controller.go Outdated Show resolved Hide resolved
cloud/linode/route_controller.go Outdated Show resolved Hide resolved
cloud/linode/route_controller.go Outdated Show resolved Hide resolved
cloud/linode/route_controller.go Outdated Show resolved Hide resolved
cloud/linode/route_controller.go Outdated Show resolved Hide resolved
@rahulait rahulait force-pushed the add-route-controller branch 3 times, most recently from 7e5f575 to 2b413b0 Compare March 11, 2024 21:24
cloud/linode/instances.go Outdated Show resolved Hide resolved
cloud/linode/instances.go Outdated Show resolved Hide resolved
cloud/linode/instances.go Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
rc.routes = make(map[int][]linodego.InstanceConfig, len(instances))

mtx := sync.Mutex{}
g := new(errgroup.Group)
Copy link
Contributor

Choose a reason for hiding this comment

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

Right last thing to note, we may want to limit the number of go routines we can create or do some sort of throttling or the API team might be unhappy with us.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It looks like we can do this by setting g.SetLimit(<some number>). Not sure what a good number is to start with. Should we set it to something like 100?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We're going to merge this PR and address this in followup PRs. I've created #188 to address it.

@AshleyDumaine AshleyDumaine self-requested a review March 13, 2024 14:57
@rahulait rahulait merged commit acb82b8 into main Mar 13, 2024
6 checks passed
@tchinmai7 tchinmai7 deleted the add-route-controller branch March 13, 2024 17:02
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.

5 participants