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

Api server is not disabled when loading a configuration that has api.enabled=false #13508

Closed
KH-Moogsoft opened this issue Jul 11, 2022 · 0 comments · Fixed by #17958
Closed
Labels
domain: reload Anything related to reloading Vector (updating configuration) type: bug A code related bug.

Comments

@KH-Moogsoft
Copy link
Contributor

KH-Moogsoft commented Jul 11, 2022

A note for the community

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment

Problem

Current behavior: When running vector with a http provider, loading a remote configuration with the api.enabled = true means that the api server can never be disabled by loading a new configuration with api.enabled = false. The reverse is also true, loading a remote configuration with the api disabled means that the api cannot be started by subsequent configurations.

Expected behavior: I would expect that the api server would start/stop depending on whether or not the loaded config has it enabled.

Based on the following code in app.rs, it appears that the api_server variable is initialized once before the main loop, and is only updated with config changes from that point on.

            // Assigned to prevent the API terminating when falling out of scope.
            let api_server = if api_config.enabled {
                use std::sync::{Arc, atomic::AtomicBool};
                emit!(ApiStarted {
                    addr: api_config.address.unwrap(),
                    playground: api_config.playground
                });

                Some(api::Server::start(topology.config(), topology.watch(), Arc::<AtomicBool>::clone(&topology.running)))
            } else {
                info!(message="API is disabled, enable by setting `api.enabled` to `true` and use commands like `vector top`.");
                None
            };

//Later on in the file
    #[cfg(feature = "api")]
         // Pass the new config to the API server.
       if let Some(ref api_server) = api_server {
          api_server.update_config(topology.config());
        }

In order to actually shut down the api server, I think that it would have to be dropped and the value of api_server would be reassigned, like so:

#[cfg(feature = "api")]
if new_config.api.enabled {
    if let None = api_server {
                    use std::sync::{Arc, atomic::AtomicBool};
        api_server = Some(api::Server::start(topology.config(), topology.watch(), Arc::<AtomicBool>::clone(&topology.running)));
    }
} else {
    if let Some(server) = api_server {
        drop(server)
    }
    api_server = None;
}

Configuration

[provider]
type = "http"
url = "http://127.0.0.1:8080/config"

To test this, I made a minimal web server that alternated between two different configurations, both identical except for api.enabled being true or false:

api.enabled = true
[sources.demo_logs]
type = "demo_logs"
format = "syslog"
interval = 1

[sinks.blackhole_sink]
type = "blackhole"
inputs = [ "demo_logs" ]
print_interval_secs = 0"

Version

0.23.0

Debug Output

The debug output isn't helpful in this case, since the server simply stays on or off.

Example Data

No response

Additional Context

No response

References

No response

@KH-Moogsoft KH-Moogsoft added the type: bug A code related bug. label Jul 11, 2022
@jszwedko jszwedko added the domain: reload Anything related to reloading Vector (updating configuration) label Jul 11, 2022
github-merge-queue bot pushed a commit that referenced this issue Jul 19, 2023
This PR attempts to resolve:
#13508

I'm not married to switching out the `Runtime` parameters for `Handle`s,
it just seemed like the easiest way to get something that could spawn
tasks into the `TopologyController` was `Handle::current()`, and that
required shifting the parameter types to match. Let me know if another
route would be preferred.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
domain: reload Anything related to reloading Vector (updating configuration) type: bug A code related bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants