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

Initial loadbalancer implementation #5

Merged
merged 27 commits into from
Nov 1, 2023
Merged

Initial loadbalancer implementation #5

merged 27 commits into from
Nov 1, 2023

Conversation

href
Copy link
Contributor

@href href commented Oct 6, 2023

See internal ticket for details.

The default API timeout was ignored. It is now also set to 15s instead
of 5s, as the latter proofed too short for some loadbalancer calls.
This is a first commit that implements the necessary logic and adds
basic unit tests. There are some missing features and there are no
integration tests yet.
@href href requested a review from alakae October 6, 2023 12:14
@href href changed the title Initial load balancer implementation Initial loadbalancer implementation Oct 6, 2023
@href href force-pushed the denis/lbs branch 2 times, most recently from b1b7dee to 481bceb Compare October 19, 2023 12:28
href added 2 commits October 19, 2023 14:34
Some commands seem to time out occiasionally, even though nothing
is wrong. Waiting a little longer seems okay, but this should be
the limit. If the timeout fails, the operation will be retried,
which might be preferrable.
href added 7 commits October 20, 2023 15:39
This allows for cleaner instantiation via cloud.go
Those need to be reusable for integration tests.
The kubernetes project states the following:

> Kubernetes reserves all labels and annotations in
> the kubernetes.io and k8s.io namespaces.

Therefore, using a namespace "owned" by cloudscale is the most
logical step. The following format is used from now on:

  k8s.cloudscale.ch/<type>-<key>

For example:

  k8s.cloudscale.ch/loadbalancer-uuid
All options should now be configurable via annotations and cause the
right actions to be taken. In most cases, the change is rendered
immediately.
href added 2 commits October 25, 2023 15:53
With this change, a forward-compatible addition has been added that
future listeners without associated pools will also be managed by
the CCM, and not be left lingering.
href added 5 commits October 25, 2023 16:12
Loadbalancers should usually be fully built after 120s, but it can
be a bit slower and that leaves too little time for additional actions
taken by the CCM.

Since it's not the job of the CCM integration tests to enforce speedy
loadbalancer creation, some slack can be added.
This ensures that the log does not include a warning/error that would
otherwise be invisible.
Otherwise a single load balancer creation process (which blocks the
worker for quite a while), prevents other services from being
updated.

Of course, there's still a limit, but the default of a single worker
is too low.
The error can still happen, as there is currently no way of knowing
if a delete has been applied already. To properly solve this, a
method to check if a delete has been successful is needed.
@href href merged commit 7da3d99 into main Nov 1, 2023
@href href deleted the denis/lbs branch January 5, 2024 13:17
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.

2 participants