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.

Sign up for GitHub

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

Jump to bottom

Extract keygen image #19241

Merged
merged 8 commits into from
Jan 21, 2016
Merged

Extract keygen image #19241

merged 8 commits into from
Jan 21, 2016

Conversation

karlkfi
Copy link
Contributor

@karlkfi karlkfi commented Jan 4, 2016

The mesos/docker cluster uses a docker image to generate rsa keys, ssl root authorities, and ssl certificates. I've extracted this docker image to its own repo.

This is part of an effort to make the mesos/docker cluster scripts more maintainable and abstract, to enable extraction of a similar non-mesos docker cluster.

Changes:

  • Move keygen image mesosphere/kubernetes-mesos-keygen -> mesosphere/kubernetes-keygen:v1.0.0
  • Remove resolveip in favor of github.com/karlkfi/resolveip (resolveip.sh)
  • Remove util-temp-dir.sh in favor of github.com/karlkfi/intemp (intemp.sh)
  • Refactor bash code to use intemp (extract functions to scripts)
  • Remove util-ssl.sh in favor of mesosphere/kubernetes-keygen

@karlkfi karlkfi self-assigned this Jan 4, 2016
@k8s-github-robot
Copy link

Labelling this PR as size/XL

@k8s-github-robot k8s-github-robot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Jan 4, 2016
@karlkfi karlkfi added area/security kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. kind/technical-debt and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Jan 4, 2016
@k8s-github-robot
Copy link

Labelling this PR as size/XL

@k8s-github-robot k8s-github-robot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Jan 4, 2016
@k8s-bot
Copy link

k8s-bot commented Jan 4, 2016

GCE e2e test build/test passed for commit f7e6c7dd2688416080188fd0859a22ebe59ff251.

@karlkfi karlkfi mentioned this pull request Jan 4, 2016
Closed
@karlkfi
Copy link
Contributor Author

karlkfi commented Jan 4, 2016

Based on conversation with @sttts I'll be extracting the keygen container source to its own repo so it's easier for us to maintain.

@karlkfi
Copy link
Contributor Author

karlkfi commented Jan 4, 2016

Extracted keygen source and on dockerhub (automated build).

@karlkfi karlkfi force-pushed the karlkfi-docker-cluster branch from 64a77e5 to 4c0b404 Compare January 4, 2016 23:49
karlkfi
karlkfi reviewed Jan 4, 2016
View reviewed changes
cluster/mesos/docker/deploy-addons.sh
if [ "${ENABLE_CLUSTER_DNS}" == "true" ]; then
echo "Deploying DNS Addon" 1>&2
#TODO: curling to get a script to make a tempdir is a bit overkill...
curl -o- https://raw.githubusercontent.com/karlkfi/intemp/v1.0.0/intemp.sh | bash -s -- -t 'kube-dns' "${bin}/deploy-dns.sh"
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'm not a big fan of curl bash piping, but there doesn't seem to be any other good way to manage external bash dependencies without requiring the user to have them installed and in PATH. On the plus side, it's executing a versioned file on github, and not something on head of master, but it's still not exactly faster or secure.

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 thought about maybe putting intemp in a docker container, but that seems like even more overkill for such a small function...

@k8s-bot
Copy link

k8s-bot commented Jan 5, 2016

GCE e2e test build/test passed for commit 64a77e5196e94152446e3241334706cb1c2b7e75.

@k8s-bot
Copy link

k8s-bot commented Jan 5, 2016

GCE e2e test build/test passed for commit 4c0b404e9c0537e86704569833b521ceaa659e24.

@k8s-bot
Copy link

k8s-bot commented Jan 5, 2016

GCE e2e test build/test passed for commit 790ad3905cf6d38a66942b717681c6417794b499.

@k8s-bot
Copy link

k8s-bot commented Jan 5, 2016

GCE e2e test build/test passed for commit d72319b697c012edf5d46c53580c68b08df7e20d.

@karlkfi karlkfi changed the title [WIP] Extract keygen image Extract keygen image Jan 5, 2016
@karlkfi
Copy link
Contributor Author

karlkfi commented Jan 5, 2016

@sttts you wanna give this another pass?

@karlkfi karlkfi assigned sttts and unassigned karlkfi Jan 5, 2016
Karl Isenberg added 3 commits January 4, 2016 19:17
Extract kubernetes-keygen to its own repo: mesosphere/kubernetes-keygen
1938414 
- Move keygen image mesosphere/kubernetes-mesos-keygen -> mesosphere/kubernetes-keygen:v1.0.0
- Remove resolveip in favor of github.com/karlkfi/resolveip (resolveip.sh)
- Remove util-temp-dir.sh in favor of github.com/karlkfi/intemp (intemp.sh)
- Refactor bash code to use intemp (extract functions to scripts)
- Remove util-ssl.sh in favor of mesosphere/kubernetes-keygen
Fix typo
54010e5
Fix buffer_output
4ca42f0
@sttts
Copy link
Contributor

sttts commented Jan 6, 2016

lgtm

@karlkfi
Copy link
Contributor Author

karlkfi commented Jan 6, 2016

Flakey mesos unit tests?

FAIL    k8s.io/kubernetes/contrib/mesos/pkg/runtime 1.399s

@karlkfi
Copy link
Contributor Author

karlkfi commented Jan 6, 2016

@k8s-bot test this please

@k8s-bot
Copy link

k8s-bot commented Jan 6, 2016

GCE e2e build/test failed for commit 36f2161.

@karlkfi
Copy link
Contributor Author

karlkfi commented Jan 7, 2016

Hey, @davidopp. This PR vendors in a bash library I wrote into /third_party/intemp. I don't think we have any other vendored bash code, and there doesn't seem to be any real standard vendoring mechanism for the bash in general. I see we have some other vendored and forked code in third_party so it seemed like a decent option, but it's not in a mesos-specific location, so I wanted to get some kind sign off.

@jdef jdef assigned jdef and davidopp and unassigned sttts and jdef Jan 10, 2016
@jdef
Copy link
Contributor

jdef commented Jan 14, 2016

@davidopp PTAL

@karlkfi
Copy link
Contributor Author

karlkfi commented Jan 14, 2016

Scale test flake:

Summarizing 1 Failure:

[Fail] Deployment [It] deployment should scale up and down in the right order 
/go/src/k8s.io/kubernetes/_output/dockerized/go/src/k8s.io/kubernetes/test/e2e/deployment.go:232

Ran 121 of 213 Specs in 522.859 seconds
FAIL! -- 120 Passed | 1 Failed | 2 Pending | 90 Skipped

@karlkfi
Copy link
Contributor Author

karlkfi commented Jan 14, 2016

@k8s-bot test this please

@k8s-bot
Copy link

k8s-bot commented Jan 14, 2016

GCE e2e build/test failed for commit 36f2161.

@karlkfi
Copy link
Contributor Author

karlkfi commented Jan 15, 2016

yet more flakiness...

Summarizing 1 Failure:

[Fail] Pods [It] should *not* be restarted with a /healthz http liveness probe [Conformance] 
/go/src/k8s.io/kubernetes/_output/dockerized/go/src/k8s.io/kubernetes/test/e2e/pods.go:109

Ran 116 of 214 Specs in 427.153 seconds
FAIL! -- 115 Passed | 1 Failed | 2 Pending | 96 Skipped

@karlkfi
Copy link
Contributor Author

karlkfi commented Jan 15, 2016

@k8s-bot e2e test this please

@k8s-bot
Copy link

k8s-bot commented Jan 15, 2016

GCE e2e test build/test passed for commit 36f2161.

@k8s-bot
Copy link

k8s-bot commented Jan 17, 2016

GCE e2e test build/test passed for commit 36f2161.

@davidopp
Copy link
Member

LGTM

@davidopp davidopp added ok-to-merge lgtm "Looks good to me", indicates that a PR is ready to be merged. and removed e2e-not-required labels Jan 18, 2016
@k8s-github-robot
Copy link

@k8s-bot test this

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

@k8s-bot
Copy link

k8s-bot commented Jan 18, 2016

GCE e2e test build/test passed for commit 36f2161.

@k8s-github-robot
Copy link

@k8s-bot test this

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

@k8s-bot
Copy link

k8s-bot commented Jan 21, 2016

GCE e2e test build/test passed for commit 36f2161.

@k8s-github-robot
Copy link

@k8s-bot test this [submit-queue is verifying that this PR is safe to merge]

@k8s-bot
Copy link

k8s-bot commented Jan 21, 2016

GCE e2e test build/test passed for commit 36f2161.

alex-mohr added a commit that referenced this pull request Jan 21, 2016
@alex-mohr
Merge pull request #19241 from mesosphere/karlkfi-docker-cluster
8f0ab67 
Extract keygen image
@alex-mohr alex-mohr merged commit 8f0ab67 into kubernetes:master Jan 21, 2016
@jdef jdef deleted the karlkfi-docker-cluster branch January 21, 2016 18:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@davidopp davidopp

Labels
area/security area/test-infra kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@karlkfi @k8s-github-robot @k8s-bot @sttts @jdef @davidopp @k8s-teamcity-mesosphere @googlebot @alex-mohr