Skip to content

Populate toxics at startup from a json string#211

Open
worldtiki wants to merge 3 commits into
Shopify:mainfrom
worldtiki:jsonconfig
Open

Populate toxics at startup from a json string#211
worldtiki wants to merge 3 commits into
Shopify:mainfrom
worldtiki:jsonconfig

Conversation

@worldtiki

Copy link
Copy Markdown

Ability to provide a configuration at startup.

My use case is the following:
I'm running a Kuberneters cluster with several applications/containers deployed. I would like to use Toxiproxy as a sidecar to some of these applications to be able to perform failure testing.

Currently we have two existing options to populate proxies, one from a json file and by making an http request. Neither option is very friendly when using Toxiproxy on containerized environments as they either require a post start script to run the http populate request or mounting a volume with the json config file.

With this change we can provide a json string (at startup) as such:
docker run -it shopify/toxiproxy:git -configJson '[{"name": "redis","listen": "0.0.0.0:9092","upstream": "redis:6379"}]'

@jpittis

jpittis commented Apr 16, 2018

Copy link
Copy Markdown
Contributor

Hey @worldtiki. Are you attempting to run Toxiproxy as a side car in a production Kubernetes cluster? If so, I would suggest extreme caution. Toxiproxy was not designed for production use and I have never heard of anyone running it in production environments.

Thanks for opening a PR. I don't have time to review this right now but will look at it later in the week.

@worldtiki

Copy link
Copy Markdown
Author

Hi @jpittis. Thanks for the feedback. This would be used in a staging/test environment, not in a production one.

@worldtiki worldtiki closed this Apr 18, 2018
@jpittis

jpittis commented Apr 19, 2018

Copy link
Copy Markdown
Contributor

Did you find an alternative solution @worldtiki? If you did, would you mind briefly mentioning what it was?

@worldtiki

Copy link
Copy Markdown
Author

Hi Jake!

Yes and no. I managed to use Kubernetes config maps feature to pass the config file to Toxiproxy.
Given that this would have been a breaking change (with the renaming of the config flags) and that there is (at least for kube) a workaround I decided to close the pr.

I did however got stuck again as the initial population seems to be populating proxies but ignoring the list of toxics. I don't know if this is expected behaviour or if I'm just doing something wrong (I never worked with Go before so I'm having a bit of a struggle to debug this)

@jpittis

jpittis commented Apr 20, 2018

Copy link
Copy Markdown
Contributor

The correct behaviour is that initial population ignores toxics. There's an open issue. #79

@jpambrun

Copy link
Copy Markdown

@worldtiki, Any chance to revive that? I like what you have done on your fork.

@jpambrun

Copy link
Copy Markdown

However, I just noticed that the ability to update toxics through the api seems to be broken on your branch.

@worldtiki

Copy link
Copy Markdown
Author

Yeah, sorry about that @jpambrun :(
I needed some extra features so I kinda butchered the project in that branch.

I'll reopen this pr (didn't know it could be of use for others). Regarding the initial population of toxics, it would be nice if someone else could give it a go, as my knowledge of go is very basic :s

@worldtiki worldtiki reopened this May 31, 2018
@marqub

marqub commented Dec 18, 2018

Copy link
Copy Markdown

@jpambrun @worldtiki
If you still want to use ToxiProxy with k8s, I come up with a solution that does not require code changes in ToxiProxy
Here a story on Medium that details the approach
http://bit.ly/2EAXRYk

Hope it helps!

@miry miry added this to the 2.2.0 milestone Oct 8, 2021
@miry miry removed this from the 2.2.0 milestone Oct 22, 2021
@miry miry self-assigned this Oct 22, 2021
@miry miry mentioned this pull request Oct 22, 2021
9 tasks
@miry miry modified the milestones: 2.2.1, 2.3.0 Oct 22, 2021
@miry miry added the Toxiproxy label Oct 22, 2021
@miry

miry commented Nov 12, 2021

Copy link
Copy Markdown
Contributor

I would replace ability to load config via ENV variable instead. Currently I am not fully convinced to change the arguments.

ENV is good, that it is easy to use in kubernetes and docker compose. It could be stored in .env file.

@worldtiki

worldtiki commented Nov 12, 2021

Copy link
Copy Markdown
Author

I would replace ability to load config via ENV variable instead. Currently I am not fully convinced to change the arguments.

ENV is good, that it is easy to use in kubernetes and docker compose. It could be stored in .env file.

Fair enough.

But just to confirm. This env var would be use to specify the name/path of a config file and not to explicitly pass the configs themselves right?

@miry

miry commented Nov 12, 2021

Copy link
Copy Markdown
Contributor

@worldtiki I tried to find other command lines that uses ENV as full config or for complex one. I failed to find a good examples (kubectl, opentelemtry-collector, grafana, nginx). Learned about best practices in https://12factor.net/ and https://clig.dev/ also suggested to have simple ENVs.

If you can suggest some example command line, it would be helpful.

Max that we can do assign SINGLE ENV to SINGLE Flag. Similar to TOML or what provides current frameworks Viper or https://github.com/urfave/cli/.

As workaround for docker we can use entrypoint.sh that would generate config file base on ENVs.

Example: https://devopsian.net/notes/docker-nginx-template-env-vars/ or use http://mustache.github.io/mustache.5.html

P.S: During learning about config structure. I feel I need extract parse config out of models. I am going to change config structure to remove arrays of proxies with dictionary. So it would be easy to create new entries. Also support more other formats.

@miry

miry commented Nov 12, 2021

Copy link
Copy Markdown
Contributor

Also thought to use flag --proxy and to specify attributes with "upastream=local:2,downstream=local,name=oops,enabled=true"

Also i think it is not unix way.

@miry miry modified the milestones: 2.3.0, 2.4.0 Dec 22, 2021
@miry miry mentioned this pull request Jan 5, 2022
8 tasks
@miry miry mentioned this pull request Mar 3, 2022
18 tasks
@miry miry modified the milestones: 2.4.0, 2.5.0 Mar 3, 2022
@miry miry mentioned this pull request Sep 4, 2022
13 tasks
@miry miry modified the milestones: 2.5.0, 2.6.0 Sep 7, 2022
@dianadevasia dianadevasia deleted the branch Shopify:main April 20, 2023 16:13
@jonatan-ivanov

Copy link
Copy Markdown

@dianadevasia By renaming master -> main I think you unintentionally closed lots of PRs including this one, see: https://github.com/Shopify/toxiproxy/pulls?q=is%3Apr+is%3Aclosed+sort%3Aupdated-desc

(I think using the rename feature of GitHub would have prevented this.)

@maxileith

Copy link
Copy Markdown

Are there any updates? I would really like to see a feature to define toxics at startup. I have pretty much the same use case. It is really hard to initialize the toxics at startup since there is not even sh in the container. So it is not even possible to create a custom entrypoint script to create toxics.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants