launchpad-reviewers team mailing list archive

[Ines Almeida <mp+455573@xxxxxxxxxxxxxxxxxx>]

Ines Almeida has proposed merging ~ines-almeida/txpkgupload:update-txpkgupload-dtpport-selection into txpkgupload:master.

Commit message:
Add portsInUse set in txpkgupload code so that passive FTP doesn't reuse connected ports

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~ines-almeida/txpkgupload/+git/txpkgupload/+merge/455573

Currently some dput uploads are failing in production. We suspect that txpkgupload is not selecting the right ports for users to use.

Possible considerations:
 - Would it be possible for a port to not be cleaned up after closing, hence limiting the port space we currently have?
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~ines-almeida/txpkgupload:update-txpkgupload-dtpport-selection into txpkgupload:master.

diff --git a/src/txpkgupload/twistedftp.py b/src/txpkgupload/twistedftp.py
index bfc195b..55b6cea 100644
--- a/src/txpkgupload/twistedftp.py
+++ b/src/txpkgupload/twistedftp.py
@@ -177,6 +177,7 @@ class FTPWithEPSV(ftp.FTP):
 
     epsvAll = False
     supportedNetworkProtocols = (_AFNUM_IP, _AFNUM_IP6)
+    portsInUse = set()
 
     def connectionLost(self, reason):
         ftp.FTP.connectionLost(self, reason)
@@ -188,12 +189,15 @@ class FTPWithEPSV(ftp.FTP):
         attribute.
         """
         for portn in self.passivePortRange:
+            if portn in self.portsInUse:
+                continue
             try:
                 dtpPort = self.listenFactory(portn, factory,
                                              interface=interface)
             except error.CannotListenError:
                 continue
             else:
+                self.portsInUse.add(portn)
                 return dtpPort
         raise error.CannotListenError('', portn,
             "No port available in range %s" %
@@ -320,6 +324,17 @@ class FTPWithEPSV(ftp.FTP):
         self.reply(ftp.ENTERING_EPSV_MODE, port)
         return self.dtpFactory.deferred.addCallback(lambda ign: None)
 
+    def cleanupDTP(self):
+        """
+        Overrides `cleanupDTP` to update the list of ports in use once DTPport
+        is closed.
+        """
+        port = self.dtpPort.getHost().port
+        super().cleanupDTP()
+
+        if port in self.portsInUse:
+            self.portsInUse.remove(port)
+
     def ftp_PORT(self):
         if self.epsvAll:
             return defer.fail(ftp.BadCmdSequenceError(