Skip to content

Commit

Permalink
Merge pull request #17032 from danwinship/synchostports-small
Browse files Browse the repository at this point in the history
Automatic merge from submit-queue (batch tested with PRs 17032, 17027).

Fix the check for "pod has HostPorts"

#16692 didn't work because `kubehostport.PodPortMapping.PortMappings` includes *all* ports that a pod declares, not just hostports, so `shouldSyncHostports` will return true if any pod declares that it listens on any port. (ie, always)

This fixes that, which should make the code work in the way #16692 intended for it to work ("only call SyncHostPorts if there is at least one pod on the node that uses HostPorts"). It doesn't attempt to change it to "only call SyncHostPorts if the set of active HostPorts actually changed" because that would be larger and less suitable as a last-minute fix.

@dcbw FYI
  • Loading branch information
openshift-merge-robot authored Oct 25, 2017
2 parents d29dd8e + d9b3efb commit 958e3fa
Showing 1 changed file with 11 additions and 2 deletions.
13 changes: 11 additions & 2 deletions pkg/network/node/pod.go
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,15 @@ func (m *podManager) getPod(request *cniserver.PodRequest) *kubehostport.PodPort
return nil
}

func hasHostPorts(pod *kubehostport.PodPortMapping) bool {
for _, mapping := range pod.PortMappings {
if mapping.HostPort != 0 {
return true
}
}
return false
}

// Return a list of Kubernetes RunningPod objects for hostport operations
func (m *podManager) shouldSyncHostports(newPod *kubehostport.PodPortMapping) []*kubehostport.PodPortMapping {
if m.hostportSyncer == nil {
Expand All @@ -201,11 +210,11 @@ func (m *podManager) shouldSyncHostports(newPod *kubehostport.PodPortMapping) []
mappings := make([]*kubehostport.PodPortMapping, 0)
for _, runningPod := range m.runningPods {
mappings = append(mappings, runningPod.podPortMapping)
if !newActiveHostports && len(runningPod.podPortMapping.PortMappings) > 0 {
if !newActiveHostports && hasHostPorts(runningPod.podPortMapping) {
newActiveHostports = true
}
}
if newPod != nil && len(newPod.PortMappings) > 0 {
if newPod != nil && hasHostPorts(newPod) {
newActiveHostports = true
}

Expand Down

0 comments on commit 958e3fa

Please sign in to comment.