-
Notifications
You must be signed in to change notification settings - Fork 39.7k
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
Conversation
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. |
@smarterclayton @deads2k @liggitt - Hopefully this is more along the lines of what you were looking for than #6508. |
heavy_breathing_cat.gif |
Surfacing briefly.. I'm still OOO and probably won't get to this until
|
@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}}' |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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:
- 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.
- 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.
There was a problem hiding this comment.
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.
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}") |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SGTM
Rebased. PTAL. |
LGTM (sorry, rebase again) |
7a23393
to
688f382
Compare
Rebased. I also updated some documentation files to talk about bearer tokens instead of basic auth. /cc @jlowdermilk |
Two issues I'm working on.
|
|
@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:
|
540fb10
to
a909838
Compare
- 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.
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. |
Waiting on green, and feedback to https://groups.google.com/forum/#!topic/kubernetes-dev/nf9L-IeB56k |
Salt reconfiguration to get rid of nginx on GCE
Instead, pass a bearer token for kubectl to the apiserver.