Skip to content

Commit

Permalink
[3.6] sdn: only sync HostPorts when we need to (backport)
Browse files Browse the repository at this point in the history
Which is the first time a pod is started, when there will be active hostports, or when there are current active hostports. Otherwise the syncer runs iptables-restore for no good reason.

Backport of openshift#16692 to 3.6 branch
  • Loading branch information
dcbw authored and knobunc committed Oct 12, 2017
1 parent 4140fb8 commit 1d74735
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 10 deletions.
34 changes: 30 additions & 4 deletions pkg/sdn/plugin/pod.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,11 @@ type podManager struct {
mtu uint32
ovs *ovsController

// true if hostports have been synced at least once
hostportsSynced bool
// true if at least one running pod has a hostport mapping
activeHostports bool

// Things only accessed through the processCNIRequests() goroutine
// and thus can be set from Start()
ipamConfig []byte
Expand Down Expand Up @@ -151,12 +156,33 @@ func (m *podManager) getPod(request *cniserver.PodRequest) *kubehostport.PodPort
}

// Return a list of Kubernetes RunningPod objects for hostport operations
func (m *podManager) getRunningPods() []*kubehostport.PodPortMapping {
pods := make([]*kubehostport.PodPortMapping, 0)
func (m *podManager) shouldSyncHostports(newPod *kubehostport.PodPortMapping) []*kubehostport.PodPortMapping {
if m.hostportSyncer == nil {
return nil
}

newActiveHostports := false
mappings := make([]*kubehostport.PodPortMapping, 0)
for _, runningPod := range m.runningPods {
pods = append(pods, runningPod.podPortMapping)
mappings = append(mappings, runningPod.podPortMapping)
if !newActiveHostports && len(runningPod.podPortMapping.PortMappings) > 0 {
newActiveHostports = true
}
}
if newPod != nil && len(newPod.PortMappings) > 0 {
newActiveHostports = true
}
return pods

// Sync the first time a pod is started (to clear out stale mappings
// if kubelet crashed), or when there are any/will be active hostports.
// Otherwise don't bother.
if !m.hostportsSynced || m.activeHostports || newActiveHostports {
m.hostportsSynced = true
m.activeHostports = newActiveHostports
return mappings
}

return nil
}

// Add a request to the podManager CNI request queue
Expand Down
18 changes: 12 additions & 6 deletions pkg/sdn/plugin/pod_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -246,8 +246,10 @@ func (m *podManager) setup(req *cniserver.PodRequest) (*cnitypes.Result, *runnin
defer func() {
if !success {
m.ipamDel(req.SandboxID)
if err := m.hostportSyncer.SyncHostports(TUN, m.getRunningPods()); err != nil {
glog.Warningf("failed syncing hostports: %v", err)
if mappings := m.shouldSyncHostports(nil); mappings != nil {
if err := m.hostportSyncer.SyncHostports(TUN, mappings); err != nil {
glog.Warningf("failed syncing hostports: %v", err)
}
}
}
}()
Expand All @@ -258,8 +260,10 @@ func (m *podManager) setup(req *cniserver.PodRequest) (*cnitypes.Result, *runnin
return nil, nil, err
}
podPortMapping := hostport.ConstructPodPortMapping(&v1Pod, podIP)
if err := m.hostportSyncer.OpenPodHostportsAndSync(podPortMapping, TUN, m.getRunningPods()); err != nil {
return nil, nil, err
if mappings := m.shouldSyncHostports(podPortMapping); mappings != nil {
if err := m.hostportSyncer.OpenPodHostportsAndSync(podPortMapping, TUN, mappings); err != nil {
return nil, nil, err
}
}

var hostVethName, contVethMac string
Expand Down Expand Up @@ -362,8 +366,10 @@ func (m *podManager) teardown(req *cniserver.PodRequest) error {
errList = append(errList, err)
}

if err := m.hostportSyncer.SyncHostports(TUN, m.getRunningPods()); err != nil {
errList = append(errList, err)
if mappings := m.shouldSyncHostports(nil); mappings != nil {
if err := m.hostportSyncer.SyncHostports(TUN, mappings); err != nil {
errList = append(errList, err)
}
}

return kerrors.NewAggregate(errList)
Expand Down

0 comments on commit 1d74735

Please sign in to comment.