only set CGO_ENABLED=0 for tests #12535
Conversation
|
@smarterclayton @stevekuznetsov @liggitt @marun as you were in the conversation on #11044 |
|
defer to @smarterclayton and @stevekuznetsov |
|
Looks fine to me [test] |
|
flake #12558 |
|
Evaluated for origin testextended up to 50b0236 |
|
continuous-integration/openshift-jenkins/testextended FAILURE (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin_extended/1002/) (Base Commit: d981aa6) (Extended Tests: all) |
|
[test] |
|
@smarterclayton please merge if happy |
hack/build-cross.sh
Outdated
| @@ -9,7 +9,9 @@ host_platform="$(os::build::host_platform)" | |||
| # Set build tags for these binaries | |||
| readonly OS_GOFLAGS_TAGS="include_gcs include_oss" | |||
| readonly OS_GOFLAGS_TAGS_LINUX_PPC64LE="gssapi" | |||
| readonly OS_GOFLAGS_TAGS_TEST_LINUX_PPC64LE="" | |||
There was a problem hiding this comment.
You shouldn't have to do this - empty strings = no-op
|
Evaluated for origin test up to 4a9fef7 |
|
LGTM [merge] |
|
Evaluated for origin merge up to 4a9fef7 |
|
continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/13480/) (Base Commit: 92ce2c9) (Image: devenv-rhel7_5832) |
| @@ -213,6 +211,7 @@ os::build::internal::build_binaries() { | |||
| fi | |||
|
|
|||
| local platform_gotags_envvar=OS_GOFLAGS_TAGS_$(echo ${platform} | tr '[:lower:]/' '[:upper:]_') | |||
| local platform_gotags_test_envvar=OS_GOFLAGS_TAGS_TEST_$(echo ${platform} | tr '[:lower:]/' '[:upper:]_') | |||
There was a problem hiding this comment.
Why was this change necessary? It looks like it's going to break anyone who was using OS_GOFLAGS_TAGS_<platform> to pass to test builds
edit: s/OS_GOFLAGS_TAGS/OS_GOFLAGS_TAGS_<platform>/
There was a problem hiding this comment.
I don't believe it does break that. The change stops automatically setting the gssapi tag, which is incompatible with CGO_ENABLED=0.
There was a problem hiding this comment.
If I were passing the platform-specific OS_GOFLAGS_TAGS_LINUX today, that would get used in the build of tests. You've changed that so I need to pass OS_GOFLAGS_TAGS_TEST_LINUX for that. This is a breaking change. I'm not sure we have a good way to identify if someone downstream will be broken by such a change.
There was a problem hiding this comment.
@liggitt is the main potential impact with gssapi
In general test tags should be explicit - if we need gssapi in integration, we need to call that out (which means it would fail on Macs, which it doesn't).
There was a problem hiding this comment.
gssapi is only for the kubectl build. We have an extended test that will break shortly if this broke something
|
continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/13477/) (Base Commit: 92ce2c9) |
#11044 accidentally caused the openshift binary to be built with CGO_ENABLED=0 if it had not been built by the time test/extended/core.sh was run. The openshift binary cannot run correctly with CGO_ENABLED=0 as cgo is used by kubelet.
This change centralises the setting of CGO_ENABLED to hack/common.sh for all .test executables.
I believe that this setting is wanted so that delve can be used against these executables.