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

Allow referencing the NLB and the target nodepool by name, and allow changing target instance-pool #102

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

Conversation

Sapd
Copy link
Contributor

@Sapd Sapd commented Feb 14, 2025

Description

There are no breaking changes.

Implemented:

Support for referencing nlb by name (for external ones) and sks nodepool by name

When using the API or IaaS its quite difficult to handle or specify ids.
As explained here: #96

Referencing the load balancer by name

For referencing the load balancer, no new annotation was implemented. It just uses the existing annotation

service.beta.kubernetes.io/exoscale-loadbalancer-name

Will be used when it is specified AND exoscale-loadbalancer-external is set to true.
If name is not specified exoscale-loadbalancer-id can still be used to achieve the same thing like before

Referencing the sks node pool by name instead of instancepool-id

Implemented

service.beta.kubernetes.io/exoscale-loadbalancer-service-sks-nodepool-name

when that is specified it will fill out the instancepool id automatically which matches the provided sks-nodepool name.

for that the user has also to specify

service.beta.kubernetes.io/exoscale-sks-cluster-name

which is only required when using this new name-annotation.

Ofc the old/existing annotation exoscale-loadbalancer-service-instancepool-id can still be used as before.

Implementation

Implementation is quite simple. If names are specified it will do an additional API call at the beginning of EnsureLoadBalancer. It will then use the result to patch the ids which the user would otherwise have to provide.

Allow changing the target instance pool without destroying load balancer

When updating a service, changing the instance pool id did nothing: #95

It was also documented in the readme:

the Instance Pool cannot be changed after NLB service creation – the
K8s Service will have to be deleted and re-created with the annotation
updated.

That is especially bad when using a non-external load balancer as it would force one to delete it - loosing its IP.

That is because the implementation just calls the operation update-load-balancer which does not take a new instance pool as parameter, so it was silently ignored: https://openapi-v2.exoscale.com/operation/operation-update-load-balancer-service

To fix it, in the end of updateLoadBalancer I check if the old InstancePoolID and new one differ. If yes it will delete and recreate the NLB service.

Other changes

Updated some comments in the functions I changed for more clarity. In the docs/example I also marked using "external" as recommended, so that unexperienced users do not loose a NLB on accident.

Checklist

(For exoscale contributors)

  • Changelog updated (under Unreleased block)
  • Testing

Testing

Runned both pytest and gmake test

pytest

Runs through. Just the usual error because of Kubernetes Version mismatch (the one from Exoscale too new) and cleanup does not run through (nginx ingress) as in tests before. However NLB tests are all working.

❯ pytest
================================================================================== test session starts ==================================================================================
platform darwin -- Python 3.11.11, pytest-8.3.4, pluggy-1.5.0
rootdir: /Users/denis/Documents/GitHub/exoscale-cloud-controller-manager
plugins: anyio-3.6.2
collected 72 items

test/tests_0_environment/test_001_environment.py .....                                                                                                                            [  6%]
test/tests_1_control_plane/test_101_control_plane.py .                                                                                                                            [  8%]
test/tests_1_control_plane/test_102_control_plane_files.py ......                                                                                                                 [ 16%]
test/tests_1_control_plane/test_103_control_plane_start.py F.                                                                                                                     [ 19%]
test/tests_2_nodes/test_201_nodes.py .                                                                                                                                            [ 20%]
test/tests_2_nodes/test_202_nodes_files.py s.                                                                                                                                     [ 23%]
test/tests_3_ccm/test_300_init.py .                                                                                                                                               [ 25%]
test/tests_3_ccm/test_301_k8s_state.py ..                                                                                                                                         [ 27%]
test/tests_3_ccm/test_302_ccm_start.py .....                                                                                                                                      [ 34%]
test/tests_3_ccm/test_303_ccm_node_csr_validation.py .                                                                                                                            [ 36%]
test/tests_3_ccm/test_304_ccm_node_csr_invalid.py ss                                                                                                                              [ 38%]
test/tests_3_ccm/test_305_ccm_nodes.py x                                                                                                                                          [ 40%]
test/tests_3_ccm/test_306_ccm_api_credentials_update.py ..                                                                                                                        [ 43%]
test/tests_3_ccm/test_307_k8s_nodes.py x.x                                                                                                                                        [ 47%]
test/tests_4_nlb/test_400_init.py ..                                                                                                                                              [ 50%]
test/tests_4_nlb/test_401_nlb_hello_external.py .....                                                                                                                             [ 56%]
test/tests_4_nlb/test_402_nlb_ingress_nginx.py .......                                                                                                                            [ 66%]
test/tests_4_nlb/test_403_nlb_hello_ingress.py ..                                                                                                                                 [ 69%]
test/tests_4_nlb/test_404_nlb_echo_udp.py ...                                                                                                                                     [ 73%]
test/tests_5_nodes_pool_resize/test_500_init.py ..                                                                                                                                [ 76%]
test/tests_5_nodes_pool_resize/test_501_k8s_state.py ...                                                                                                                          [ 80%]
test/tests_5_nodes_pool_resize/test_502_ccm_nodes_deletion.py s                                                                                                                   [ 81%]
test/tests_5_nodes_pool_resize/test_503_ccm_nodes_csr_validation.py .                                                                                                             [ 83%]
test/tests_5_nodes_pool_resize/test_504_k8s_nodes.py .                                                                                                                            [ 84%]
test/tests_5_nodes_pool_resize/test_505_nlb_hello.py ..                                                                                                                           [ 87%]
test/tests_5_nodes_pool_resize/test_501_k8s_state.py ...                                                                                                                          [ 91%]
test/tests_5_nodes_pool_resize/test_502_ccm_nodes_deletion.py X                                                                                                                   [ 93%]
test/tests_5_nodes_pool_resize/test_503_ccm_nodes_csr_validation.py s                                                                                                             [ 94%]
test/tests_5_nodes_pool_resize/test_504_k8s_nodes.py .                                                                                                                            [ 95%]
test/tests_5_nodes_pool_resize/test_505_nlb_hello.py ..                                                                                                                           [ 98%]
test/tests_9_the_end/test_999_finalize.py .^C
gmake test

Runs through

❯ gmake test
go.mk/version.mk:13: warning: overriding recipe for target 'get-version-tag'
/Users/denis/Documents/GitHub/exoscale-cloud-controller-manager/go.mk/version.mk:13: warning: ignoring old recipe for target 'get-version-tag'
go.mk/version.mk:13: warning: overriding recipe for target 'get-version-tag'
/Users/denis/Documents/GitHub/exoscale-cloud-controller-manager/go.mk/version.mk:13: warning: ignoring old recipe for target 'get-version-tag'
'/opt/homebrew/bin/go' test \
-race \
-timeout 15s \
 \
github.com/exoscale/exoscale-cloud-controller-manager/exoscale
ok  	github.com/exoscale/exoscale-cloud-controller-manager/exoscale	5.958s

@Sapd Sapd changed the title Update CHANGELOG.md Allow referencing the NLB and the target nodepool by name Feb 14, 2025
@Sapd Sapd changed the title Allow referencing the NLB and the target nodepool by name Allow referencing the NLB and the target nodepool by name, and allow changing target instance-pool Feb 14, 2025
@pierre-emmanuelJ pierre-emmanuelJ requested a review from a team March 7, 2025 14:18
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.

4 participants