Made new config for e2e tests having a non alpine image with cgo enabled #5066
Conversation
tend2infinity
changed the title
There was a problem hiding this comment.
This looking nicely 👏 I added few comments + just please make sure you signed your commits (see section 'Signing your work' in https://github.com/thanos-io/thanos/blob/main/CONTRIBUTING.md#pull-request-process), so that we can kick start the CI as well!
Signed-off-by: soumya <somu12.ss@gmail.com>
Signed-off-by: soumya <somu12.ss@gmail.com>
Signed-off-by: soumya <somu12.ss@gmail.com>
Signed-off-by: soumya <somu12.ss@gmail.com>
|
@matej-g made the required changes |
There was a problem hiding this comment.
Looking good, thanks @tend2infinity! Let's ping maintainers to make CI run for this PR.
Dockerfile.e2e-tests
Outdated
| @@ -0,0 +1,20 @@ | |||
| # By default we pin to amd64 sha. Use make docker to automatically adjust for arm64 versions. | |||
There was a problem hiding this comment.
We need to update this comment, i think:
| # By default we pin to amd64 sha. Use make docker to automatically adjust for arm64 versions. | |
| # By default we pin to amd64 sha. Use make docker-e2e to automatically adjust for arm64 versions. |
There was a problem hiding this comment.
Gotcha! will update this
.promu.e2e.yml
Outdated
| crossbuild: | ||
| platforms: | ||
| - linux/amd64 | ||
| - darwin/amd64 |
There was a problem hiding this comment.
Why do we need anything other than Linux for the e2e tests? All of the tests run inside of a container, which means that the will run on Linux, and thus need a Linux binary, right?
There was a problem hiding this comment.
Okay, makes sense , we can remove the other platforms for sure
Makefile
Outdated
| @echo ">> building Thanos binary in $(PREFIX)" | ||
| @$(PROMU) --config=".promu.e2e.yml" build --prefix $(PREFIX) | ||
|
|
||
|
|
Signed-off-by: soumya <somu12.ss@gmail.com>
|
Hey @matej-g can you review it once ;) |
Signed-off-by: soumya <somu12.ss@gmail.com>
|
Changes done, works alright! |
|
Thanks @tend2infinity and sorry for the delayed answer! Looks like everything is working fine, we are actually already catching some data races. Did you hit these during local runs? I think we should fix them before we merge this (otherwise E2E tests will be broken). What I found based on test logs on my local machine:
|
|
Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward? This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. |
|
Still relevant but not finished, on my to-do list... |
|
Hey @matej-g sorry for the delay, I will look into these issues that you've mentioned and will let you know the progress |
|
Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward? This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. |
Changes
Verification
Part of
Issue #4664