call for comments -> "ceph-disk" making OSD directories on typos and is inconsistent (useability).

call for comments -> "ceph-disk" making OSD directories on typos and is inconsistent (useability).

[Owen Synge]

Dear all,

By default "ceph-disk" will do the following:

# ceph-disk -vvvv prepare  --fs-type xfs --cluster ceph -- /dev/sdk
DEBUG:ceph-disk:Preparing osd data dir /dev/sdk

No block device "/dev/sdk" exists so "ceph-disk" decides a block device
is not wanted and makes a directory for an OSD.

I think as policy "ceph-disk" should not assume by default, that a non
existent target is correct, make a directory for the "disk" type OSD to
reside in, and set it up as a "disk" type OSD.

Hence I proposed this patch:

https://github.com/ceph/ceph/pull/2160

As a second best option I would be happier with an explicit "don’t fail
if target is not present and just make a directory at the target". But
then you get on to the question of deeper directory structures being
handled:

The current behavior with deeper directory structures is currently
inconsistent as this output shows:

# ceph-disk prepare --fs-type xfs --cluster ceph -- /mnt/vdu/vdu
Traceback (most recent call last):
  File "/usr/sbin/ceph-disk", line 2605, in <module>
    main()
  File "/usr/sbin/ceph-disk", line 2583, in main
    args.func(args)
  File "/usr/sbin/ceph-disk", line 1311, in main_prepare
    os.mkdir(args.data)
OSError: [Errno 2] No such file or directory: '/mnt/vdu/vdu'

I think as a third best option would be to only make directories the
"--data-dir" parameter is used, but still suffers the deeper directory
structures question.

I am still unsure if I like the idea of creating directories for deeper
directory structures, as again the potential for typos leading to vastly
different directory paths with a single misplaced character, and for
consistency would rather "ceph-disk" just failed if the target is not
available.

Although I propose failing fast and clearly with a clear error message
when a target does not exist, removing the assumption that all non
existent targets are valid and "disk" based OSD's and to try and make an
"appropriate" directory, I do see 2 issues with this change:

(A) This is a change to the current default behavior so effecting
deployment frameworks.
(B) This would effect "ceph-deploy" which under some circumstances uses
this behavior.

I propose the following patch to mitigate side effect (B).

https://github.com/ceph/ceph-deploy/pull/224

I see no way to resolve issue (A) in general if my proposal for change
is selected.

I have discussed this issue with "alfredodeza" on IRC both privately and
later on the "ceph-devel" IRC channel and he is "really divided here"
hence we decided I would bring this up for discussion on this mailing list.

Best regards

Owen
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: call for comments -> "ceph-disk" making OSD directories on typos and is inconsistent (useability).

[Sage Weil]

On Fri, 1 Aug 2014, Owen Synge wrote:
> Dear all,
> 
> By default "ceph-disk" will do the following:
> 
> # ceph-disk -vvvv prepare  --fs-type xfs --cluster ceph -- /dev/sdk
> DEBUG:ceph-disk:Preparing osd data dir /dev/sdk
> 
> No block device "/dev/sdk" exists so "ceph-disk" decides a block device
> is not wanted and makes a directory for an OSD.
> 
> I think as policy "ceph-disk" should not assume by default, that a non
> existent target is correct, make a directory for the "disk" type OSD to
> reside in, and set it up as a "disk" type OSD.
> 
> Hence I proposed this patch:
> 
> https://github.com/ceph/ceph/pull/2160

In a vacuum, this seems like the cleanest approach...
 
> As a second best option I would be happier with an explicit "don?t fail
> if target is not present and just make a directory at the target". But
> then you get on to the question of deeper directory structures being
> handled:
> 
> The current behavior with deeper directory structures is currently
> inconsistent as this output shows:
> 
> # ceph-disk prepare --fs-type xfs --cluster ceph -- /mnt/vdu/vdu
> Traceback (most recent call last):
>   File "/usr/sbin/ceph-disk", line 2605, in <module>
>     main()
>   File "/usr/sbin/ceph-disk", line 2583, in main
>     args.func(args)
>   File "/usr/sbin/ceph-disk", line 1311, in main_prepare
>     os.mkdir(args.data)
> OSError: [Errno 2] No such file or directory: '/mnt/vdu/vdu'
> 
> I think as a third best option would be to only make directories the
> "--data-dir" parameter is used, but still suffers the deeper directory
> structures question.
> 
> I am still unsure if I like the idea of creating directories for deeper
> directory structures, as again the potential for typos leading to vastly
> different directory paths with a single misplaced character, and for
> consistency would rather "ceph-disk" just failed if the target is not
> available.
> 
> Although I propose failing fast and clearly with a clear error message
> when a target does not exist, removing the assumption that all non
> existent targets are valid and "disk" based OSD's and to try and make an
> "appropriate" directory, I do see 2 issues with this change:
> 
> (A) This is a change to the current default behavior so effecting
> deployment frameworks.
> (B) This would effect "ceph-deploy" which under some circumstances uses
> this behavior.
> 
> I propose the following patch to mitigate side effect (B).
> 
> https://github.com/ceph/ceph-deploy/pull/224

Is the problem that ceph-deploy will let you specify a directory /foo/bar 
and create bar for you?

Given that I don't think the directory use case is a common as the 
device one, perhaps simply requiring that that directory already exist 
(user can do mkdir -p $dir) will get us to the cleanest point.  I think 
one of the more common use cases for this is when the user has their own 
mount they want to set up, in which case it'll be an existing 
directory (mount point).

The other main use case that comes to mind is the 'quickly deploy in a 
bunch of dirs for testing', and in that case we can simply add mkdir to 
the instructions...?

sage

> 
> I see no way to resolve issue (A) in general if my proposal for change
> is selected.
> 
> I have discussed this issue with "alfredodeza" on IRC both privately and
> later on the "ceph-devel" IRC channel and he is "really divided here"
> hence we decided I would bring this up for discussion on this mailing list.
> 
> Best regards
> 
> Owen
> --
> To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 

Re: call for comments -> "ceph-disk" making OSD directories on typos and is inconsistent (useability).

[Owen Synge]

On 08/01/2014 05:10 PM, Sage Weil wrote:
> On Fri, 1 Aug 2014, Owen Synge wrote:
>> Dear all,
>>
>> By default "ceph-disk" will do the following:
>>
>> # ceph-disk -vvvv prepare  --fs-type xfs --cluster ceph -- /dev/sdk
>> DEBUG:ceph-disk:Preparing osd data dir /dev/sdk
>>
>> No block device "/dev/sdk" exists so "ceph-disk" decides a block device
>> is not wanted and makes a directory for an OSD.
>>
>> I think as policy "ceph-disk" should not assume by default, that a non
>> existent target is correct, make a directory for the "disk" type OSD to
>> reside in, and set it up as a "disk" type OSD.
>>
>> Hence I proposed this patch:
>>
>> https://github.com/ceph/ceph/pull/2160
> 
> In a vacuum, this seems like the cleanest approach...

I agree, and think for major releases going forward this should be how
ceph-disk works.

For maintenance releases for firefly and before, should we follow Loic
Dachary's suggestion of only refusing to make a data dir if it starts
with "/dev"?

I am happy to write an additional patch to support this mode of
operation for maintenance releases, even though I think the idea of
making directory targets if the target does not exist is not a good idea
even for backward compatibility.

>> As a second best option I would be happier with an explicit "don?t fail
>> if target is not present and just make a directory at the target". But
>> then you get on to the question of deeper directory structures being
>> handled:
>>
>> The current behavior with deeper directory structures is currently
>> inconsistent as this output shows:
>>
>> # ceph-disk prepare --fs-type xfs --cluster ceph -- /mnt/vdu/vdu
>> Traceback (most recent call last):
>>   File "/usr/sbin/ceph-disk", line 2605, in <module>
>>     main()
>>   File "/usr/sbin/ceph-disk", line 2583, in main
>>     args.func(args)
>>   File "/usr/sbin/ceph-disk", line 1311, in main_prepare
>>     os.mkdir(args.data)
>> OSError: [Errno 2] No such file or directory: '/mnt/vdu/vdu'
>>
>> I think as a third best option would be to only make directories the
>> "--data-dir" parameter is used, but still suffers the deeper directory
>> structures question.
>>
>> I am still unsure if I like the idea of creating directories for deeper
>> directory structures, as again the potential for typos leading to vastly
>> different directory paths with a single misplaced character, and for
>> consistency would rather "ceph-disk" just failed if the target is not
>> available.
>>
>> Although I propose failing fast and clearly with a clear error message
>> when a target does not exist, removing the assumption that all non
>> existent targets are valid and "disk" based OSD's and to try and make an
>> "appropriate" directory, I do see 2 issues with this change:
>>
>> (A) This is a change to the current default behavior so effecting
>> deployment frameworks.
>> (B) This would effect "ceph-deploy" which under some circumstances uses
>> this behavior.
>>
>> I propose the following patch to mitigate side effect (B).
>>
>> https://github.com/ceph/ceph-deploy/pull/224
> 
> Is the problem that ceph-deploy will let you specify a directory /foo/bar 

Yes it will.

> and create bar for you?

No it wont, only ceph-disk does this.

If ceph-disk fails when the target directory /foo/bar is missing, then
ceph-deploy is not backwards compatible.

Since this is only desirable (if it is desirable to make a target
directory) then it is only desirable if the target is a target directory
and not a block device.

Since the original error prompting this discovery was a block device eg
"/dev/sdk" that was thought to exist but did not, I enclosed the rather
big assumption that ceph-deploy would assume all files under "/dev" are
block devices.

> Given that I don't think the directory use case is a common as the 
> device one, perhaps simply requiring that that directory already exist 
> (user can do mkdir -p $dir) will get us to the cleanest point.  I think 
> one of the more common use cases for this is when the user has their own 
> mount they want to set up, in which case it'll be an existing 
> directory (mount point).

I agree.

> The other main use case that comes to mind is the 'quickly deploy in a 
> bunch of dirs for testing', and in that case we can simply add mkdir to 
> the instructions...?

I agree.

Based on your assumptions:

https://github.com/ceph/ceph-deploy/pull/224

should be refused if:

https://github.com/ceph/ceph/pull/2160

is accepted.

Only if it is desired to keep the functionality of making directories
for backwards compatibility would you want to keep "224".

I am not in favor of making directory targets if the target does not
exist, but if this is desired functionality, accepting "224" is better
than the current situation for ceph-deploy users. After seeing Loic
Dachary's suggestion of "ceph-disk" only refusing to make a data dir if
it starts with "/dev" it seems a more universal solution than "224" but
does continue the assumption that you want to make directory targets if
the target does not exist.

Since I am not in favor of making directory targets if the target does
not exist, accepting "2160" and refusing "224" would be my preference
going forward.

Best regards

Owen




> 
>>
>> I see no way to resolve issue (A) in general if my proposal for change
>> is selected.
>>
>> I have discussed this issue with "alfredodeza" on IRC both privately and
>> later on the "ceph-devel" IRC channel and he is "really divided here"
>> hence we decided I would bring this up for discussion on this mailing list.
>>
>> Best regards
>>
>> Owen
>> --
>> To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>>
> --
> To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>