Pulp

Assuming this theory is true, I wonder if it's possible to have urrlib3's connection pool behave such that it does not attempt to automatically re-establish a dropped connection, but rather waits for another request that wants to use that connection.

Okay, so I've done more digging. I filed a report upstream[0] which I recommend anyone who wants to understand the problem read. Sadly, it was a known issue[1] I just missed because I was looking for CA-related problems. It turns out we've got more than one problem:

1. Using one Session for downloads that require more than one client certificate across them (quite likely in the streamer) will not work, no matter what we do to Nectar.
2. Using one Session across Nectar downloads is not currently possible because of the way Nectar configures the Session.

Here are some potential solutions I've thought of:

1. Drop connection pooling on the streamer and continue to use Nectar. This may or may not result in frequent download failures with the streamer, and it will certainly hurt its performance in a very, very big way.
2. Modify Nectar in some way to avoid writing the certificates and keys to temporary files repeatedly. I don't think Nectar should be responsible for this task though, and getting it done reasonably in the platform may or may not be easy.
3. Stop using Nectar in the streamer and do either 3a or 3b.
3a. configure requests directly and manage where we put the certs and keys so that we don't write and delete them for every request AND also make sure we clear the connection pool any time a new client cert/key gets used. We'll get some connection pooling, but not as much.
3b. Use the library provided by twisted instead of Nectar or requests.
4. Change how CA certificates and client certificates/keys are stored in Pulp, and until the issue is addressed upstream create and manage the lifecycle of a pool of Session objects in the streamer.

[0] https://github.com/kennethreitz/requests/issues/3058
[1] https://github.com/kennethreitz/requests/issues/2863

It sounds like no matter what we do, we have to start storing ssl certs (both CAs and client certs) on the filesystem in a permanent location, so it can be referenced and re-referenced at any time on the filesystem by urllib3.

There is the additional problem that urllib3 does not account for different ssl settings (certs included) when managing its connection pool. Here is a potential short-term solution that could be useful until the problem gets fixed in urllib3. (This is number 4 on the list above.)

We could manage our own pool of session objects, and create a unique session for each unique collection of ssl settings. It would go something like this:

1. Create a namedtuple that contains a field for each ssl setting that we manage.
2. Create a dictionary where keys are instances of the above namedtuple, and values are a Session object
3. When making a request, create a namedtuple with the current ssl settings, use that to find-or-create a Session object, and then use it.
4. Something would need to keep track of how long it's been since a particular session was used, and evict old ones from the pool. That might require some cleanup, but perhaps it is sufficient to just let normal garbage collection happen.

I've mulled on this a bit more and I've thought of another thing we need to consider when managing our own pool. What happens when a client asks for a big file? I'm talking several gigabytes or more. How long do we keep Sessions in the pool? An hour without new requests? A day? I can't think of a good way to figure out if there is an active download in the Session so we risk killing long-running downloads.

I'm not saying there's not a solution to that problem, but it's a problem.

Good point. The way I'm hoping we can address that is by not killing the session when we remove it from the pool. The key question is:

When you're done using a session, do you have to kill it or otherwise do cleanup? Or can you just let it fall out of scope, and let normal garbage collection deal with it? The latter is the far more likely case, by virtue of being pythonic.

If that's correct, then we can just remove a session from the pool, and any existing requests using it will finish up without a problem.

In theory we could end up with a small number of redundant Session objects this way, but I don't think it's enough that it would be likely to cause problems.

I see that it has a close() method, but nothing says it has to be called, and the examples don't call it. We should get confirmation of 1) do we have to call close(), and 2) what happens if it just falls out of scope when it's not being used anymore, perhaps by asking them.

Well, not to be too pedantic, but nothing says you need to close files or anything else, but I don't think that's a good reason to not do it. They've implemented the `__enter__` and `__exit__` methods, so it's safe to assume (I think) that users should either use `with` or call `close`.

Luckily, I've taken a look at how urllib3 cleans up and it looks like it lets connections that are in use finish up before closing shop, so we can probably safely call `close` on a session, let it drop out of scope, and carry on.

That sounds good. If there are any questions remaining about whether it is safe to remove a session from a pool, do some cleanup, and let existing requests finish gracefully, let's get in touch with upstream and work with them to get firm answers. Hopefully we won't need to make any assumptions.

Should we s/mysteriously/miserably/ on the subject?

This PR isn't done so I'm not marking it POST yet, but it lives here:

https://github.com/pulp/pulp/pull/2499

mhrivnak wrote:

I see that it has a close() method, but nothing says it has to be called, and the examples don't call it. We should get confirmation of 1) do we have to call close(), and 2) what happens if it just falls out of scope when it's not being used anymore, perhaps by asking them.

Looks like nectar is just letting sessions fall out of scope and not closing the session explicitly. Since no connection leaks have been observed, I think it's safe to assume that the session pooling approach can do the same.

I suspect you have an old version of the patch. The merged PR has 3 values (the proper amount) instead of the 2 in your traceback: https://github.com/pulp/pulp/pull/2499/files#diff-0401aafbc062d63f501d2648d64ae307L211

Ok, got passed the above error, thanks for the pointer.

Now I'm getting:

pulp.streamer.server:ERROR: (25128-63488) An unexpected error occurred while handling the request.
pulp.streamer.server:ERROR: (25128-63488) Traceback (most recent call last):
pulp.streamer.server:ERROR: (25128-63488)   File "/usr/lib/python2.7/site-packages/pulp/streamer/server.py", line 184, in _handle_get
pulp.streamer.server:ERROR: (25128-63488)     self._download(catalog_entry, request, responder)
pulp.streamer.server:ERROR: (25128-63488)   File "/usr/lib/python2.7/site-packages/pulp/streamer/server.py", line 215, in _download
pulp.streamer.server:ERROR: (25128-63488)     db_importer, catalog_entry.url, working_dir='/tmp')
pulp.streamer.server:ERROR: (25128-63488)   File "/usr/lib/python2.7/site-packages/pulp/plugins/importer.py", line 82, in get_downloader_for_db_importer
pulp.streamer.server:ERROR: (25128-63488)     nectar_config = importer_to_nectar_config(importer, working_dir=working_dir)
pulp.streamer.server:ERROR: (25128-63488)   File "/usr/lib/python2.7/site-packages/pulp/plugins/util/nectar_config.py", line 66, in importer_to_nectar_config
pulp.streamer.server:ERROR: (25128-63488)     return importer_config_to_nectar_config(config, working_dir, download_config_kwargs)
pulp.streamer.server:ERROR: (25128-63488)   File "/usr/lib/python2.7/site-packages/pulp/plugins/util/nectar_config.py", line 97, in importer_config_to_nectar_config
pulp.streamer.server:ERROR: (25128-63488)     download_config = DownloaderConfig(**download_config_kwargs)
pulp.streamer.server:ERROR: (25128-63488)   File "/usr/lib/python2.7/site-packages/nectar/config.py", line 136, in __init__
pulp.streamer.server:ERROR: (25128-63488)     self._process_ssl_settings()
pulp.streamer.server:ERROR: (25128-63488)   File "/usr/lib/python2.7/site-packages/nectar/config.py", line 165, in _process_ssl_settings
pulp.streamer.server:ERROR: (25128-63488)     raise AttributeError('Cannot read file: %%s' %% file_arg_value)
pulp.streamer.server:ERROR: (25128-63488) AttributeError: Cannot read file: /var/lib/pulp/importers/mmccune-org-Red_Hat_Enterprise_Linux_Server-Red_Hat_Enterprise_Linux_7_Server_RPMs_x86_64_7Server-yum_importer/pki/ca.crt
[-] 127.0.0.1 - - [31/Mar/2016:03:38:22 +0000] "GET /var/lib/pulp/content/units/rpm/a5/6a0f4a22ca8b911e4d0185683aee7d1e75f9a85ad0231eccbd5f845a2ef6d0/yum-utils-1.1.31-34.el7.noarch.rpm HTTP/1.1" 500 - "-" "urlgrabber/3.10 yum/3.4.3"

is there some sort of migration I'm missing to get these ca.crt files on disk?

a bit more info, I setup new repositories after applying the new code, this time it creates the files:

# ls -Z /var/lib/pulp/importers/neworg-3-Red_Hat_Enterprise_Linux_Server-Red_Hat_Satellite_Tools_6_1_for_RHEL_7_Server_RPMs_x86_64-yum_importer/pki/
-rw-------. apache apache system_u:object_r:httpd_sys_rw_content_t:s0 ca.crt
-rw-------. apache apache system_u:object_r:httpd_sys_rw_content_t:s0 client.crt
-rw-------. apache apache system_u:object_r:httpd_sys_rw_content_t:s0 client.key

but still get the same error as above:

 AttributeError: Cannot read file: /var/lib/pulp/importers/neworg-3-Red_Hat_Enterprise_Linux_Server-Red_Hat_Satellite_Tools_6_1_for_RHEL_7_Server_RPMs_x86_64-yum_importer/pki/ca.crt

after re-restarting httpd and pulp_streamer as well as setting up new repositories I was able to lazy fetch content with this new routine without error.

that said, I'd imagine we need some sort of migration to populate the /pki dir for existing repos?

I suppose it couldn't hurt to have a migration. The files are created when the importer is saved (and cleaned up when the importer is deleted) so even if you turned an existing repository into an on-demand one I believe the files will get created when that change is saved. The only case I can think of is people out there who already have lazy repositories (some community users, maybe) and don't change any configuration after upgrading from 2.8.z to 2.8.2

Randy,
Could you also add detailed steps on how to verify this.
And point me to any pulp-smash test you might have written for this.

Thanks

Preethi,

I just followed the steps that Jeremy listed in the first comment. I ended up creating and on-demand syncing the rhel-6-server repository. Once that was done, I looked in MongoDB for the storage path of an RPM. I ended up using 389-ds-base-libs, and requested it from the streamer:

curl -O http://127.0.0.1:8751/var/lib/pulp/content/units/rpm/66/0e7bf17e1bd386dcd61759fd2822cb01ab396b13d69f33a3e6b5d67ce29ff6/389-ds-base-libs-1.2.10.2-15.el6.i686.rpm

Then I waited a while (tea time!) and came back and did the same thing. Before the fix it should fail because it can't read the certificates/key, but with the fix it should still work.

verified

[root@ibm-x3550m3-09 ~]# curl -O http://127.0.0.1:8751/var/lib/pulp/content/units/rpm/66/0e7bf17e1bd386dcd61759fd2822cb01ab396b13d69f33a3e6b5d67ce29ff6/389-ds-base-libs-1.2.10.2-15.el6.i686.rpm
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100  377k  100  377k    0     0   363k      0  0:00:01  0:00:01 --:--:--  364k
[root@ibm-x3550m3-09 ~]# 
[root@ibm-x3550m3-09 ~]# 
[root@ibm-x3550m3-09 ~]# curl -O http://127.0.0.1:8751/var/lib/pulp/content/units/rpm/66/0e7bf17e1bd386dcd61759fd2822cb01ab396b13d69f33a3e6b5d67ce29ff6/389-ds-base-libs-1.2.10.2-15.el6.i686.rpm
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100  377k  100  377k    0     0   257k      0  0:00:01  0:00:01 --:--:--  257k
[root@ibm-x3550m3-09 ~]# 
[root@ibm-x3550m3-09 ~]# 
[root@ibm-x3550m3-09 ~]# curl -O http://127.0.0.1:8751/var/lib/pulp/content/units/rpm/66/0e7bf17e1bd386dcd61759fd2822cb01ab396b13d69f33a3e6b5d67ce29ff6/389-ds-base-libs-1.2.10.2-15.el6.i686.rpm
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100  377k  100  377k    0     0   571k      0 --:--:-- --:--:-- --:--:--  571k
[root@ibm-x3550m3-09 ~]# 
[root@ibm-x3550m3-09 ~]# 
[root@ibm-x3550m3-09 ~]# curl -O http://127.0.0.1:8751/var/lib/pulp/content/units/rpm/66/0e7bf17e1bd386dcd61759fd2822cb01ab396b13d69f33a3e6b5d67ce29ff6/389-ds-base-libs-1.2.10.2-15.el6.i686.rpm
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100  377k  100  377k    0     0   302k      0  0:00:01  0:00:01 --:--:--  303k