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 support for SourceIP preservation in Azure LBs #36557

Merged
merged 1 commit into from
Nov 10, 2016
Merged

Add support for SourceIP preservation in Azure LBs #36557

merged 1 commit into from
Nov 10, 2016
+78 −14

Conversation

brendandburns
Copy link
Contributor

@brendandburns brendandburns commented Nov 10, 2016 •
edited by k8s-oncall
Loading

@thockin @colemickens now that we are setting DSR for Azure LBs this should "just work"

This change is Reviewable

@brendandburns
Copy link
Contributor Author

ugh, nm, bad push, hang on...

@k8s-github-robot k8s-github-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. release-note-label-needed labels Nov 10, 2016
@brendandburns
Copy link
Contributor Author

Ok, this is now ready for review...

@k8s-ci-robot
Copy link
Contributor

Jenkins verification failed for commit 1be44555df71129287fa3b31687ac5fdca92c518. Full PR test history.

The magic incantation to run this job again is @k8s-bot verify test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

colemickens
colemickens approved these changes Nov 10, 2016
View reviewed changes
Copy link
Contributor

@colemickens colemickens left a comment

Choose a reason for hiding this comment

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

LGTM, one small question/note.

pkg/cloudprovider/providers/azure/azure_loadbalancer.go
Name: &lbRuleName,
Properties: &network.ProbePropertiesFormat{
RequestPath: to.StringPtr(podPresencePath),
Protocol: network.ProbeProtocolHTTP,
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there any assumptions about this probe port/path? Particularly, is the contract for this say "the pod presence probe must be HTTP, not HTTPS"? I only ask because I know our probes don't support HTTPS.

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 think it's HTTP for now, so I think we're ok...

@brendandburns brendandburns added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-label-needed labels Nov 10, 2016
@colemickens
Copy link
Contributor

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 10, 2016
@brendandburns brendandburns added this to the v1.5 milestone Nov 10, 2016
@k8s-github-robot
Copy link

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

@k8s-github-robot
Copy link

Automatic merge from submit-queue

@k8s-github-robot k8s-github-robot merged commit 620854b into kubernetes:master Nov 10, 2016
@colemickens colemickens mentioned this pull request Dec 2, 2016
Merged
@chentao1596 chentao1596 mentioned this pull request Dec 5, 2016
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@colemickens colemickens colemickens approved these changes

Assignees

@colemickens colemickens

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/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Milestone
v1.5
Development

Successfully merging this pull request may close these issues.
5 participants
@brendandburns @k8s-ci-robot @colemickens @k8s-github-robot @googlebot