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

Kubelet talks securely to apiserver #2387

Merged
merged 3 commits into from Nov 18, 2014

Conversation

erictune
Copy link
Member

Configure apiserver to serve Securely on port 6443.
Generate token for kubelets during master VM startup.
Put token into file apiserver can get and another file the kubelets can get.

@erictune
Copy link
Member Author

@jbeda looking for confirmation that this is the right approach.

If so, I will add to aws, and try to add to other cloud providers.

Also, working on an e2e test that will be part of the final PR.

@@ -28,3 +28,19 @@ EOF

mkdir -p /srv/salt-overlay/salt/nginx
echo $MASTER_HTPASSWD > /srv/salt-overlay/salt/nginx/htpasswd

# TODO: do aws.
Copy link
Contributor

Choose a reason for hiding this comment

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

We have more than AWS -- we need to get vSphere, rackspace and vagrant too.

@jbeda
Copy link
Contributor

jbeda commented Nov 15, 2014

Approach looks good to me!

To be honest, this doesn't give us much -- we auto-accept minions to the master now...

@erictune
Copy link
Member Author

I don't get your comment about this not buying much.

@brendandburns
Copy link
Contributor

Eric,
Currently Salt is set up in such a way that it will accept anything as a
minion/worker. That means it's pretty easy to spoof that you are a minion
to the master. The correct approach here is to pre-populate the salt keys
into the master and minion from the startup script. (obviously doesn't
have to be in this PR)

--brendan

On Fri, Nov 14, 2014 at 5:03 PM, Eric Tune notifications@github.com wrote:

I don't get your comment about this not buying much.


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

@erictune
Copy link
Member Author

by startup-script you mean the gce startup script?

@brendandburns
Copy link
Contributor

No, the scripts that actually build the release. We should have the
release tarball include the keys that the salt minions should use, and the
salt masters should accept.

Although, to be honest, this may all be a little bit moot, as I think we're
going to try to move away from using salt for (at least) worker node
configuration.

--brendan

On Fri, Nov 14, 2014 at 5:26 PM, Eric Tune notifications@github.com wrote:

by startup-script you mean the gce startup script?


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

@jbeda
Copy link
Contributor

jbeda commented Nov 17, 2014

@erictune Let me know when you are ready for me to take another pass. We need to be somewhat careful that we don't break other cloud/deployments.

@erictune
Copy link
Member Author

@jbeda

Would you be okay with a PR which only adds the new functionality on GCE, and which make no change on other platforms (e.g. using {% if grains.cloud == 'gce' %}). Then I can file issues to add the feature to other platforms. Until other platforms get the feature, the kubelet events would be recorded in a local log but not in the API.

Also, shall I go ahead and file an issue for the salt credential distribution?

@jbeda
Copy link
Contributor

jbeda commented Nov 17, 2014

@erictune That seems reasonable. Or you can take your best guess at making this happen for the other platforms and we can contact owners to verify.

I like the idea of making the kubelet->master communication optional for now and revert to logging events locally. It isn't idea but it gives us a way forward.

@jbeda
Copy link
Contributor

jbeda commented Nov 18, 2014

I'm still worried that we are going to break other clouds. In kube-apiserver/defaults we need to make sure we don't pass the flag if we don't have the auth_file.

I'd test this with vagrant and make sure things still come up.

@erictune
Copy link
Member Author

yep, will do. Just debugging issues on GCP at the moment, but planning to
do just what you said.

On Mon, Nov 17, 2014 at 5:05 PM, Joe Beda notifications@github.com wrote:

I'm still worried that we are going to break other clouds. In
kube-apiserver/defaults we need to make sure we don't pass the flag if we
don't have the auth_file.

I'd test this with vagrant and make sure things still come up.


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

Configure apiserver to serve Securely on port 6443.
Generate token for kubelets during master VM startup.
Put token into file apiserver can get and another file the kubelets can get.
Added e2e test.
@erictune
Copy link
Member Author

Okay, I added checks to the salt to only expect the known_tokens.csv on GCE and a check to the e2e test to only expect events on GCE.

I'm running the e2e on vagrant now (wow that is slow.)

@erictune
Copy link
Member Author

I'll file bugs for other providers after this merges.

@erictune
Copy link
Member Author

Vagrant sets up correctly with the apiserver not crashing. I can't get the e2e test to run for reasons that I think are unrelated to this PR. So, I think this is safe to merge.

@brendandburns
Copy link
Contributor

This lgtm. I'm merging, we'll see if Jenkins is OK with it...

brendandburns added a commit that referenced this pull request Nov 18, 2014
Kubelet talks securely to apiserver
@brendandburns brendandburns merged commit d1768cc into kubernetes:master Nov 18, 2014
@erictune
Copy link
Member Author

thanks.

@erictune
Copy link
Member Author

@brendanburns where is jenkins?
I can't read jenkins.kubernetes.io or jenkins.kubernetes.io:8080

@erictune
Copy link
Member Author

Okay, events are done for GCE. Yay. @dchen1107 @lavalamp

@brendandburns
Copy link
Contributor

Cool! I will get this cherry picked into the 0.5 release.

Thanks!
Brendan
On Nov 18, 2014 9:05 AM, "Eric Tune" notifications@github.com wrote:

Okay, events are done for GCE. Yay. @dchen1107
https://github.com/dchen1107 @lavalamp https://github.com/lavalamp


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

@dchen1107
Copy link
Member

Great, thanks!

@derekwaynecarr
Copy link
Member

@erictune - so this is what caused the e2e tests to not pass because even though the apiserver is up, it broke the kubelet from starting on the minions. I had been looking at other things so had missed this PR, but if you need help in future, please feel free to just mention me and I would be more than happy to test out the change prior to merging.

@erictune
Copy link
Member Author

Sorry for the trouble. Will ask for help next time with vagrant.

On Wed, Nov 19, 2014 at 6:00 PM, Derek Carr notifications@github.com
wrote:

@erictune https://github.com/erictune - so this is what caused the e2e
tests to not pass because even though the apiserver is up, it broke the
kubelet from starting on the minions. I had been looking at other things so
had missed this PR, but if you need help in future, please feel free to
just mention me and I would be more than happy to test out the change prior
to merging.


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

seans3 pushed a commit to seans3/kubernetes that referenced this pull request Apr 10, 2019
…ssion

Add more detailed plan for admission controller and webhook support to dry-run kep
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants