Thanks for your pull request.

It looks like this may be your first contribution to a Google open source project, in which case you'll need to sign a Contributor License Agreement (CLA) at https://cla.developers.google.com/.

If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check the information on your CLA or see this help article on setting the email on your git commits.

Once you've done that, please reply here to let us know. If you signed the CLA as a corporation, please let us know the company's name.

Required for #1473 and #5475

I'm also a little worried about defaulting to running the unit tests twice - this will make make test twice as slow, and I don't think Travis will be able to keep up with running all unit tests in coverage mode twice.

Updated the code as per comments.
Ready for another round of review.
Thanks!

Have updated the code so that test-go.sh and test-integration.sh now use KUBE_TEST_API_VERSIONS env variable to figure out the API Versions which should be tested.
It is a comma separated list of api versions, with default value of "v1beta1,v1beta3".
Updated .travis.yaml to set KUBE_TEST_API_VERSIONS to "v1beta1" when running the tests for go version 1.3 and set KUBE_TEST_API_VERSIONS to "v1beta3" for go version 1.4.

Hence, when run locally, both versions are tested. And when run on travis, v1beta1 is tested with go version 1.3 and v1beta3 is tested with go version 1.4

cc @bgrant0607

If it looks good, it can be merged right after #5887

cc @smarterclayton

IANA-Bash-Wizard - the travis changes look ok to me. If we're confident in the bash?

I have tested the scripts locally and on travis and they work fine.

Replied to comments and have updated the code accordingly.
PTAL. Thanks!

LGTM, but are the Shippable and Travis failures related to this PR?

TestSyncPodsDeletesWithNoPodInfraContainer seems flaky. I have started the tests again to confirm. Will look in more detail if it still fails.

That test is flaky. I believe there is an issue open.

----- Original Message -----

TestSyncPodsDeletesWithNoPodInfraContainer seems flaky. I have started the
tests again to confirm. Will look in more detail if it still fails.

---

Reply to this email directly or view it on GitHub:
#5687 (comment)

Yes. It passed this time.

My failures are happening in a dev env without that env set.

On Mar 25, 2015, at 5:46 PM, Nikhil Jindal notifications@github.com wrote:

This is the travis run which seems to have picked up the correct change https://travis-ci.org/GoogleCloudPlatform/kubernetes/builds/55866307

—
Reply to this email directly or view it on GitHub.

This line of output is very suspicious:
Running tests for APIVersion: v1beta1 v1beta3

It suggests that somehow this for loop isn't doing the right thing:

# Convert the CSV to an array of API versions to test                             
IFS=',' read -a apiVersions <<< ${KUBE_TEST_API_VERSIONS}                         
for apiVersion in "${apiVersions[@]}"; do                                         
  echo "Running tests for APIVersion: $apiVersion"                                
  runTestsForVersion $apiVersion "${@}"                                           
done

Is the version of bash on OS X wacky or something? Which version of OS X are you using, @smarterclayton?

As another data point, @smarterclayton what happens when you run
KUBE_TEST_API_VERSIONS=v1beta3 hack/test-go.sh?

Yosimite

On Mar 25, 2015, at 5:59 PM, Jeff Grafton notifications@github.com wrote:

This line of output is very suspicious:
Running tests for APIVersion: v1beta1 v1beta3

It suggests that somehow this for loop isn't doing the right thing:

Convert the CSV to an array of API versions to test

IFS=',' read -a apiVersions <<< ${KUBE_TEST_API_VERSIONS}
for apiVersion in "${apiVersions[@]}"; do
echo "Running tests for APIVersion: $apiVersion"
runTestsForVersion $apiVersion "${@}"
done
Is the version of bash on OS X wacky or something? Which version of OS X are you using, @smarterclayton?

—
Reply to this email directly or view it on GitHub.

@smarterclayton can you run hack/test-go.sh overriding KUBE_TEST_API_VERSIONS as I suggested and let me know if that works?

I'm pretty sure this broke the build?

This hit shippable as well. E.g. https://app.shippable.com/builds/551345dd9461c40b00437a04

There is a new shippable.yaml submitted just now #5753 which might be causing this.
This PR was merged 3 hours ago!

Actually that PR was also merged an hour ago. Taking a look.

The shippable.yml PR just mirrored the changes in this PR.

Also, it passed: https://app.shippable.com/builds/55133fed9461c40b004379a8

That run (https://app.shippable.com/builds/55133fed9461c40b004379a8) seems like an old one since the tests were run only for "v1beta1" and not for both.
Daniel tells me that his tests pass on travis and locally. They fail only on shippable.

@nikhiljindal that's what the configuration specifies - go1.4 runs only v1beta1; the other build, which ran go1.3, runs only v1beta3, just like the Travis builds.

The failing runs (as well as @smarterclayton's local run) all appear to not be specifying KUBE_TEST_API_VERSIONS in the environment, so it uses the default value testing both API versions.

For whatever reason this is messing up and the script tries to run both API versions at once, rather than splitting them up as it's supposed to.

@filbranden as an expert shell expert who has reviewed changes here before, do you have any ideas as to why this might be failing in some environments?

I rebased and re-pushed, and now it works. So presumably it was the change to shippable.yml that fixed it.

ok. I can revert this PR if its causing too much pain.
I cant understand why it works sometimes and sometimes it doesnt. (I just re-ran tests on failed PRs and they passed)
Maybe its just that some instances havent caught up with the new config file yet and things work fine if they have the new version.
Open to ideas or suggestions.

I would say-- don't revert unless rebasing doesn't fix this for people.

Setting KUBE_TEST_API_VERSIONS=v1beta1 works fine for me, just not leaving it unset.

Can we only do this from things like build/release.sh and travis and not
during normal development? Our dev process is grinding to a halt.

And dont get me started about all the slow hooks - a rebase can take 5
seconds per commit to run gendocs and all the other crap...

On Wed, Mar 25, 2015 at 5:41 PM, Clayton Coleman notifications@github.com
wrote:

Setting KUBE_TEST_API_VERSIONS=v1beta1 works fine for me, just not leaving
it unset.

—
Reply to this email directly or view it on GitHub
#5687 (comment)
.

FYI, for people having trouble on their local machines, #5974 should help.

@ixdy There's just so much wrong with this PR...

Don't use IFS. Ever.

Use arrays for multiple values.

It's setting KUBE_TEST_API_VERSIONS to something unconditionally even if it wasn't really being set before.

runTestsForVersion is exporting KUBE_API_VERSION and leaving it exported after the function returns.

And missing double quotes in about all variable accesses even though most (or all?) are supposed to expand to a single token (the ones that don't should be using arrays anyways.)

Unfortunately the state of bash in Kubernetes got to the point where putting out fires is getting harder and harder... Probably the time to start rethinking this and replacing it with Go or if not Go then Python. Shell is just not maintainable.

This is what people smarter than me from a quite successful software company had to say about it:
https://google-styleguide.googlecode.com/svn/trunk/shell.xml?showone=When_to_use_Shell#When_to_use_Shell