Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

nautilus: mgr/telemetry: fix device id splitting when anonymizing serial #37318

Merged
merged 1 commit into from Sep 24, 2020

Conversation

yaarith
Copy link
Contributor

@yaarith yaarith commented Sep 22, 2020

Anonymizing the serial number in the device id string fails in rare
cases where 'vendor' and 'model' are missing from the device id
string. Ideally, device id is generated (in blkdev.cc) as
'vendor_model_serial', in case all fields were successfully retrieved
from the device. In cases where they were not, device id can also be
generated as 'model_serial' or 'serial'. Splitting by '_' fails in the
latter case (since 'serial' is the only element in the string).

In order to anonymize serial numbers in smartctl reports we now rely
on the serial number value as retrieved from the raw smartctl report
itself (as opposed to the one in device id). That's in order to avoid
possible inconsistencies between the serial retrieved from device id and
the one in the report.

Fixes: https://tracker.ceph.com/issues/46977
Signed-off-by: Yaarit Hatuka <yaarit@redhat.com>
(cherry picked from commit e5099a7)

Conflicts:
	src/pybind/mgr/telemetry/module.py

In master we use Python 3's f-string formatting to create 'anon_devid':
	anon_devid = f"{devid.rsplit('_', 1)[0]}_{uuid.uuid1()}"

The conflict happened since Nautilus still uses Python 2, and 'anon_id'
is created via string concatenation.
	anon_devid = devid[:devid.rfind('_')] + '_' + str(uuid.uuid1())
@yaarith yaarith added this to the nautilus milestone Sep 22, 2020
@yaarith
Copy link
Contributor Author

yaarith commented Sep 22, 2020

Conflicts:
src/pybind/mgr/telemetry/module.py

In master we use Python 3's f-string formatting to create anon_devid:
anon_devid = f"{devid.rsplit('_', 1)[0]}_{uuid.uuid1()}"

The conflict happened since Nautilus still uses Python 2, and anon_id is created via string concatenation.
anon_devid = devid[:devid.rfind('_')] + '_' + str(uuid.uuid1())

(added to the commit message)

Copy link
Contributor

@bk201 bk201 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

Test logs:

@smithfarm
Copy link
Contributor

jenkins test api

@smithfarm smithfarm removed the nautilus-batch-1 nautilus point releases label Sep 23, 2020
@smithfarm
Copy link
Contributor

@yaarith Please don't add nautilus-batch-1 until you have ensured that the PR passed all checks.

@smithfarm smithfarm added nautilus-batch-1 nautilus point releases needs-qa labels Sep 23, 2020
@yaarith
Copy link
Contributor Author

yaarith commented Sep 23, 2020

Sorry about that, @smithfarm, and thanks for the feedback.

@yuriw
Copy link
Contributor

yuriw commented Sep 23, 2020

@yuriw yuriw merged commit 42af857 into ceph:nautilus Sep 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants