Include more clean up to the storage type conversion #811
Conversation
muhamadazmy
requested review from
LeeSmet,
delandtj,
maxux and
zaibon
as code owners
zaibon
left a comment
zaibon
left a comment
There was a problem hiding this comment.
I think this is a good step to the right direction. Added a couple of remarks to it.
pkg/flist/flist.go
Outdated
|
|
||
| if len(opts.Type) == 0 { | ||
| // sanity check in case type is not set always use hdd | ||
| opts.Type = pkg.HDDDevice |
There was a problem hiding this comment.
I would return an error instead. At this point we should have the information, not having it is mostly sign of a bad request from the user.
There was a problem hiding this comment.
actually this is an internal call. the user does not have a direct access to flist.Mount.
There was a problem hiding this comment.
But in that case, since this is optional (for read/write flists) I should move this check when it's needed.
pkg/provision/primitives/zdb.go
Outdated
| rootFS, err := flist.Mount(zdbFlistURL, "", pkg.MountOptions{ | ||
| Limit: 10, | ||
| ReadOnly: false, | ||
| Type: pkg.HDDDevice, |
There was a problem hiding this comment.
Do you remember why we didn't set this to readonly ? Feel like this is the way to go so we don't have to allocate the subvolume for the 0-db other then the one for the actual data.
There was a problem hiding this comment.
I remember we had issues when the rootfs of zdb was readonly, but that was all what I remember. BTW this line is the actual cause of the issue.
There was a problem hiding this comment.
Then I think we need to have more logic. Cause now if there is no HDD, then we cannot deploy 0-db at all.
So I think we should try to deploy on any type of disks
There was a problem hiding this comment.
I agree, but this is not related to this issue. We need to create a new issue (also to retry read-only rootfs) but this now cause minio setup on devent to fail.
pkg/provision/primitives/zdb.go
Outdated
| rootFS, err := flist.Mount(zdbFlistURL, "", pkg.MountOptions{ | ||
| Limit: 10, | ||
| ReadOnly: false, | ||
| Type: pkg.HDDDevice, |
There was a problem hiding this comment.
Then I think we need to have more logic. Cause now if there is no HDD, then we cannot deploy 0-db at all.
So I think we should try to deploy on any type of disks
| Limit: 10, | ||
| ReadOnly: false, | ||
| }) | ||
| var err error |
No description provided.