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

Don't clear configuration when the fetch request fails #427

Merged
merged 1 commit into from
May 18, 2022

Conversation

thll
Copy link
Contributor

@thll thll commented May 3, 2022

When the configuration fetch request failed without an HTTP status code, the sidecar would treat this as a config change and clear its current configuration.

With this change, the failed request won't lead to the invalidation of the current configuration anymore.

Fixes #413

Reproducing this is a bit tricky because the behaviour can only be triggered when the request to /api/sidecar/collectors succeeds, but the subsequent request to /api/sidecar/configurations/render/* fails.
I used caddy as a proxy server between the sidecar and the Graylog server to selectively let the latter call fail. This is the Caddyfile I was using:

{
    debug
}

:2016 {

    # alternating every 10 seconds, this will match or not match the configuration fetch request
    @config_fetch {
        path /api/sidecar/configurations/render/*
        expression (int({time.now.unix}) % 20) < 10
    }

    handle @config_fetch {
        abort
    }

    # catch-all
    handle {
        reverse_proxy 127.0.0.1:9000
    }

}

Sidecar is run with the following two settings in sidecar.yml:

server_url: "http://127.0.0.1:2016/api/"
update_interval: 5

@thll thll requested a review from mpfz0r May 3, 2022 11:39
Copy link
Contributor

@mpfz0r mpfz0r left a comment

Choose a reason for hiding this comment

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

Good catch! LGTM 👍

@mpfz0r mpfz0r merged commit 83daefc into master May 18, 2022
@mpfz0r mpfz0r deleted the handle-config-fetch-error branch May 18, 2022 13:40
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.

Sidecar renders configuration if Server response is not successfull
2 participants