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

add configuration to disable listening on ipv6 #371

Merged
merged 1 commit into from
Mar 8, 2017

Conversation

gianrubio
Copy link
Contributor

Fix #368

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Mar 3, 2017
@k8s-reviewable
Copy link

This change is Reviewable

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.008%) to 45.37% when pulling 812ff19 on gianrubio:disable-ipv6 into 75124bc on kubernetes:master.

@aledbf
Copy link
Member

aledbf commented Mar 8, 2017

@gianrubio please rebase

@databus23
Copy link
Contributor

I can confirm that this fixes the nginx ingress controllers on our systems where ipv6 is not enabled.

@gianrubio gianrubio force-pushed the disable-ipv6 branch 3 times, most recently from 66fc4ff to 48f28bd Compare March 8, 2017 12:24
@gianrubio
Copy link
Contributor Author

@aledbf done!

@@ -384,7 +384,7 @@ http {
# Use the port 18080 (random value just to avoid known ports) as default port for nginx.
# Changing this value requires a change in:
# https://github.com/kubernetes/contrib/blob/master/ingress/controllers/nginx/nginx/command.go#L104
listen [::]:18080 ipv6only=off default_server reuseport backlog={{ .BacklogSize }};
listen {{ if not $cfg.DisableIpv6 }}[::]:{{ end }}18080 ipv6only=off default_server reuseport backlog={{ .BacklogSize }};
Copy link
Member

Choose a reason for hiding this comment

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

missing {{ if not $cfg.DisableIpv6 }}ipv6only=off{{ end }}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

@aledbf
Copy link
Member

aledbf commented Mar 8, 2017

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 8, 2017
@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) to 46.297% when pulling 63b5f2f on gianrubio:disable-ipv6 into f1062e0 on kubernetes:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) to 46.297% when pulling 63b5f2f on gianrubio:disable-ipv6 into f1062e0 on kubernetes:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) to 46.297% when pulling 63b5f2f on gianrubio:disable-ipv6 into f1062e0 on kubernetes:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) to 46.297% when pulling 63b5f2f on gianrubio:disable-ipv6 into f1062e0 on kubernetes:master.

@aledbf aledbf merged commit fedf342 into kubernetes:master Mar 8, 2017
@aledbf
Copy link
Member

aledbf commented Mar 8, 2017

@gianrubio thanks!

@databus23
Copy link
Contributor

Thanks!

@PRAJINPRAKASH
Copy link

how achive this from helm chart

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants