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

create ip holder per cluster + tests, update docs #246

Merged
merged 1 commit into from
Nov 26, 2024

Conversation

rosskirkpat
Copy link
Contributor

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

@rosskirkpat rosskirkpat self-assigned this Nov 18, 2024
Copy link

codecov bot commented Nov 18, 2024

Codecov Report

Attention: Patch coverage is 77.02703% with 17 lines in your changes missing coverage. Please review.

Project coverage is 56.63%. Comparing base (66afcae) to head (b14d4a1).
Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
cloud/linode/cilium_loadbalancers.go 81.13% 8 Missing and 2 partials ⚠️
cloud/linode/cloud.go 37.50% 4 Missing and 1 partial ⚠️
cloud/linode/loadbalancers.go 84.61% 0 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #246      +/-   ##
==========================================
+ Coverage   56.08%   56.63%   +0.54%     
==========================================
  Files          12       12              
  Lines        2350     2412      +62     
==========================================
+ Hits         1318     1366      +48     
- Misses        881      892      +11     
- Partials      151      154       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

main.go Show resolved Hide resolved
README.md Show resolved Hide resolved
@AshleyDumaine AshleyDumaine added the added-feature for new features in the changelog. label Nov 19, 2024
Copy link
Member

@AshleyDumaine AshleyDumaine left a comment

Choose a reason for hiding this comment

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

LGTM 👍

Copy link
Collaborator

@rahulait rahulait left a comment

Choose a reason for hiding this comment

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

lgtm

@rosskirkpat
Copy link
Contributor Author

I was able to perform e2e validation of this CCM PR for a LKE Cluster ross-ccm-pr246-e2e.

linode-ccm-ip-holder-us-ord-ross-ccm-pr246-e2e is the ip-holder nanode label with an id of 67381594

CCM successfully shared IPs for the four test nginx LB services that I created (ccm logs w/ v5 verbosity below).

# ccm args
  - args:
    - --v=5
    - --secure-port=10253
    - --webhook-secure-port=0
    - --kubeconfig=xxxxxx
    - --enable-route-controller=true
    - --vpc-name=ross-ccm-pr246-e2e
    - --configure-cloud-routes=true
    - --cluster-cidr=10.248.0.0/14
    - --load-balancer-type=cilium-bgp
    - --ip-holder-suffix=ross-ccm-pr246-e2e
    - --leader-elect=true
    - --leader-elect-resource-lock=leases
% KUBECONFIG=/Users/rkirkpat/.kube/clusters/ross-ccm-pr246-e2e.yaml k get svc -A -o wide
NAMESPACE     NAME         TYPE           CLUSTER-IP       EXTERNAL-IP       PORT(S)                  AGE     SELECTOR
default       kubernetes   ClusterIP      10.252.0.1       <none>            443/TCP                  4h47m   <none>
kube-system   kube-dns     ClusterIP      10.252.0.10      <none>            53/UDP,53/TCP,9153/TCP   4h47m   k8s-app=kube-dns
test          nginx        LoadBalancer   10.252.11.57     172.234.194.145   80:31537/TCP             4h42m   app=nginx
test1         nginx1       LoadBalancer   10.253.150.119   172.232.10.99     80:30448/TCP             17s     app=nginx1
test2         nginx2       LoadBalancer   10.252.192.57    172.232.10.193    80:30400/TCP             17s     app=nginx2
test3         nginx3       LoadBalancer   10.253.106.41    172.234.212.223   80:31243/TCP             17s     app=nginx3
% linode-cli linodes ips-list 67381594

┌─────────────────┬──────┬────────┬──────────────────────────────────────────┬────────┬───────────┐
│ address         │ type │ public │ rdns                                     │ region │ linode_id │
├─────────────────┼──────┼────────┼──────────────────────────────────────────┼────────┼───────────┤
│ 172.232.10.99   │ ipv4 │ True   │ 172-232-10-99.ip.linodeusercontent.com   │ us-ord │ 67381594  │
├─────────────────┼──────┼────────┼──────────────────────────────────────────┼────────┼───────────┤
│ 172.232.10.193  │ ipv4 │ True   │ 172-232-10-193.ip.linodeusercontent.com  │ us-ord │ 67381594  │
├─────────────────┼──────┼────────┼──────────────────────────────────────────┼────────┼───────────┤
│ 172.234.194.144 │ ipv4 │ True   │ 172-234-194-144.ip.linodeusercontent.com │ us-ord │ 67381594  │
├─────────────────┼──────┼────────┼──────────────────────────────────────────┼────────┼───────────┤
│ 172.234.194.145 │ ipv4 │ True   │ 172-234-194-145.ip.linodeusercontent.com │ us-ord │ 67381594  │
├─────────────────┼──────┼────────┼──────────────────────────────────────────┼────────┼───────────┤
│ 172.234.212.223 │ ipv4 │ True   │ 172-234-212-223.ip.linodeusercontent.com │ us-ord │ 67381594  │
└─────────────────┴──────┴────────┴──────────────────────────────────────────┴────────┴───────────┘
# test/nginx - first service
2024-11-20T20:44:39.474162665Z I1120 20:44:39.474075       1 controller.go:398] Ensuring load balancer for service test/nginx
2024-11-20T20:44:39.474179875Z I1120 20:44:39.474125       1 controller.go:954] Adding finalizer to service test/nginx
2024-11-20T20:44:39.474361885Z I1120 20:44:39.474281       1 event.go:376] "Event occurred" object="test/nginx" fieldPath="" kind="Service" apiVersion="v1" type="Normal" reason="EnsuringLoadBalancer" message="Ensuring load balancer"
2024-11-20T20:44:39.478502821Z I1120 20:44:39.478327       1 loadbalancers.go:202] handling LoadBalancer Service test/nginx as io.cilium/bgp-control-plane
2024-11-20T20:44:39.507988296Z I1120 20:44:39.507897       1 cilium_loadbalancers.go:570] Creating CiliumBGPPeeringPolicy
2024-11-20T20:44:39.515700108Z I1120 20:44:39.515593       1 loadbalancers.go:228] using parameter-based IP Holder suffix ross-ccm-pr246-e2e for Service test/nginx
2024-11-20T20:44:39.589834763Z I1120 20:44:39.589728       1 cilium_loadbalancers.go:404] generated IP Holder Linode label: linode-ccm-ip-holder-us-ord-ross-ccm-pr246-e2e
2024-11-20T20:44:39.660945974Z I1120 20:44:39.660841       1 cilium_loadbalancers.go:404] generated IP Holder Linode label: linode-ccm-ip-holder-us-ord-ross-ccm-pr246-e2e
2024-11-20T20:44:40.689065302Z I1120 20:44:40.688946       1 cilium_loadbalancers.go:346] created new IP Holder instance linode-ccm-ip-holder-us-ord-ross-ccm-pr246-e2e
2024-11-20T20:44:41.185269749Z I1120 20:44:41.185011       1 cilium_loadbalancers.go:142] shared IPs [172.234.194.145] on Linode 67381460
2024-11-20T20:44:41.394766903Z I1120 20:44:41.394641       1 cilium_loadbalancers.go:142] shared IPs [172.234.194.145] on Linode 67381497
2024-11-20T20:44:41.631458439Z I1120 20:44:41.631197       1 cilium_loadbalancers.go:142] shared IPs [172.234.194.145] on Linode 67381498
2024-11-20T20:44:41.635326205Z I1120 20:44:41.635231       1 controller.go:887] Finished syncing service "test/nginx" (2.161190041s)
2024-11-20T20:44:41.635398945Z I1120 20:44:41.635315       1 event.go:376] "Event occurred" object="test/nginx" fieldPath="" kind="Service" apiVersion="v1" type="Normal" reason="EnsuredLoadBalancer" message="Ensured load balancer"

# test1/nginx1
2024-11-21T01:27:17.375533272Z I1121 01:27:17.375405       1 controller.go:398] Ensuring load balancer for service test1/nginx1
2024-11-21T01:27:17.375546472Z I1121 01:27:17.375447       1 controller.go:954] Adding finalizer to service test1/nginx1
2024-11-21T01:27:17.375670722Z I1121 01:27:17.375612       1 event.go:376] "Event occurred" object="test1/nginx1" fieldPath="" kind="Service" apiVersion="v1" type="Normal" reason="EnsuringLoadBalancer" message="Ensuring load balancer"
2024-11-21T01:27:17.379937379Z I1121 01:27:17.379840       1 loadbalancers.go:202] handling LoadBalancer Service test1/nginx1 as io.cilium/bgp-control-plane
2024-11-21T01:27:17.407794381Z I1121 01:27:17.407722       1 loadbalancers.go:228] using parameter-based IP Holder suffix ross-ccm-pr246-e2e for Service test1/nginx1
2024-11-21T01:27:17.534516385Z I1121 01:27:17.534410       1 cilium_loadbalancers.go:404] generated IP Holder Linode label: linode-ccm-ip-holder-us-ord-ross-ccm-pr246-e2e
2024-11-21T01:27:18.439290785Z I1121 01:27:18.439175       1 cilium_loadbalancers.go:142] shared IPs [172.232.10.99 172.234.194.145] on Linode 67381497
2024-11-21T01:27:18.629980569Z I1121 01:27:18.629878       1 cilium_loadbalancers.go:142] shared IPs [172.232.10.99 172.234.194.145] on Linode 67381498
2024-11-21T01:27:18.863495941Z I1121 01:27:18.863409       1 cilium_loadbalancers.go:142] shared IPs [172.232.10.99 172.234.194.145] on Linode 67381460
2024-11-21T01:27:18.867289168Z I1121 01:27:18.867185       1 controller.go:887] Finished syncing service "test1/nginx1" (1.491799725s)
2024-11-21T01:27:18.867312558Z I1121 01:27:18.867234       1 event.go:376] "Event occurred" object="test1/nginx1" fieldPath="" kind="Service" apiVersion="v1" type="Normal" reason="EnsuredLoadBalancer" message="Ensured load balancer"

# test2/nginx2
2024-11-21T01:27:18.867301148Z I1121 01:27:18.867223       1 controller.go:398] Ensuring load balancer for service test2/nginx2
2024-11-21T01:27:18.867306128Z I1121 01:27:18.867235       1 controller.go:954] Adding finalizer to service test2/nginx2
2024-11-21T01:27:18.867395639Z I1121 01:27:18.867324       1 event.go:376] "Event occurred" object="test2/nginx2" fieldPath="" kind="Service" apiVersion="v1" type="Normal" reason="EnsuringLoadBalancer" message="Ensuring load balancer"
2024-11-21T01:27:18.873495860Z I1121 01:27:18.873432       1 loadbalancers.go:202] handling LoadBalancer Service test2/nginx2 as io.cilium/bgp-control-plane
2024-11-21T01:27:18.878336408Z I1121 01:27:18.878237       1 loadbalancers.go:228] using parameter-based IP Holder suffix ross-ccm-pr246-e2e for Service test2/nginx2
2024-11-21T01:27:19.093526207Z I1121 01:27:19.093418       1 cilium_loadbalancers.go:404] generated IP Holder Linode label: linode-ccm-ip-holder-us-ord-ross-ccm-pr246-e2e
2024-11-21T01:27:19.888952577Z I1121 01:27:19.888840       1 cilium_loadbalancers.go:142] shared IPs [172.232.10.193 172.234.194.145 172.232.10.99] on Linode 67381460
2024-11-21T01:27:20.289297847Z I1121 01:27:20.289164       1 cilium_loadbalancers.go:142] shared IPs [172.232.10.193 172.234.194.145 172.232.10.99] on Linode 67381497
2024-11-21T01:27:20.594113941Z I1121 01:27:20.594031       1 cilium_loadbalancers.go:142] shared IPs [172.232.10.193 172.234.194.145 172.232.10.99] on Linode 67381498
2024-11-21T01:27:20.597783828Z I1121 01:27:20.597716       1 controller.go:887] Finished syncing service "test2/nginx2" (1.730502931s)

# test3/nginx3
2024-11-21T01:27:20.597802248Z I1121 01:27:20.597757       1 controller.go:398] Ensuring load balancer for service test3/nginx3
2024-11-21T01:27:20.597807888Z I1121 01:27:20.597774       1 controller.go:954] Adding finalizer to service test3/nginx3
2024-11-21T01:27:20.597911198Z I1121 01:27:20.597859       1 event.go:376] "Event occurred" object="test3/nginx3" fieldPath="" kind="Service" apiVersion="v1" type="Normal" reason="EnsuringLoadBalancer" message="Ensuring load balancer"
2024-11-21T01:27:20.602067645Z I1121 01:27:20.602002       1 loadbalancers.go:202] handling LoadBalancer Service test3/nginx3 as io.cilium/bgp-control-plane
2024-11-21T01:27:20.607685146Z I1121 01:27:20.607591       1 loadbalancers.go:228] using parameter-based IP Holder suffix ross-ccm-pr246-e2e for Service test3/nginx3
2024-11-21T01:27:20.915749196Z I1121 01:27:20.915649       1 cilium_loadbalancers.go:404] generated IP Holder Linode label: linode-ccm-ip-holder-us-ord-ross-ccm-pr246-e2e
2024-11-21T01:27:21.941528232Z I1121 01:27:21.941412       1 cilium_loadbalancers.go:142] shared IPs [172.234.212.223 172.234.194.145 172.232.10.99 172.232.10.193] on Linode 67381460
2024-11-21T01:27:22.332218995Z I1121 01:27:22.332073       1 cilium_loadbalancers.go:142] shared IPs [172.234.212.223 172.234.194.145 172.232.10.99 172.232.10.193] on Linode 67381497
2024-11-21T01:27:22.978518271Z I1121 01:27:22.978393       1 cilium_loadbalancers.go:142] shared IPs [172.234.212.223 172.234.194.145 172.232.10.99 172.232.10.193] on Linode 67381498
2024-11-21T01:27:22.982149548Z I1121 01:27:22.982029       1 controller.go:887] Finished syncing service "test3/nginx3" (2.38428759s)
2024-11-21T01:27:22.982181658Z I1121 01:27:22.982067       1 event.go:376] "Event occurred" object="test3/nginx3" fieldPath="" kind="Service" apiVersion="v1" type="Normal" reason="EnsuredLoadBalancer" message="Ensured load balancer"

@rosskirkpat
Copy link
Contributor Author

Validation details for latest changes where ccm exits during cloud provider initialization if the ip-holder-suffix is >23 characters.

Linode Cloud Controller Manager starting up
SENTRY_DSN not set, not initializing Sentry
I1125 18:53:09.419900       1 serving.go:380] Generated self-signed cert in-memory
I1125 18:53:09.592880       1 client.go:73] Linode client created with default timeout of 2m0s
W1125 18:53:09.592901       1 cloud.go:96] vpc-name flag is deprecated. Use vpc-names instead
I1125 18:53:09.592906       1 route_controller.go:72] TTL for routeCache set to 60 seconds
I1125 18:53:09.592911       1 instances.go:133] TTL for nodeCache set to 15
I1125 18:53:09.592917       1 cloud.go:114] Using IP holder suffix 'OaTJrRuufacHVougjwkpBpmstiqvswvBNEMWXsRYfMBTCkKIUTXpbGIcIbDWSQp'
E1125 18:53:09.592921       1 cloud.go:118] ip-holder-suffix must be 23 characters or less. OaTJrRuufacHVougjwkpBpmstiqvswvBNEMWXsRYfMBTCkKIUTXpbGIcIbDWSQp is 63 characters
F1125 18:53:09.592939       1 main.go:162] Cloud provider could not be initialized: could not init cloud provider "linode": ip-holder-suffix must be 23 characters or less. OaTJrRuufacHVougjwkpBpmstiqvswvBNEMWXsRYfMBTCkKIUTXpbGIcIbDWSQp is 63 characters

Copy link
Collaborator

@rahulait rahulait left a comment

Choose a reason for hiding this comment

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

LGTM

@rosskirkpat rosskirkpat merged commit 3510bc4 into main Nov 26, 2024
7 checks passed
@rosskirkpat rosskirkpat deleted the lke5823-ip-holder-per-cluster branch November 26, 2024 00:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
added-feature for new features in the changelog.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants