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

Kubenet host-port support through iptables #25604

Merged
merged 1 commit into from
May 26, 2016
Merged

Kubenet host-port support through iptables #25604

merged 1 commit into from
May 26, 2016
+500 −115

Conversation

freehan
Copy link
Contributor

@freehan freehan commented May 14, 2016 •
edited by thockin
Loading

@thockin @dcbw

closes #15

@freehan freehan added the release-note-none Denotes a PR that doesn't merit a release note. label May 14, 2016
@freehan freehan force-pushed the kubenethostport branch from fa67e6a to f15ae2f Compare May 14, 2016 02:51
@k8s-github-robot k8s-github-robot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label May 14, 2016
thockin
thockin reviewed May 14, 2016
View reviewed changes
pkg/kubelet/network/kubenet/kubenet_linux.go
execer: utilexec.New(),
podCIDRs: make(map[kubecontainer.ContainerID]string),
hostPortMap: make(map[hostport]closeable),
MTU: 1460,
Copy link
Member

Choose a reason for hiding this comment

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

TODO: don't hardcode this

@freehan freehan force-pushed the kubenethostport branch from f15ae2f to 1e2db18 Compare May 16, 2016 23:44
@freehan
Copy link
Contributor Author

freehan commented May 16, 2016

@thockin Ready for round 2

thockin
thockin reviewed May 17, 2016
View reviewed changes
pkg/kubelet/network/kubenet/kubenet_linux.go
kubenetPostroutingChain utiliptables.Chain = "KUBENET-POSTROUTING"
// the mark-for-masquerade chain
kubenetMarkChain utiliptables.Chain = "KUBENET-MARK"
// TODO: do not hard code mark
Copy link
Member

Choose a reason for hiding this comment

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

How do we resolve this TODO? do we need a flag to kubelet? Why is this a different bit than the kube-proxy? What is the long-term plan to manage these

@freehan freehan force-pushed the kubenethostport branch 2 times, most recently from ce2c777 to 86ab203 Compare May 17, 2016 01:42
@freehan
Copy link
Contributor Author

freehan commented May 17, 2016

@thockin Ready for round 3

@freehan freehan force-pushed the kubenethostport branch 2 times, most recently from d1617d9 to 3281811 Compare May 17, 2016 18:39
thockin
thockin reviewed May 18, 2016
View reviewed changes
pkg/kubelet/network/kubenet/kubenet_linux.go

// the hostport chain
kubenetHostportChain utiliptables.Chain = "KUBENET-HOSTPORT"
// prefix for kubenet hostort chains
Copy link
Member

Choose a reason for hiding this comment

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

typo: hostort

@freehan freehan added this to the v1.3 milestone May 18, 2016
@freehan freehan force-pushed the kubenethostport branch 2 times, most recently from 84a9b40 to 3846ac8 Compare May 18, 2016 23:43
thockin
thockin reviewed May 19, 2016
View reviewed changes
pkg/kubelet/network/kubenet/kubenet_linux.go
@@ -48,6 +53,11 @@ const (
DefaultCNIDir = "/opt/cni/bin"

sysctlBridgeCallIptables = "net/bridge/bridge-nf-call-iptables"

// the hostport chain
kubenetHostportChain utiliptables.Chain = "KUBENET-HOSTPORTS"
Copy link
Member

Choose a reason for hiding this comment

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

nit: kubenetHostportsChain

@thockin
Copy link
Member

thockin commented May 19, 2016

just one name nit, then LGTM. fix and self-apply.

I want to point out that this PR will close #15 - the OLDEST open bug on the project.

@thockin thockin added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-none Denotes a PR that doesn't merit a release note. labels May 19, 2016
@thockin thockin changed the title add hostport support for kubenet Kubenet host-port support through iptables May 19, 2016
@thockin
Copy link
Member

thockin commented May 19, 2016

re-titled for release note.

@freehan freehan force-pushed the kubenethostport branch from 3846ac8 to 4f693a3 Compare May 19, 2016 17:41
@freehan
Copy link
Contributor Author

freehan commented May 19, 2016

@k8s-bot test this issue: #25897

@dcbw
Copy link
Member

dcbw commented May 19, 2016

Could we eventually move most of this to a CNI plugin instead?

@freehan freehan added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 19, 2016
@freehan
Copy link
Contributor Author

freehan commented May 19, 2016

That is the plan.

@dcbw
Copy link
Member

dcbw commented May 19, 2016

LGTM too, for what it's worth...

@thockin
Copy link
Member

thockin commented May 23, 2016

Needs rebase :(

@freehan freehan force-pushed the kubenethostport branch from 4f693a3 to 7f62c0f Compare May 23, 2016 05:11
@k8s-github-robot k8s-github-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 23, 2016
@freehan freehan force-pushed the kubenethostport branch from 7f62c0f to 6a3ad1d Compare May 23, 2016 05:19
@freehan freehan added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 23, 2016
@freehan
Copy link
Contributor Author

freehan commented May 23, 2016

@k8s-bot test this issue: #25845

@thockin
Copy link
Member

thockin commented May 23, 2016

Still failing

@freehan
Copy link
Contributor Author

freehan commented May 23, 2016

@k8s-bot test this issue: #26087

@alex-mohr
Copy link
Contributor

Because submit queue is blocked, I'm going to kick off re-tests of the top 5 PRs in the queue, then merge them if they pass.
@k8s-bot test this please: issue #IGNORE

@freehan
Copy link
Contributor Author

freehan commented May 26, 2016

@k8s-bot test this issue: #IGNORE

Looks like a random failure to me

+ go get -u github.com/onsi/gomega
package gopkg.in/yaml.v2: unrecognized import path "gopkg.in/yaml.v2" (https fetch: Get https://gopkg.in/yaml.v2?go-get=1: net/http: TLS handshake timeout)
Build step 'Execute shell' marked build as failure

@k8s-bot
Copy link

k8s-bot commented May 26, 2016

GCE e2e build/test passed for commit 6a3ad1d.

@caesarxuchao caesarxuchao mentioned this pull request May 26, 2016
Closed
@alex-mohr alex-mohr merged commit aab6c43 into kubernetes:master May 26, 2016
@danwinship danwinship mentioned this pull request Aug 29, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@thockin thockin

Labels
lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Milestone
v1.3
Development

Successfully merging this pull request may close these issues.

Port forwarding should be through iptables
7 participants
@freehan @thockin @dcbw @alex-mohr @k8s-bot @googlebot @k8s-github-robot