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.

Sign up for GitHub

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

Jump to bottom

Skip e2e test for Mac. #43671

Merged
merged 1 commit into from
Apr 5, 2017
Merged

Skip e2e test for Mac. #43671

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
20 changes: 14 additions & 6 deletions hack/lib/golang.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,11 +158,20 @@ readonly KUBE_TEST_PORTABLE=(
# need to target server platforms.
# These binaries will be distributed in the kubernetes-test tarball.
# If you update this list, please also update build/release-tars/BUILD.
readonly KUBE_TEST_SERVER_TARGETS=(
cmd/kubemark
vendor/github.com/onsi/ginkgo/ginkgo
test/e2e_node/e2e_node.test
)
kube::golang::server_test_targets() {
local targets=(
cmd/kubemark
vendor/github.com/onsi/ginkgo/ginkgo
)

if [[ "${OSTYPE:-}" == "linux"* ]]; then
Copy link
Member

@ixdy ixdy Mar 27, 2017 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is checking the wrong thing - $OSTYPE is referring to the host OS, but we should still support cross-compiling linux targets from macOS, which this would disable.

we probably need to change the logic so that we only build the server test targets when $GOOS is linux. Or fix test/e2e/e2e_node so that it builds on mac (even though it's kinda pointless to do so).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For test/e2e/e2e_node, I'd like to skip it for mac as we had several PRs for build error :(. And for the cross-compling, no $OSTYPE here?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

using $OSTYPE instead of the target platform means that a macOS user can't cross-compile e2e_node.test for linux.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ixdy , what's the command to cross-compile e2e_node.test from MacOS for Linux? I'd like to debug script to fix it :).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

make quick-release should do it. You'll need a working docker setup, though.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ixdy , the OSTYPE works when make quick-release.

Here's the diff to show OSTYPE when building

-        kube::log::status "Building go targets for ${platform}:" "${targets[@]}"
+        kube::log::status "Building go targets for ${platform}/${OSTYPE}:" "${targets[@]}"

And here's the output of make quick-release:

MacPro:kubernetes klaus$ make quick-release
+++ [0402 08:34:10] Verifying Prerequisites....
+++ [0402 08:34:10] Using Docker for MacOS
+++ [0402 08:34:10] Building Docker image kube-build:build-6433d374d6-5-v1.7.5-2
+++ [0402 08:34:16] Syncing sources to container
+++ [0402 08:34:18] Running build command...
+++ [0402 08:34:25] Building the toolchain targets:
    k8s.io/kubernetes/hack/cmd/teststale
    k8s.io/kubernetes/vendor/github.com/jteeuwen/go-bindata/go-bindata
+++ [0402 08:34:25] Generating bindata:
    test/e2e/generated/gobindata_util.go
~ ~/test/e2e/generated
~/test/e2e/generated
+++ [0402 08:34:26] Building go targets for linux/amd64 (linux-gnu):
    cmd/kube-proxy
    cmd/kube-apiserver
    cmd/kube-controller-manager
    cmd/cloud-controller-manager
    cmd/kubelet
    cmd/kubeadm
    cmd/hyperkube
    vendor/k8s.io/kube-aggregator
    plugin/cmd/kube-scheduler

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hm, interesting, I suspect this still only works by accident - the $OSTYPE in your docker container is probably set to linux.

This previously wasn't an issue because we only cross-build KUBE_TEST_SERVER_TARGETS for linux platforms.
I think you can only run into the compilation failure if you try to build everything (i.e. just run make) on macOS natively, without docker or anything else.

If you tried cross-building on macOS natively I think this will do the wrong thing. But I'm not sure this is a use case we need to worry about.

Further thoughts: we should perhaps not include KUBE_SERVER_TARGETS or KUBE_TEST_SERVER_TARGETS in KUBE_ALL_TARGETS when building on non-linux, since neither of those sets is expected to be built for non-linux.

targets+=( test/e2e_node/e2e_node.test )
fi

echo "${targets[@]}"
}

readonly KUBE_TEST_SERVER_TARGETS=($(kube::golang::server_test_targets))
readonly KUBE_TEST_SERVER_BINARIES=("${KUBE_TEST_SERVER_TARGETS[@]##*/}")
readonly KUBE_TEST_SERVER_PLATFORMS=("${KUBE_SERVER_PLATFORMS[@]}")

Expand Down Expand Up @@ -487,7 +496,6 @@ kube::golang::build_binaries_for_platform() {
V=2 kube::log::info "Env for ${platform}: GOOS=${GOOS-} GOARCH=${GOARCH-} GOROOT=${GOROOT-} CGO_ENABLED=${CGO_ENABLED-} CC=${CC-} GO=${GO}"

for binary in "${binaries[@]}"; do

if [[ "${binary}" =~ ".test"$ ]]; then
tests+=($binary)
elif kube::golang::is_statically_linked_library "${binary}"; then
Expand Down