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

SurgicalPool extension by the load balancing algorithm (the same way as the RoutePool) #457

Closed

Conversation

Dariquest
Copy link
Contributor

@Dariquest Dariquest commented Dec 12, 2024

Summary

Issue #458

LoadBalancing Algorithm was introduced for the RoutePool in the PR.
The load balancing algorithm is also needed to be passed to the SurgicalPool (included in the RouteOpts for the initialization).
Missing load balancing algorithm in the surgical pool leads to the error "invalid-pool-load-balancing-algorithm".
This error is intended validation for a pool load balancing algorithm. An additional log parameter was added for this error either.

Reproduction Steps

When I curl an app with an x-cf-app-instance header

curl -v "https://example-app.com/" -H "x-cf-app-instance: app_id:instance_id"

The same curl without the instance header works properly.

Backward Compatibility


Breaking Change? No

@Dariquest Dariquest requested a review from a team as a code owner December 12, 2024 14:51
@Dariquest Dariquest changed the title Extend SurgicalPool by the load balancing algorithm (the same way as a RoutePool) SurgicalPool extension by the load balancing algorithm (the same way as the RoutePool) Dec 12, 2024
@Dariquest Dariquest marked this pull request as draft December 12, 2024 14:54
@Dariquest
Copy link
Contributor Author

Dariquest commented Dec 13, 2024

Adding reference to an issue #458

route/pool.go Outdated Show resolved Hide resolved
@Dariquest Dariquest requested a review from peanball December 16, 2024 14:40
@Dariquest Dariquest marked this pull request as ready for review December 16, 2024 15:17
route/pool.go Outdated Show resolved Hide resolved
Co-authored-by: Alexander Lais <[email protected]>
route/pool.go Outdated Show resolved Hide resolved
Dariquest and others added 13 commits December 17, 2024 09:10
Co-authored-by: Alexander Lais <[email protected]>
* do not log an empty new line when writing to syslog
* allow operator to configure truncation on syslog
The go standard library implementation of log/syslog has been abandoned
and is no longer maintained. In our tests with syslog logging in
gorouter we noticed a significant performance penalty over file based
logging. However, with file based logging we frequently observe broken
logs because logrotate interferes with log reading / writing.

This commit inlines the relevant file from log/syslog and refactors the
implementation to be on-par with the file based logging.

Co-authored-by: Alexander Lais <[email protected]>
* feat: make syslog logging useable

* do not log an empty new line when writing to syslog
* allow operator to configure truncation on syslog

* fix: allow syslog-only logging

* feat: replace log/syslog with internal version

The go standard library implementation of log/syslog has been abandoned
and is no longer maintained. In our tests with syslog logging in
gorouter we noticed a significant performance penalty over file based
logging. However, with file based logging we frequently observe broken
logs because logrotate interferes with log reading / writing.

This commit inlines the relevant file from log/syslog and refactors the
implementation to be on-par with the file based logging.

Co-authored-by: Alexander Lais <[email protected]>

* fix: rename testState.newRequest -> testState.newGetRequest

* test: add integration test for syslog access logs

* test: check errors

---------

Co-authored-by: Alexander Lais <[email protected]>
@Dariquest
Copy link
Contributor Author

Replacing this PR by #460

@Dariquest Dariquest closed this Dec 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

4 participants