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

Populate toxics at startup from a json string #211

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

worldtiki
Copy link

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
Copy link
Contributor

jpittis commented Apr 16, 2018

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
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
Copy link
Contributor

jpittis commented Apr 19, 2018

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

@worldtiki
Copy link
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
Copy link
Contributor

jpittis commented Apr 20, 2018

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

@jpambrun
Copy link

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

@jpambrun
Copy link

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

@worldtiki
Copy link
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
Copy link

marqub commented Dec 18, 2018

@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
Copy link
Contributor

miry commented Nov 12, 2021

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
Copy link
Author

worldtiki commented Nov 12, 2021

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
Copy link
Contributor

miry commented Nov 12, 2021

@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
Copy link
Contributor

miry commented Nov 12, 2021

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

@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.)

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.

7 participants