Can one of the admins verify that this patch is reasonable to test? (reply "ok to test", or if you trust the user, reply "add to whitelist")

If this message is too spammy, please complain to ixdy.

Labelling this PR as size/M

ok to test

GCE e2e test build/test passed for commit e15058fe989fadbb22d96112a6ceb1e4df5d7108.

• Build Log
• Test Artifacts
• Internal Jenkins Results

The author of this PR is not in the whitelist for merge, can one of the admins add the 'ok-to-merge' label?

GCE e2e test build/test passed for commit 31e27a19800b7794cd9d55907a9ee3bd390dc9b1.

• Build Log
• Test Artifacts
• Internal Jenkins Results

GCE e2e test build/test passed for commit 825f2d8608754409563f565a85492ba47ffaf161.

• Build Log
• Test Artifacts
• Internal Jenkins Results

GCE e2e test build/test passed for commit 937e9412796cfdc4ed973828c03484ac7f6659a9.

• Build Log
• Test Artifacts
• Internal Jenkins Results

GCE e2e test build/test passed for commit f4cac5dde390a339caece06cdc8d4e33aa673f2b.

• Build Log
• Test Artifacts
• Internal Jenkins Results

This is a work in progress, not ready for merge yet.

GCE e2e test build/test passed for commit fe7efc180ae9085f6ebe817001ab16a2d97388a4.

• Build Log
• Test Artifacts
• Internal Jenkins Results

GCE e2e test build/test passed for commit 7f83398e4282e39fea24cab939c86992135d74b7.

• Build Log
• Test Artifacts
• Internal Jenkins Results

GCE e2e build/test failed for commit 9cf34bc3bdc7b425cd2fd71aeca572db8e47d1f9.

• Build Log
• Test Artifacts
• Internal Jenkins Results

GCE e2e build/test failed for commit 99e5e7c262e028a32dfbb4678841bea3dc52f360.

• Build Log
• Test Artifacts
• Internal Jenkins Results

@MikeSpreitzer high order bit is I'd really like to streamline this with local-up-cluster and give that the ability to handle CNI plugins as well, because that's used by a wider population and so less likely to break silently. Optionally, it would be cool if I could just run CNI plugins that were auto installed from a tarball at a url (https://github.com/appc/cni/releases/tag/v0.1.0 perhaps?).

@resouer as defacto maintainer of ubuntu install

Regarding local-up-cluster.sh, I see that it does not now share a lot with the cluster/ubuntu scripting, but I think @bprashanth you are saying you would like to change that. While that may be a laudatory goal, it seems to me to be a separate project from adding CNI support. Perhaps you are saying the unification project should come first. I will not object if it happens quickly, but that might have to be done by other hands than mine since I am new to all this. @resouer what do you think?

I have added (1) comments documenting several functions in util.sh and (2) sanity checking of the new CNI configuration envars, as requested.
I think the other two suggestions mentioned above --- tarball URL instead of file pathnames, and unifying with local-up-cluster.sh --- are separable projects that should be left for separate PRs; this one is big enough already.
The tarball suggestion sounds intriguing but I think it is pulling on a bigger ball of yarn, namely establishing a standard archive format for containing a CNI plugin. I think that is a potentially good thing, but deserves some discussion of its own merits in a community broader than that reviewing this PR. And it can be added as a later after this PR is merged, if ultimately desired.
The unification with local-up-cluster.sh is clearly a distinct project. It strikes me as a bit odd to unify local-up-cluster.sh with one of the several providers for clusters.

GCE e2e test build/test passed for commit 2224629.

• Build Log
• Test Artifacts
• Internal Jenkins Results

The unification with local-up-cluster.sh is clearly a distinct project. It strikes me as a bit odd to unify local-up-cluster.sh with one of the several providers for clusters.

We're talking about sharing code in a common library. Any number of providers can use it. Local-up-cluster is just the most common script. Did it not work, or just too much to refactor? it's ok to defer if that's the case

I did not try a refactoring. I'm not sure what makes sense. Are you suggesting to move the create-kube-*-opts functions (with the style of the function names adjusted accordingly) from cluster/ubuntu/util.sh into hack/lib/util.sh? That itself would be easy to do, I see no serious difficutly (just the need to make sure hack/lib/util.sh is available everywhere that cluster/ubuntu/util.sh is sourced). It just seems a little odd to me, to move a piece of cluster/ubuntu into hack/lib.

GCE e2e test build/test passed for commit 6b8e7e1.

• Build Log
• Test Artifacts
• Internal Jenkins Results

@bprashanth ?
@dalanlan ?
@zmerlynn ?

LGTM. Beware that ther's a pretty high chance this will drift out of sync as we start making modifications to CNI integration for 1.3, unless you pay close attention to it. If you found a way to share the CNI parts with local-up-cluster, many more people will pay attention to keeping it in sync, but that can easily come in a follow up pr.

@k8s-bot test this

Tests are more than 48 hours old. Re-running tests.

GCE e2e build/test passed for commit 6b8e7e1.

• Build Log
• Test Artifacts
• Internal Jenkins Results

Automatic merge from submit-queue

@bprashanth - thanks for the heads-up and the progress.