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

CLI #92

Merged
merged 12 commits into from
Jul 29, 2022
Merged

CLI #92

merged 12 commits into from
Jul 29, 2022

Conversation

alexsnaps
Copy link
Member

This is proposal for the CLI for limitador-server… Best is probably to play with it, but tl;dr:

  • storage is configured as a subcommand
  • args that only apply to a given storage are passed to that subcommand only
  • generic arg are applied at the "root" level.

I've also added a log line (with log level info or -vv) that prints the config as parsed asap:

$ limitador-server -vv limitador-server/examples/limits.yaml redis_cached redis://127.0.0.1:6379 --ttl 3 --ratio 4 --flush-period 10 --max-cached 2000

[2022-07-19T13:53:20Z INFO  limitador_server] Using config: Configuration { limits_file: "limitador-server/examples/limits.yaml", storage: Redis(RedisStorageConfiguration { url: "redis://127.0.0.1:6379", cache: Some(RedisStorageCacheConfiguration { flushing_period: 10, max_ttl: 3, ttl_ratio: 4, max_counters: 2000 }) }), rls_host: "0.0.0.0", rls_port: 8081, http_host: "0.0.0.0", http_port: 8080, limit_name_in_labels: false }

Base usage

$ limitador-server --help

Limitador Server 0.5.1
The Kuadrant team - github.com/Kuadrant
Rate Limiting Server

USAGE:
    limitador-server [OPTIONS] <LIMITS_FILE> [STORAGE]

ARGS:
    <LIMITS_FILE>    The limit file to use

OPTIONS:
    -b, --rls-ip <ip>              The IP to listen on for RLS [default: 0.0.0.0]
    -p, --rls-port <port>          The port to listen on for RLS [default: 8081]
    -B, --http-ip <http_ip>        The IP to listen on for HTTP [default: 0.0.0.0]
    -P, --http-port <http_port>    The port to listen on for HTTP [default: 8080]
    -l, --limit-name-in-labels     Include the Limit Name in prometheus label
    -v                             Sets the level of verbosity
    -E, --use-env-vars             Sets the server up from ENV VARS instead of these options
    -h, --help                     Print help information
    -V, --version                  Print version information

STORAGES:
    memory          Counters are held in Limitador (ephemeral) [default storage]
    redis           Uses Redis to store counters
    redis_cached    Uses Redis to store counters, with an in-memory cache
    infinispan      Uses Infinispan to store counters

Using in-memory counters:

Counters are held in Limitador (ephemeral) [default storage]

USAGE:
    limitador-server <LIMITS_FILE> memory

OPTIONS:
    -h, --help    Print help information

Using Redis

Bare

Uses Redis to store counters

USAGE:
    limitador-server <LIMITS_FILE> redis <URL>

ARGS:
    <URL>    Redis URL to use

OPTIONS:
    -h, --help    Print help information

Cached counters

Uses Redis to store counters, with an in-memory cache

USAGE:
    limitador-server <LIMITS_FILE> redis_cached [OPTIONS] <URL>

ARGS:
    <URL>    Redis URL to use

OPTIONS:
        --ttl <TTL>               TTL for cached counters in seconds [default: 5]
        --ratio <ratio>           Ratio to apply to the TTL from Redis on local cached counters
                                  [default: 10]
        --flush-period <flush>    Flushing period for counters in seconds [default: 1]
        --max-cached <max>        Maximum amount of counters cached [default: 10000]
    -h, --help                    Print help information

Infinispan

Uses Infinispan to store counters

USAGE:
    limitador-server <LIMITS_FILE> infinispan [OPTIONS] <URL>

ARGS:
    <URL>    Infinispan URL to use

OPTIONS:
    -n, --cache-name <cache name>      Name of the cache to store counters in [default: limitador]
    -c, --consistency <consistency>    The consistency to use to read from the cache [default:
                                       Strong] [possible values: Strong, Weak]
    -h, --help                         Print help information

@eguzki
Copy link
Contributor

eguzki commented Jul 25, 2022

good job 🎖️

Few notes:

  • --use-env-vars optional argument is confusing to me. Usually the order of precedence is: command line args, env vars and lastly defaults. IMO there is no need to add an optional arg to enable env vars. Env vars are always enabled.
  • I have developed a sandbox to test the available deployment options on top of the current config branch. sandbox #96. Maybe it is not related to this PR and it was broken before. Or maybe the testing env is wrong. The fact is that limitador's integration with infinispan is not working in the testing environment.

when make deploy-infinispan is run (check #96 for doc), limitador's container dies silently

limitador_1   | [2022-07-25T09:30:24Z INFO  limitador_server] Using config: Configuration { limits_file: "/opt/kuadrant/limits/limits.yaml", storage: Infinispan(InfinispanStorageConfiguration { url: "http://username:password@infinispan:11222", cache: Some("limitador-sandbox"), consistency: Some("Strong") }), rls_host: "0.0.0.0", rls_port: 8081, http_host: "0.0.0.0", http_port: 8080, limit_name_in_labels: false }
sandbox_limitador_1 exited with code 139

@alexsnaps alexsnaps requested a review from eguzki July 28, 2022 16:02
.unwrap_or_else(|_| (DEFAULT_MAX_TTL_CACHED_COUNTERS_SEC * 1000).to_string());
let redis_flushing_period_default = env::var("REDIS_LOCAL_CACHE_FLUSHING_PERIOD_MS")
.unwrap_or_else(|_| (DEFAULT_FLUSHING_PERIOD_SEC * 1000).to_string());
let redis_max_cached_counters_default = DEFAULT_MAX_CACHED_COUNTERS.to_string();
Copy link
Member Author

Choose a reason for hiding this comment

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

This wasn't configurable until now... I did not add it to the env vars tho... should we?

Copy link
Contributor

Choose a reason for hiding this comment

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

🤷

create issue and somebody can implement

})
}
Some(("memory", _sub)) => StorageConfiguration::InMemory,
None => match storage_config_from_env() {
Copy link
Member Author

Choose a reason for hiding this comment

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

If no STORAGE has been provided, there also can't be any storage related config being passed in thru the command line, so we can safely delegate to loading all further config from the env vars or their defaults.

.unwrap_or_else(|_| (DEFAULT_MAX_TTL_CACHED_COUNTERS_SEC * 1000).to_string());
let redis_flushing_period_default = env::var("REDIS_LOCAL_CACHE_FLUSHING_PERIOD_MS")
.unwrap_or_else(|_| (DEFAULT_FLUSHING_PERIOD_SEC * 1000).to_string());
let redis_max_cached_counters_default = DEFAULT_MAX_CACHED_COUNTERS.to_string();
Copy link
Contributor

Choose a reason for hiding this comment

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

🤷

create issue and somebody can implement

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