-
Notifications
You must be signed in to change notification settings - Fork 40.1k
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
Create a standalone k8s binary, capable of running a full cluster #2121
Conversation
eff91f1
to
d953a94
Compare
@@ -358,3 +364,42 @@ func (h *EtcdHelper) AtomicUpdate(key string, ptrToType runtime.Object, tryUpdat | |||
return err | |||
} | |||
} | |||
|
|||
func checkEtcd(host string) error { | |||
response, err := http.Get(host + "/version") |
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.
IIRC version can race on startup because it loads earlier than other attributes. Can't find the issue now, but we switched to reading the root key in a few places because of it.
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.
Yeah - /version
returns 200s before things are up and really running. See: #963 (comment)
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 that's ok. Nothing crashes if etcd isn't fully there, and if there
is a /version there, it will get there eventually...
I can switch to hitting the root key, if we really want to, but unlike the
integration tests, I don't immediately start hammering the server and
expecting correct responses.
--brendan
On Sun, Nov 2, 2014 at 10:11 AM, Joe Beda notifications@github.com wrote:
In pkg/tools/etcd_tools.go:
@@ -358,3 +364,42 @@ func (h *EtcdHelper) AtomicUpdate(key string, ptrToType runtime.Object, tryUpdat
return err
}
}
+
+func checkEtcd(host string) error {
- response, err := http.Get(host + "/version")
Yeah - /version returns 200s before things are up and really running.
See: #963 (comment)
#963 (comment)—
Reply to this email directly or view it on GitHub
https://github.com/GoogleCloudPlatform/kubernetes/pull/2121/files#r19713556
.
Is there a plan to add an option in the future to just start as go-routines what would typically go in the kubernetes-master, and an option to just start what would go on the kubernetes-minions using this single top-level binary instead of the separate daemons? I like this as it more aligns with what we started doing in OpenShift Origin v3, and makes the CLI setup much simpler. |
Yes, I'd like to achieve this. So there's a single binary for everything, There's more work to do to achieve this, mostly around networking, but yes, --brendan On Sun, Nov 2, 2014 at 7:03 PM, Derek Carr notifications@github.com wrote:
|
Structurally, we went with a config -> Run pattern for each of the routines. https://github.com/openshift/origin/blob/master/pkg/cmd/server/kubernetes/master.go has some examples of how we launch the master, although some of this we have to change as the authorizer changes @erictune is working on. |
Would you either change this PR or file an issue so that cmd/apiserver calls RunApiServer |
Agree that's the long term goal. I'll file an issue. On Mon, Nov 3, 2014 at 8:48 AM, Eric Tune notifications@github.com wrote:
|
So, is your long term goal to get rid of the standalone binaries? |
What is the motivation behind this? For testing? or other stuff? I couldn't figure out by reading the PR. |
ok, @bgrant0607 just pointed #108 to me. |
The goal of this is to both reduce the size of our download (since each Also to make local development/experimentation easier, since you only need --brendan On Mon, Nov 3, 2014 at 11:49 AM, Dawn Chen notifications@github.com wrote:
|
Ok, with this PR, are all cmd/$binary should be removed by this single binary? |
d953a94
to
c268d49
Compare
Eventually I will remove binaries, but for now, I want to just check in the standalone version. Rebased, please take another look. Thanks! |
c268d49
to
2c12218
Compare
When you move on to the next step I want to sync about standardizing a run in a Docker container config for the Kubelet - I got the first steps working in OpenShift re: running in a container (https://lists.openshift.redhat.com/openshift-archives/dev/2014-November/msg00042.html) but I want to figure out what security implications and patterns are going to work best for that and make sure we match.
|
Travis break was a flake. I can have mergez? |
LGTM |
Create a standalone k8s binary, capable of running a full cluster
mux := http.NewServeMux() | ||
apiserver.NewAPIGroup(m.API_v1beta1()).InstallREST(mux, "/api/v1beta1") | ||
apiserver.NewAPIGroup(m.API_v1beta2()).InstallREST(mux, "/api/v1beta2") | ||
apiserver.InstallSupport(mux) |
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 apiserver setup code looks wrong. https://github.com/GoogleCloudPlatform/kubernetes/blob/master/cmd/apiserver/apiserver.go is quite different.
Should extract code into a library-- perhaps in pkg/master/cmd/ or something.
Not much point to an all-in-one binary if it doesn't behave the same as our current binaries.
@lavalamp I'm going to be picking this stuff up. I'll continue to refactor this and will make sure you are the reviews. Thanks. |
Yeah, I have to fix this. standalone.go is doing things that now require access to private members of the master. |
I think this was forked off of cmd/apiserver.go before @erictune's refactoring. Otherwise I'm not sure how it got like this. |
Anything standalone go needs Openshift will need :)
|
I think I can just delete the mux and calls to NewAPIGroup. All of that is already done by master.New. |
Ahh, just realized you guys were talking about this PR which I merged two days ago. Yes, this was merged before @erictune's refactoring and auth stuff in. Now I believe this shouldn't work with the rest of system with newly PRs for auth. My bad. I should insist on cleaning / consolidate two copies of apiserver, or at least add a TODO in cmd/apiserver/apiserver.go. |
Pleases nobody touch this now |
See #108