Use complete port configs when plumbing mark rules#1432
Merged
Conversation
Currently, a reference counting scheme is used to reference count all individual port configs that need to be plumbed in the ingress to make sure that in situations where a service with the same set of port configs is getting added or removed doesn't accidentally remove the port config plumbing if the add/remove notifications come out of order. This same reference counting scheme is also used for plumbing the port-based marking rules. But marking rules should not be plumbed based on that because marks are always different for different instantiations of the same service. So fixed the code to plumb port-based mark rules based on the complete set of port configs, while plumbing pure port rules and proxies based on a filter set of port configs based on the reference count. Signed-off-by: Jana Radhakrishnan <mrjana@docker.com>
aboch
reviewed
Sep 21, 2016
| } | ||
|
|
||
| func writePortsToFile(ports []*PortConfig) (string, error) { | ||
| f, err := ioutil.TempFile("", "port_configs") |
Contributor
There was a problem hiding this comment.
Are we removing these files somewhere, or are they just accumulating in the tmp directory ?
Can the file creation frequency be that high that we should worry about removing them ?
Contributor
Author
There was a problem hiding this comment.
We are not removing them but that has been the case forever since this code was there. We can fix it here though.
With port redirect in the ingress path happening before ipvs in the ingess sandbox, there is a chance of 5-tuple collision in the ipvs connection table for two entirely different services have different PublishedPorts but the same TargetPort. To disambiguate the ipvs connection table, delay the port redirect from PublishedPort to TargetPort until after the loadbalancing has happened in ipvs. To be specific, perform the redirect after the packet enters the real backend container namespace. Signed-off-by: Jana Radhakrishnan <mrjana@docker.com>
aboch
reviewed
Sep 21, 2016
|
|
||
| ingressPortsFile = f.Name() | ||
| f.Close() | ||
| defer os.Remove(ingressPortsFile) |
aboch
reviewed
Sep 21, 2016
| if err != nil { | ||
| return err | ||
| } | ||
| defer os.Remove(ingressPortsFile) |
Contributor
|
LGTM |
1 similar comment
Contributor
|
LGTM |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Currently, a reference counting scheme is used to reference count all
individual port configs that need to be plumbed in the ingress to make
sure that in situations where a service with the same set of port
configs is getting added or removed doesn't accidentally remove the port
config plumbing if the add/remove notifications come out of order. This
same reference counting scheme is also used for plumbing the port-based
marking rules. But marking rules should not be plumbed based on that
because marks are always different for different instantiations of the
same service. So fixed the code to plumb port-based mark rules based on
the complete set of port configs, while plumbing pure port rules and
proxies based on a filter set of port configs based on the reference
count.
Signed-off-by: Jana Radhakrishnan mrjana@docker.com