Adds JUJU to the Kubernetes Provider listing #5414

lazypower · 2015-03-12T23:39:22Z

This feature adds Juju provisioning to the kube-up script. It currently
parses out the pre-requisits on debian/ubuntu based systems and installs
them if they are missing.

From there we followed the integration path that was found in the
libvirt-coreos path, implementing the methods found in the boilerplate
and calling juju service calls. There are a few "arbitrary sleeps" in
the code to allow the cloud provider to settle and properly deploy.
These are work-around cases from the script executing faster than juju
was able to communicate from the state server to subsequent nodes. I
left comments inline at these points.

To exercise this:

export KUBERNETES_PROVIDER=juju
cluster/kube-up.sh

It will spin up a ref arch with 1 Kubernetes Master, 2 minions, and run
the cluster validation checks against the deployment. Bridging the gap
between the juju specific bits and the upstream recommended guides for
getting started with Kubernetes.

To note, if you do not have a "current environment" set in Juju, it will
spin up the quickstart integration wizard in interactive mode, allowing
you to configure juju, and add the proper provider/use it. Otherwise it
assumes you're in the provider you wish to use, and will deploy there.

This feature adds Juju provisioning to the kube-up script. It currently parses out the pre-requisits on debian/ubuntu based systems and installs them if they are missing. From there we followed the integration path that was found in the libvirt-coreos path, implementing the methods found in the boilerplate and calling juju service calls. There are a few "arbitrary sleeps" in the code to allow the cloud provider to settle and properly deploy. These are work-around cases from the script executing faster than juju was able to communicate from the state server to subsequent nodes. I left comments inline at these points. To exercise this: export KUBERNETES_PROVIDER=juju cluster/kube-up.sh It will spin up a ref arch with 1 Kubernetes Master, 2 minions, and run the cluster validation checks against the deployment. Bridging the gap between the juju specific bits and the upstream recommended guides for getting started with Juju. To note, if you do not have a "current environment" set in Juju, it will spin up the quickstart integration wizard in interactive mode, allowing you to configure juju, and add the proper provider/use it. Otherwise it assumes you're in the provider you wish to use, and will deploy there.

googlebot · 2015-03-12T23:39:23Z

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.

erictune · 2015-03-13T05:26:02Z

It seems that the flags for the various binaries are stored outside this repo, at places like https://github.com/whitmo/charm-kubernetes-master/blob/master/files/apiserver.upstart.tmpl.

For the other cloud providers which support kube-up.sh, the flags are in our repository. This allows us to make changes in a single commit to both the source code and the flags for all those distros.

Is there a reason to have the code for kubernetes-on-juju split across repos in this way?

lazypower · 2015-03-13T17:38:32Z

Thanks for the review!

The current structure of our repositories reflects a pattern we use to separate concerns in the charm store - but this does not reflect the pattern you would need to perform easy updates as the kubernetes project moves foward. I apologize for not recognizing that before issuing the PR.

I'll get the charms/bundle pulled into this branch and we will certainly look into conglomerating our resources that are outside of the GCP namespace so it's easier to track.

filbranden · 2015-03-13T17:43:31Z

cluster/juju/prereqs/ubuntu-juju.sh

+# Kubernetes Juju Charms project - located here: https://github.com/whitmo/bundle-kubernetes
+
+function check_for_ppa(){
+    grep -s ^ /etc/apt/sources.list /etc/apt/sources.list.d/* | grep juju


Why not just:

grep -qsw juju /etc/apt/sources.list /etc/apt/sources.list.d/*

Use -q to make it quiet and -w to make sure you're matching a whole word (i.e. you want to match juju but not jujuba).

You want to make the ppa name parameterizable, or rename the function to check_for_juju_ppa.

erictune · 2015-03-13T18:58:59Z

@chuckbutler @whitmo

Have you thought about whether your goal is (1) to have periodic releases of Kubernetes-on-JuJu, or (2) to ensure that Kubernetes-on-JuJu is always compatible with the very latest K8s source code. Option (2) comes with higher expectations in terms of test automation and your overall level of involvement in the kubernetes project.

If your goal is (1), then we should talk about whether this PR is needed at all.

If it is (2), then this PR is definitely the right direction, but I expect additional steps will be needed too, such as moving JuJu Charm files into the kubernetes repo, as mentioned above; and running kubernets e2e test continuously, etc.

I'm happy to see either outcome. Also, it is fine to start with 1 and move to 2 in the future.

filbranden · 2015-03-13T18:59:45Z

cluster/juju/util.sh

+   }
+
+function detect-minions(){
+    KUBE_MINION_IP_ADDRESSES=($(juju run --service kubernetes "unit-get private-address" --format=yaml | grep Stdout | cut -d "'" -f 2))


awk can do both of what grep and cut do in a single command... can you use it instead?

filbranden · 2015-03-13T19:04:11Z

Bunch of comments on the bash script... I'll reassign to @erictune to decide whether it's OK to merge though, I'm not very familiar with Juju (well, I know what it is and that's that...) so I'd prefer if someone else would take a look at it.

Let me know if you need me to take another look at the shell scripts later in the code review.

Cheers,
Filipe

lazypower · 2015-03-13T19:31:32Z

Thank you for the review @filbranden. I'll be working on implementing the comments and should have this ready for re-review on Monday.

Cheers!

erictune · 2015-03-13T20:38:37Z

Thanks, @filbranden. All your comments look reasonable to me.

- Changed check_for_ppa to be parameterized - Added bash strictmode - refactored the package_status method to consume variables and be a bit nicer to future re-use of the method. - Cut out extra echo -n statements in favor of tr -d or native awk - Refactored branching logic paths to leverage double brackets - normalized local variable annotation - Updated globals to be all CAPS - remainder of filbrandens feedback in validate-cluster.sh

erictune · 2015-03-17T18:40:12Z

@chuckbutler hoping to hear your thoughts on #5414 (comment)

mbruzek · 2015-03-18T15:45:03Z

@erictune,

We’ve had a bit of a discussion around this to ensure everyone working on the Juju Kubernetes integration is on the same page. We’re all in agreement that having the Juju Charms and Bundles in the Kubernetes repository is in the best interest of our project moving forward. We’re actively working on addressing the automation of e2e testing - and currently have some basic stand up testing and validation running in our CI. It’s not as automated as requested above, but we’re working towards that end. If you’re interested in the CI reports, you can view them here: http://reports.vapour.ws/all-bundle-and-charm-results/gh%3Awhitmo%252Fbundle-kubernetes

Adding the visibility of Juju as an officially supported (and hopefully recommended) method to deploy K8’s - we feel this brings users a new and different way to deploy a cluster that has implications beyond just standing up Kubes.

We plan to continue adjusting the charms to ensure they work with the latest upstream, and to provide configuration options to run different releases. We would like to continue to support this moving forward as the project evolves.

What do you feel are the next steps for the team to proceed forward?

erictune · 2015-03-26T17:47:15Z

I'm happy to hear you plan to work on e2e testing, and I'm happy for users to have Juju as an option for deploying Kubernetes.

I think the next steps are:

you fix the build failure
I'll merge this PR
over time, the Juju+Kubernetes folks to try to get as many of the e2e tests passing as possible.
as you do that, you will probably send additional PRs to extent the cluster/juju/... files.
we may make reasonable requests for features or file reorganizaton on those future PRs

erictune · 2015-03-26T17:48:46Z

Looking at the build failure, it looks like it doesn't like the boilerplate at the top of your shell scripts.
You may need to do this to run precommit checks:

cd kubernetes/.git/hooks/
ln -s ../../hooks/prepare-commit-msg .
ln -s ../../hooks/commit-msg .

erictune · 2015-03-26T17:49:55Z

cluster/juju/prereqs/ubuntu-juju.sh

+
+set -euo pipefail
+
+# Copyright 2014 Canonical LTD. All rights reserved.


I don't think we can take code that has anything other than the Kubernetes boilerplate at the top. See comment on conversation tab about github hooks.

Brilliant. Thanks for following up on this Eric. I'll get the headers updated and link in the hooks.
Sent using Boxer
On Mar 26, 2015 1:50 PM, Eric Tune notifications@github.com wrote:In cluster/juju/prereqs/ubuntu-juju.sh:

@@ -0,0 +1,48 @@
+#!/bin/bash
+
+set -euo pipefail
+
+# Copyright 2014 Canonical LTD. All rights reserved.

I don't think we can take code that has anything other than the Kubernetes boilerplate at the top. See comment on conversation tab about github hooks.

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

erictune · 2015-03-26T18:56:13Z

I've looked several places and I don't see a CLA that would apply to this PR. Let me know if you think this is wrong, or need to talk about it off-PR.

lazypower · 2015-03-26T18:59:00Z

@erictune we're at a point that we should probably talk off PR. I'm going to pull @whitmo and @mbruzek into the thread.

because of failing build notifications on kubernetes#5414

lazypower · 2015-03-31T14:33:25Z

@erictune I've got confirmation from legal that our CLA is now signed, and our team has joined the CLA google group. Can you confirm that this is indeed the case?

erictune · 2015-03-31T18:33:18Z

I can't see the CLA yet. Let's wait one day to see if things need time to propagate through my side.

erictune · 2015-04-01T22:19:15Z

@chuckbutler contact me at etune@google.com

erictune · 2015-04-02T16:17:25Z

Ongoing offline CLA discussion...

erictune · 2015-04-02T16:27:49Z

I've verified CLA offline.

Adds JUJU to the Kubernetes Provider listing

googlebot added the cla: no label Mar 12, 2015

lazypower mentioned this pull request Mar 12, 2015

Tracking conversion of "Getting Started Guides" to use apiserver for kubelet and kube-proxy. #4334

Closed

satnam6502 assigned filbranden Mar 13, 2015

erictune assigned erictune and unassigned filbranden Mar 13, 2015

filbranden reviewed Mar 13, 2015
View reviewed changes

bgrant0607 mentioned this pull request Mar 18, 2015

Create a CoreOS (fleet) cloudprovider #2890

Closed

erictune reviewed Mar 26, 2015
View reviewed changes

Rewrote shell script headers with accepted kubernetes boilerplate

a68de62

because of failing build notifications on kubernetes#5414

erictune added a commit that referenced this pull request Apr 2, 2015

Merge pull request #5414 from chuckbutler/juju-cluster-up

ecf39a6

Adds JUJU to the Kubernetes Provider listing

erictune merged commit ecf39a6 into kubernetes:master Apr 2, 2015

mbruzek mentioned this pull request Apr 22, 2015

Add the Juju charms to Kubernetes #7195

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adds JUJU to the Kubernetes Provider listing #5414

Adds JUJU to the Kubernetes Provider listing #5414

lazypower commented Mar 12, 2015

googlebot commented Mar 12, 2015

erictune commented Mar 13, 2015

lazypower commented Mar 13, 2015

filbranden Mar 13, 2015

filbranden Mar 13, 2015

erictune commented Mar 13, 2015

filbranden Mar 13, 2015

filbranden commented Mar 13, 2015

lazypower commented Mar 13, 2015

erictune commented Mar 13, 2015

erictune commented Mar 17, 2015

mbruzek commented Mar 18, 2015

erictune commented Mar 26, 2015

erictune commented Mar 26, 2015

erictune Mar 26, 2015

lazypower Mar 26, 2015

erictune commented Mar 26, 2015

lazypower commented Mar 26, 2015

lazypower commented Mar 31, 2015

erictune commented Mar 31, 2015

erictune commented Apr 1, 2015

erictune commented Apr 2, 2015

erictune commented Apr 2, 2015


		set -euo pipefail

		# Copyright 2014 Canonical LTD. All rights reserved.

Adds JUJU to the Kubernetes Provider listing #5414

Adds JUJU to the Kubernetes Provider listing #5414

Conversation

lazypower commented Mar 12, 2015

googlebot commented Mar 12, 2015

erictune commented Mar 13, 2015

lazypower commented Mar 13, 2015

filbranden Mar 13, 2015

Choose a reason for hiding this comment

filbranden Mar 13, 2015

Choose a reason for hiding this comment

erictune commented Mar 13, 2015

filbranden Mar 13, 2015

Choose a reason for hiding this comment

filbranden commented Mar 13, 2015

lazypower commented Mar 13, 2015

erictune commented Mar 13, 2015

erictune commented Mar 17, 2015

mbruzek commented Mar 18, 2015

erictune commented Mar 26, 2015

erictune commented Mar 26, 2015

erictune Mar 26, 2015

Choose a reason for hiding this comment

lazypower Mar 26, 2015

Choose a reason for hiding this comment

erictune commented Mar 26, 2015

lazypower commented Mar 26, 2015

lazypower commented Mar 31, 2015

erictune commented Mar 31, 2015

erictune commented Apr 1, 2015

erictune commented Apr 2, 2015

erictune commented Apr 2, 2015