-
Notifications
You must be signed in to change notification settings - Fork 286
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
Make race detector optional. Enable by default. #2386
Conversation
image-test: image fv/Dockerfile.test.amd64 bin/pktgen bin/test-workload bin/test-connection bin/calico-felix-race-amd64 image-wgtool | ||
docker build -t $(BUILD_IMAGE)-test:latest-$(ARCH) --build-arg QEMU_IMAGE=$(CALICO_BUILD) --file ./fv/Dockerfile.test.$(ARCH) bin; | ||
ifeq ($(FV_RACE_DETECTOR_ENABLED),true) | ||
FV_BINARY=calico-felix-race-amd64 |
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.
This seem to be forcing ARCH=amd64
but the docker build
command below consider $(ARCH).
Is this expected behavior?
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.
@fasaxc, can you follow up with this later please? Since this isn't a regression we're merging this through so that I can get unblocked.
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.
Yeah, FVs are amd64 only (previously that was just hard-coded in the dockerfile).
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.
Unblocking @Brian-McM
Description
Add a new build option to enable/disable the race detector:
Enabled by default.
Todos
Release Note