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.

Sign up for GitHub

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

Jump to bottom

feat(load-balancer): ignore nodes that don't use known provider IDs #780

Merged
merged 5 commits into from
Nov 20, 2024
Merged

feat(load-balancer): ignore nodes that don't use known provider IDs #780

merged 5 commits into from
Nov 20, 2024

Conversation

lukasmetzner
Copy link
Contributor

With this feature the load balancer is more resilient to hybrid clusters as it skips nodes with unkown provider ids.

@lukasmetzner lukasmetzner linked an issue Nov 5, 2024 that may be closed by this pull request
feat(load-balancer): ignore nodes that don't use known provider IDs #530
Closed
@codecov Codecov
Copy link

codecov bot commented Nov 5, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 3.70370% with 26 lines in your changes missing coverage. Please review.

Project coverage is 69.93%. Comparing base (674696f) to head (b79c64f).

Files with missing lines Patch % Lines
internal/hcops/load_balancer.go 0.00% 11 Missing ⚠️
internal/providerid/providerid.go 11.11% 8 Missing ⚠️
hcloud/instances.go 0.00% 7 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main     #780      +/-   ##
==========================================
- Coverage   70.45%   69.93%   -0.53%     
==========================================
  Files          31       31              
  Lines        3226     3250      +24     
==========================================
  Hits         2273     2273              
- Misses        779      803      +24     
  Partials      174      174

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

🚨 Try these New Features:

@lukasmetzner lukasmetzner self-assigned this Nov 19, 2024
@lukasmetzner lukasmetzner marked this pull request as ready for review November 19, 2024 14:14
@lukasmetzner lukasmetzner requested a review from a team as a code owner November 19, 2024 14:14
@lukasmetzner lukasmetzner force-pushed the 530-featload-balancer-ignore-nodes-that-dont-use-known-provider-ids branch from 6c17332 to b79c64f Compare November 19, 2024 14:16
lukasmetzner and others added 4 commits November 20, 2024 10:03
@lukasmetzner
feat(load-balancer): ignore nodes that don't use known provider IDs
7ec64de 
With this feature the load balancer is more resilient to hybrid clusters as it skips nodes with unkown provider ids.
@lukasmetzner
test: extended tests to check for custom types
2a44df8 
Updated table-driven tests to validate the new error type specifically, instead of merely checking for the presence of an error.
@lukasmetzner @jooola
Update internal/providerid/providerid.go
3989e62 
Co-authored-by: Jonas L. <[email protected]>
@lukasmetzner
fix: did not properly check error type
67c46a8
@lukasmetzner lukasmetzner force-pushed the 530-featload-balancer-ignore-nodes-that-dont-use-known-provider-ids branch from 54e55c5 to a76140c Compare November 20, 2024 09:03
@lukasmetzner lukasmetzner force-pushed the 530-featload-balancer-ignore-nodes-that-dont-use-known-provider-ids branch from a76140c to b8681d8 Compare November 20, 2024 09:07
@lukasmetzner lukasmetzner force-pushed the 530-featload-balancer-ignore-nodes-that-dont-use-known-provider-ids branch from b8681d8 to 1364d19 Compare November 20, 2024 09:08
@lukasmetzner lukasmetzner merged commit 6544740 into main Nov 20, 2024
10 checks passed
@lukasmetzner lukasmetzner deleted the 530-featload-balancer-ignore-nodes-that-dont-use-known-provider-ids branch November 20, 2024 11:39
This was referenced Nov 19, 2024
Closed
Merged
lukasmetzner pushed a commit that referenced this pull request Nov 26, 2024
@hcloud-bot
chore(main): release v1.21.0 (#806)
2316769 
<!-- section-start changelog -->
### Feature Highlights &amp; Upgrade Notes

#### Load Balancer IPs set to Private IPs

If networking support is enabled, the load balancer IPs are now
populated with the private IPs, unless the
`load-balancer.hetzner.cloud/disable-private-ingress` annotation is set
to `true`. Please make sure that you configured the annotation according
to your needs, for example if you are using `external-dns`.

#### Provided-By Label

We introduced a the label `instance.hetzner.cloud/provided-by`, which
will be automatically added to all **new** nodes. This label can have
the values `cloud` or `robot` to distinguish between our products. We
use this label in the csi-driver to ensure the daemonset is only running
on cloud nodes. We recommend to add this label to your existing nodes
with the appropriate value.

- `kubectl label node $CLOUD_NODE_NAME
instance.hetzner.cloud/provided-by=cloud`
- `kubectl label node $ROBOT_NODE_NAME
instance.hetzner.cloud/provided-by=robot`

#### Load Balancer IPMode Proxy

Kubernetes KEP-1860 added a new field to the Load Balancer Service
Status that allows us to mark if the IP address we add should be
considered as a Proxy (always send traffic here) and VIP (allow
optimization by keeping the traffic in the cluster).

Previously Kubernetes considered all IPs as VIP, which caused issues
when when the PROXY protocol was in use. We have previously recommended
to use the annotation `load-balancer.hetzner.cloud/hostname` to
workaround this problem.

We now set the new field to `Proxy` if the PROXY protocol is active so
the issue should no longer appear. If you only added the
`load-balancer.hetzner.cloud/hostname` annotation for this problem, you
can remove it after upgrading.

Further information:

- kubernetes/enhancements#1860
-
#160 (comment)

### Features

- **service**: Specify private ip for loadbalancer (#724)
- add support &amp; tests for Kubernetes 1.31 (#747)
- **helm**: allow setting extra pod volumes via chart values  (#744)
- **instance**: add label to distinguish servers from Cloud and Robot
(#764)
- emit event when robot server name and node name mismatch (#773)
- **load-balancer**: Set IPMode to &#34;Proxy&#34; if load balancer is
configured to use proxy protocol (#727) (#783)
- **routes**: emit warning if cluster cidr is misconfigured (#793)
- **load-balancer**: ignore nodes that don&#39;t use known provider IDs
(#780)
- drop tests for kubernetes v1.27 and v1.28

### Bug Fixes

- populate ingress private ip when disable-private-ingress is false
(#715)
- wrong version logged on startup (#729)
- invalid characters in label instance-type of robot servers (#770)
- no events are emitted as broadcaster has no sink configured (#774)

### Kubernetes Support

This version was tested with Kubernetes 1.29 - 1.31. Furthermore, we
dropped v1.27 and v1.28 support.

<!-- section-end changelog -->

---

<details>
<summary><h4>PR by <a
href="https://github.com/apricote/releaser-pleaser">releaser-pleaser</a>
🤖</h4></summary>

If you want to modify the proposed release, add you overrides here. You
can learn more about the options in the docs.

## Release Notes

### Prefix / Start

This will be added to the start of the release notes.

```rp-prefix
### Feature Highlights & Upgrade Notes

#### Load Balancer IPs set to Private IPs

If networking support is enabled, the load balancer IPs are now populated with the private IPs, unless the `load-balancer.hetzner.cloud/disable-private-ingress` annotation is set to `true`. Please make sure that you configured the annotation according to your needs, for example if you are using `external-dns`.

#### Provided-By Label

We introduced a the label `instance.hetzner.cloud/provided-by`, which will be automatically added to all **new** nodes. This label can have the values `cloud` or `robot` to distinguish between our products. We use this label in the csi-driver to ensure the daemonset is only running on cloud nodes. We recommend to add this label to your existing nodes with the appropriate value.

- `kubectl label node $CLOUD_NODE_NAME instance.hetzner.cloud/provided-by=cloud`
- `kubectl label node $ROBOT_NODE_NAME instance.hetzner.cloud/provided-by=robot`

#### Load Balancer IPMode Proxy

Kubernetes KEP-1860 added a new field to the Load Balancer Service Status that allows us to mark if the IP address we add should be considered as a Proxy (always send traffic here) and VIP (allow optimization by keeping the traffic in the cluster).

Previously Kubernetes considered all IPs as VIP, which caused issues when when the PROXY protocol was in use. We have previously recommended to use the annotation `load-balancer.hetzner.cloud/hostname` to workaround this problem.

We now set the new field to `Proxy` if the PROXY protocol is active so the issue should no longer appear. If you  only added the `load-balancer.hetzner.cloud/hostname` annotation for this problem, you can remove it after upgrading.

Further information:
- kubernetes/enhancements#1860
- #160 (comment)
```

### Suffix / End

This will be added to the end of the release notes.

```rp-suffix
### Kubernetes Support

This version was tested with Kubernetes 1.29 - 1.31. Furthermore, we dropped v1.27 and v1.28 support.
```

</details>

Co-authored-by: releaser-pleaser <>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jooola jooola jooola left review comments

@apricote apricote apricote approved these changes

Assignees

@lukasmetzner lukasmetzner

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

feat(load-balancer): ignore nodes that don't use known provider IDs
3 participants
@lukasmetzner @apricote @jooola