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

ceph-volume: do not use --key during mkfs #19260

Closed
wants to merge 2 commits into from

Conversation

liewegas
Copy link
Member

@liewegas liewegas commented Nov 30, 2017

Also, don't print the raw key to the log.

Signed-off-by: Sage Weil <sage@redhat.com>
base_command.extend(['--key', keyring])
import tempfile
temp = tempfile.NamedTemporaryFile()
os.chmod(f.name, 0600)
Copy link
Member

Choose a reason for hiding this comment

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

what is f here? are you meaning temp.name?

@@ -205,7 +205,12 @@ def osd_mkfs_bluestore(osd_id, fsid, keyring=None, wal=False, db=False):
]

if keyring is not None:
base_command.extend(['--key', keyring])
import tempfile
Copy link
Member

Choose a reason for hiding this comment

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

imports at top?

@alfredodeza
Copy link
Contributor

@liewegas can you push this to ceph-ci so that we can trigger our functional tests?

@@ -221,7 +226,7 @@ def osd_mkfs_bluestore(osd_id, fsid, keyring=None, wal=False, db=False):

command = base_command + supplementary_command

process.run(command, obfuscate='--key')
process.run(command, obfuscate='--keyfile')
Copy link
Contributor

Choose a reason for hiding this comment

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

if the temp file is never closed it will not be removed. Are we OK with the file being around? If we want to remove it we would need to check if temp was created, and if so, then close it after process.run here

We do not want the key to show up on the command line (it may appear in
the process list or sudo log file).

Fixes: http://tracker.ceph.com/issues/22283
Signed-off-by: Sage Weil <sage@redhat.com>
@liewegas
Copy link
Member Author

liewegas commented Nov 30, 2017 via email

@alfredodeza
Copy link
Contributor

jenkins test ceph-volume lvm xenial-bluestore-create

@alfredodeza
Copy link
Contributor

jenkins test ceph-volume lvm centos7-bluestore-create

@alfredodeza
Copy link
Contributor

@liewegas valid failures from the bluestore side of things:

 stderr: 2017-11-30 22:42:34.572 7f14730cd540 -1 write_meta failed to read keyfile
/tmp/tmpgje08I: can't open /tmp/tmpgje08I: (13) Permission denied: (13) Permission denied

From:
https://jenkins.ceph.com/job/ceph-volume-prs-lvm-xenial-bluestore-create/4/console

@tchaikov
Copy link
Contributor

tchaikov commented Dec 1, 2017

@liewegas not sure why the uid changes after ceph-volume launches ceph-osd using subprocess.Popen(). but i posted #19276 to workaround this issue based on your change.

@liewegas liewegas closed this Dec 1, 2017
@liewegas liewegas deleted the wip-volume-key branch December 1, 2017 13:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants