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.

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

Salt reconfiguration to get rid of nginx on GCE #6618

Merged
merged 1 commit into from
Apr 22, 2015

Conversation

roberthbailey
Copy link
Contributor

Instead, pass a bearer token for kubectl to the apiserver.

  • install libcap2-bin w/ apiserver
  • use setcap to allow the apiserver to bind to low numbered ports
  • set secure port for the apiserver to 443
  • Kubelet talk to port 443 on gce

@roberthbailey
Copy link
Contributor Author

I've tested that cluster creation works (as does kubectl for the new cluster) but haven't tested cluster upgrade or run e2e tests yet.

@roberthbailey
Copy link
Contributor Author

@smarterclayton @deads2k @liggitt - Hopefully this is more along the lines of what you were looking for than #6508.

@lavalamp
Copy link
Member

lavalamp commented Apr 9, 2015

heavy_breathing_cat.gif

@zmerlynn
Copy link
Member

zmerlynn commented Apr 9, 2015

Surfacing briefly.. I'm still OOO and probably won't get to this until
tomorrow or so. Access and time has been spottier than anticipated.
On Apr 9, 2015 1:37 PM, "Daniel Smith" [email protected] wrote:

heavy_breathing_cat.gif


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

@roberthbailey
Copy link
Contributor Author

@zmerlynn - don't worry. I wanted to get some early feedback from the RedHat folks on this. I don't want it merged before the 0.15 cut next week.

# is missing.
# Note: we save dot ('.') to $root because the 'with' action overrides it.
# See http://golang.org/pkg/text/template/.
local token='{{$root := .}}{{with index $root "current-context"}}{{with index $root "contexts" .}}{{with index . "user"}}{{with index $root "users" .}}{{index . "token"}}{{end}}{{end}}{{end}}{{end}}'
Copy link
Contributor

Choose a reason for hiding this comment

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

This format just changed (today #6476). See https://gist.github.com/deads2k/c294a882c9358ea7679f for examples of the new template format.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the heads up @deads2k. I'm waiting on a few things before rebasing this:

  1. We need to do a bit of work in GKE to make this a smooth transition. It'll take a couple of weeks for us to get things in place.
  2. Once the apiserver runs in a container, then I won't need to install the libcap2-bin package or run /sbin/setcap on the apiserver, which will simplify things a bit.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've fixed this to use the new template format.

@erictune
Copy link
Member

This looks awesome.

if [ ! -e "${KNOWN_TOKENS_FILE}" ]; then
mkdir -p /srv/salt-overlay/salt/kube-apiserver
(umask 077;
echo "${KUBELET_TOKEN},kubelet,kubelet" > "${KNOWN_TOKENS_FILE}")
echo "${KUBE_BEARER_TOKEN},admin,admin" > "${KNOWN_TOKENS_FILE}";
echo "${KUBELET_TOKEN},kubelet,kubelet" >> "${KNOWN_TOKENS_FILE}")
Copy link
Member

Choose a reason for hiding this comment

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

If I'm being pedantic, this has to handle the upgrade case. See the note on line 273/267 below.

Copy link
Member

Choose a reason for hiding this comment

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

Maybe this should actually be split up into a token directory and reassembled here? (Like how .conf files are done with foo.d/* directories?). Just a random thought so that any future upgrades are less obnoxious.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think your suggestions are orthogonal to this PR (seeing as how the token file already has many files with the service account tokens) but they are good points. Since we don't yet support upgrade, I'm leaning towards leaving this as a breaking change between 0.15.0 and 0.16.0.

Copy link
Member

Choose a reason for hiding this comment

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

SGTM

@roberthbailey
Copy link
Contributor Author

Rebased. PTAL.

@zmerlynn
Copy link
Member

LGTM (sorry, rebase again)

@roberthbailey
Copy link
Contributor Author

Rebased. I also updated some documentation files to talk about bearer tokens instead of basic auth. /cc @jlowdermilk

@roberthbailey
Copy link
Contributor Author

Two issues I'm working on.

  1. The create external load balancer e2e test is failing due to a timeout:
Summarizing 1 Failure:

[Fail] Services [It] should be able to create a functioning external load balancer 
/go/src/github.com/GoogleCloudPlatform/kubernetes/_output/dockerized/go/src/github.com/GoogleCloudPlatform/kubernetes/test/e2e/service.go:339

Ran 35 of 40 Specs in 1244.857 seconds
FAIL! -- 34 Passed | 1 Failed | 1 Pending | 4 Skipped F0420 22:45:12.452252   99306 driver.go:94] At least one test failed
  1. go run hack/e2e.go -v --up is failing to validate the e2e cluster
Running: /Users/robertbailey/Work/go/src/github.com/roberthbailey/kubernetes/hack/e2e-internal/../../cluster/../cluster/gce/../../cluster/../cluster/../cluster/gce/../../cluster/../cluster/../cluster/gce/../../cluster/../_output/dockerized/bin/darwin/amd64/kubectl get cs
2015/04/20 23:05:23 Error running up: exit status 1
2015/04/20 23:05:23 Error starting e2e cluster. Aborting.
exit status 1

@roberthbailey
Copy link
Contributor Author

go run hack/e2e.go --up is failing because of #7139 (and it's broken for me on the master branch as well).

@roberthbailey
Copy link
Contributor Author

@a-robinson pointed out that the reason the e2e test was failing was that I didn't have a firewall to allow http requests in my project. It's now passing as well:

$ go run hack/e2e.go -v --test --test_args="--ginkgo.focus=Service.*functioning external load balancer.*"
...
Ran 1 of 40 Specs in 61.238 seconds
SUCCESS! -- 1 Passed | 0 Failed | 0 Pending | 39 Skipped I0421 15:34:21.475133   42739 driver.go:96] All tests pass

@roberthbailey roberthbailey force-pushed the no-nginx branch 2 times, most recently from 540fb10 to a909838 Compare April 22, 2015 05:45
@roberthbailey roberthbailey changed the title WIP: Salt reconfiguration to get rid of nginx on GCE Salt reconfiguration to get rid of nginx on GCE Apr 22, 2015
 - Configure the apiserver to listen securely on 443 instead of 6443.
 - Configure the kubelet to connect to 443 instead of 6443.
 - Update documentation to refer to bearer tokens instead of basic auth.
@roberthbailey
Copy link
Contributor Author

Now that the latest gcloud release is out, we can move forward with this PR. I rebased on master and re-ran e2e tests. services.sh failed (with the same error I'm seeing in the latest run in Jenkins) but everything else passed.

@zmerlynn
Copy link
Member

LGTM.

nginx-tombstone

@zmerlynn
Copy link
Member

Waiting on green, and feedback to https://groups.google.com/forum/#!topic/kubernetes-dev/nf9L-IeB56k

@zmerlynn zmerlynn added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 22, 2015
zmerlynn added a commit that referenced this pull request Apr 22, 2015
Salt reconfiguration to get rid of nginx on GCE
@zmerlynn zmerlynn merged commit 26aeb6c into kubernetes:master Apr 22, 2015
@roberthbailey roberthbailey deleted the no-nginx branch April 23, 2015 04:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm "Looks good to me", indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants