lightos driver: stop global urllib3 warning suppression - #29
Open
yuval-lb wants to merge 1 commit into
Open
Conversation
The driver called urllib3.disable_warnings() at import scope. With no warning category argument it suppressed every urllib3 warning for the whole cinder-volume process, so a TLS warning raised by any other backend sharing that process was silently dropped. Running at import time it also took effect in deployments that have no Lightbits backend configured, and it overrode the operator's choice of suppress_requests_ssl_warnings. Remove the call and the urllib3 import it was the only user of. Operators who want these warnings suppressed can set suppress_requests_ssl_warnings, which cinder-volume already applies process-wide for exactly this purpose. Deployments running the driver with driver_ssl_cert_verify = False will see InsecureRequestWarning again; an upgrade release note covers this and points at both suppress_requests_ssl_warnings and the better fix of enabling certificate verification. No unit test accompanies this change: it removes a module-level side effect and adds no branch or function to cover, and asserting on global warning-filter state would be testing the interpreter rather than the driver. Generated-By: Claude Opus 5 (Claude Code) Change-Id: I08d2c85b947b30600239eadaaeefb7f7c4da2f2e Signed-off-by: Yuval Brave <yuval@lightbitslabs.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR description
The driver called
urllib3.disable_warnings()at import scope. With no warningcategory argument it suppressed every urllib3 warning for the whole
cinder-volume process, so a TLS warning raised by any other backend sharing that
process was silently dropped. Running at import time it also took effect in
deployments that have no Lightbits backend configured, and it overrode the
operator's choice of
suppress_requests_ssl_warnings.This removes the call and the
urllib3import it was the only user of.Operators who want these warnings suppressed can set
suppress_requests_ssl_warnings, which cinder-volume already appliesprocess-wide for exactly this purpose (
cinder/volume/manager.py:305).Deployments running with
driver_ssl_cert_verify = Falsewill seeInsecureRequestWarningagain. The release note covers this and points at boththat option and the better fix of enabling certificate verification.
How was the PR tested?
Verified on a live 3-node cluster (kolla, Lightbits 3.21.1), by patching the
driver in place and restarting cinder-volume:
Driver imports and initializes with the import removed — container healthy,
no tracebacks or import errors.
Behaviour measured against a baseline:
InsecureRequestWarningcount was 0before the change and 2 after, so the change restores the warning rather than
being a silent no-op.
Frequency: the warning is not deduplicated — it scales with driver
activity. Measured on an idle backend and then across real operations:
That is roughly 3 warnings per volume create and 6 per attach, with no drift
while idle, and all of them naming the same API endpoint — so the usual
per-message dedup does not apply here. An operator doing bulk volume work on
a backend with
driver_ssl_cert_verify = Falseshould expect a proportionalnumber of log lines, and set
suppress_requests_ssl_warningsif that isunwanted. The release note calls this out.
Not verified: that
suppress_requests_ssl_warnings = Truesuppresses thiswarning in practice.
cinder/volume/manager.py:305-309disablesInsecureRequestWarningexplicitly and runs before the driver isinstantiated, so it is correct by construction — but it was not measured.
No unit test is added: the change removes a module-level side effect and adds no
branch or function to cover, and asserting on global warning-filter state would
test the interpreter rather than the driver.
PR dependencies
Jira Ticket
Issue: LBM1-47383