-
Notifications
You must be signed in to change notification settings - Fork 349
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
Use testcontainers to run redis-server
#1960
Conversation
If it works we can also try to use it for etcd skipper/etcd/etcdtest/mocketcd.go Lines 45 to 47 in 100ef1b
|
e147bf9
to
4417c1d
Compare
Mitigates build failures caused by package manager redirects during `redis-server` installation. Note that change adds many dependencies but they are only used for tests and do not end up in the build. Signed-off-by: Alexander Yastrebov <alexander.yastrebov@zalando.de>
4417c1d
to
aaaff64
Compare
ContainerRequest: testcontainers.ContainerRequest{ | ||
Image: "redis:6-alpine", | ||
Cmd: args, | ||
ExposedPorts: []string{"6379/tcp"}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does a test container gets it's own IP?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It gets a mapped port on localhost that we obtain using .Endpoint below
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sounds like we could test sharding with this too, but we need to change the port for the next ones :)
Do you also want to add etcd? |
I thought about doing it in separate PR, this one should fix build failures |
👍 |
@@ -8,8 +8,6 @@ pipeline: | |||
commands: | |||
- desc: build-push | |||
cmd: | | |||
apt-get update -o Acquire::http::AllowRedirect=false | |||
apt-get install -o Acquire::http::AllowRedirect=false -y bzr redis-server jq |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see bzr
used anywhere but I think this would break a Makefile that depends on jq
👍 |
`jq` installation was removed by #1960 but it is required by `packaging/Makefile` Signed-off-by: Alexander Yastrebov <alexander.yastrebov@zalando.de>
`jq` installation was removed by #1960 but it is required by `packaging/Makefile` Signed-off-by: Alexander Yastrebov <alexander.yastrebov@zalando.de>
Mitigates build failures caused by package manager redirects during
redis-server
installation.Note that change adds many dependencies but they are only used for tests and do not end up in the build.
Signed-off-by: Alexander Yastrebov alexander.yastrebov@zalando.de