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

Add containerized option to kubelet binary #7741

Merged
merged 1 commit into from May 4, 2015
Merged

Add containerized option to kubelet binary #7741

merged 1 commit into from May 4, 2015

Conversation

pmorie
Copy link
Member

@pmorie pmorie commented May 4, 2015

@vmarmol PTAL

vmarmol
vmarmol reviewed May 4, 2015
View reviewed changes
cmd/kubelet/app/server.go
@@ -213,6 +215,7 @@ func (s *KubeletServer) AddFlags(fs *pflag.FlagSet) {
// Flags intended for testing, not recommended used in production environments.
fs.BoolVar(&s.ReallyCrashForTesting, "really-crash-for-testing", s.ReallyCrashForTesting, "If true, when panics occur crash. Intended for testing.")
fs.Float64Var(&s.ChaosChance, "chaos-chance", s.ChaosChance, "If > 0.0, introduce random client errors and latency. Intended for testing. [default=0.0]")
fs.BoolVar(&s.Containerized, "containerized", s.Containerized, "Experimental support for running kubelet in a container. Intended for testing.")
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: add "Default: false"

@vmarmol
Copy link
Contributor

vmarmol commented May 4, 2015

LGTM

@pmorie
Copy link
Member Author

pmorie commented May 4, 2015

@vmarmol Fixed

@vmarmol
Copy link
Contributor

vmarmol commented May 4, 2015

Thanks @pmorie! Will merge on green.

@vmarmol vmarmol self-assigned this May 4, 2015
@vmarmol vmarmol added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 4, 2015
@pmorie
Copy link
Member Author

pmorie commented May 4, 2015

@vmarmol tomorrow I'll do PRs for the image and local-up-cluster accommodation for this use-case

vmarmol added a commit that referenced this pull request May 4, 2015
@vmarmol
Merge pull request #7741 from pmorie/containerized
9340b7a 
Add containerized option to kubelet binary
@vmarmol vmarmol merged commit 9340b7a into kubernetes:master May 4, 2015
@vmarmol
Copy link
Contributor

vmarmol commented May 4, 2015

Awesome! Merged :)

@ixdy
Copy link
Member

ixdy commented May 4, 2015

This PR broke the non-linux builds:

+++ [0504 23:19:57] darwin/amd64: go build started
# github.com/GoogleCloudPlatform/kubernetes/cmd/kubelet/app
_output/dockerized/go/src/github.com/GoogleCloudPlatform/kubernetes/cmd/kubelet/app/server.go:280: undefined: mount.NsenterMounter
!!! Error in /go/src/github.com/GoogleCloudPlatform/kubernetes/hack/lib/golang.sh:340
  'go install "${goflags[@]:+${goflags[@]}}" -ldflags "${version_ldflags}" "${nonstatics[@]:+${nonstatics[@]}}"' exited with status 2
Call stack:
  1: /go/src/github.com/GoogleCloudPlatform/kubernetes/hack/lib/golang.sh:340 kube::golang::build_binaries_for_platform(...)
  2: /go/src/github.com/GoogleCloudPlatform/kubernetes/hack/lib/golang.sh:446 kube::golang::build_binaries(...)
  3: hack/build-cross.sh:31 main(...)
Exiting with status 1
+++ [0504 23:19:57] darwin/386: go build started
# github.com/GoogleCloudPlatform/kubernetes/cmd/kubelet/app
_output/dockerized/go/src/github.com/GoogleCloudPlatform/kubernetes/cmd/kubelet/app/server.go:280: undefined: mount.NsenterMounter
!!! Error in /go/src/github.com/GoogleCloudPlatform/kubernetes/hack/lib/golang.sh:340
  'go install "${goflags[@]:+${goflags[@]}}" -ldflags "${version_ldflags}" "${nonstatics[@]:+${nonstatics[@]}}"' exited with status 2
Call stack:
  1: /go/src/github.com/GoogleCloudPlatform/kubernetes/hack/lib/golang.sh:340 kube::golang::build_binaries_for_platform(...)
  2: /go/src/github.com/GoogleCloudPlatform/kubernetes/hack/lib/golang.sh:446 kube::golang::build_binaries(...)
  3: hack/build-cross.sh:31 main(...)
Exiting with status 1
+++ [0504 23:19:57] windows/amd64: go build started
# github.com/GoogleCloudPlatform/kubernetes/cmd/kubelet/app
_output/dockerized/go/src/github.com/GoogleCloudPlatform/kubernetes/cmd/kubelet/app/server.go:280: undefined: mount.NsenterMounter
!!! Error in /go/src/github.com/GoogleCloudPlatform/kubernetes/hack/lib/golang.sh:340
  'go install "${goflags[@]:+${goflags[@]}}" -ldflags "${version_ldflags}" "${nonstatics[@]:+${nonstatics[@]}}"' exited with status 2
Call stack:
  1: /go/src/github.com/GoogleCloudPlatform/kubernetes/hack/lib/golang.sh:340 kube::golang::build_binaries_for_platform(...)
  2: /go/src/github.com/GoogleCloudPlatform/kubernetes/hack/lib/golang.sh:446 kube::golang::build_binaries(...)
  3: hack/build-cross.sh:31 main(...)
Exiting with status 1

@justinsb justinsb mentioned this pull request May 4, 2015
Closed
@vmarmol
Copy link
Contributor

vmarmol commented May 4, 2015

Grr sorry @ixdy. @pmorie we need to add the stub version for non-Linux systems.

Will revert and we'll try again. Thanks for the notice!

@vmarmol vmarmol mentioned this pull request May 4, 2015
Merged
@pmorie
Copy link
Member Author

pmorie commented May 4, 2015

Dang
On Mon, May 4, 2015 at 7:34 PM Victor Marmol notifications@github.com
wrote:

Grr sorry @ixdy https://github.com/ixdy. @pmorie
https://github.com/pmorie we need to add the stub version for non-Linux
systems.

Will revert and we'll try again. Thanks for the notice!

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

@pmorie
Copy link
Member Author

pmorie commented May 4, 2015

My bad! I will fix
On Mon, May 4, 2015 at 7:43 PM Paul Morie pmorie@gmail.com wrote:

Dang
On Mon, May 4, 2015 at 7:34 PM Victor Marmol notifications@github.com
wrote:

Grr sorry @ixdy https://github.com/ixdy. @pmorie
https://github.com/pmorie we need to add the stub version for
non-Linux systems.

Will revert and we'll try again. Thanks for the notice!

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

@vmarmol
Copy link
Contributor

vmarmol commented May 4, 2015

No worries, should have caught it in review. Also, we're not running build cross in CI? Let me look into fixing that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@vmarmol vmarmol

Labels
lgtm "Looks good to me", indicates that a PR is ready to be merged.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@pmorie @vmarmol @ixdy @googlebot