Merge ~ines-almeida/txpkgupload:update-txpkgupload-dtpport-selection into txpkgupload:master

[Ines Almeida]

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

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?

[Colin Watson (cjwatson)]

There's definitely a non-zero risk of something like that, but I don't know how to assess how likely it is, and this seems worth a try. I guess we can at least check logs occasionally to see whether the data ports we're issuing seem to be growing without bound?

review: Approve

[Ines Almeida (ines-almeida)]

Sounds like a plan, we'll have to monitor that... I'll do some more testing in qastaging before deploying to production

[Otto Co-Pilot (otto-copilot)]

Running landing tests failed
https://jenkins.ols.ps5.canonical.com/job/txpkgupload/143/

[Otto Co-Pilot (otto-copilot)]

Running landing tests failed
https://jenkins.ols.ps5.canonical.com/job/txpkgupload/144/

69c165f... by Ines Almeida on 2023-11-15

Use actual dtpPort instead of assumed `portn` when getting passive FTP port

The dtpPort port number should be created from `portn`, but this change removes the need for that assumption altogether.

[Colin Watson (cjwatson)]

review: Approve

diff --git a/src/txpkgupload/twistedftp.py b/src/txpkgupload/twistedftp.py
index bfc195b..a652553 100644
--- a/src/txpkgupload/twistedftp.py
+++ b/src/txpkgupload/twistedftp.py
@@ -177,23 +177,38 @@ class FTPWithEPSV(ftp.FTP):
 
     epsvAll = False
     supportedNetworkProtocols = (_AFNUM_IP, _AFNUM_IP6)
+    portsInUse = set()
 
     def connectionLost(self, reason):
         ftp.FTP.connectionLost(self, reason)
         self.epsvAll = False
 
+    def cleanupDTP(self):
+        """
+        Overrides `cleanupDTP` to update the list of ports in use once DTPport
+        is closed.
+        """
+        port = self.dtpPort.getHost().port if self.dtpPort else None
+        super().cleanupDTP()
+
+        if port and port in self.portsInUse:
+            self.portsInUse.remove(port)
+
     def getDTPPort(self, factory, interface=''):
         """
         Return a port for passive access, using C{self.passivePortRange}
         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(dtpPort.getHost().port)
                 return dtpPort
         raise error.CannotListenError('', portn,
             "No port available in range %s" %