This repository has been archived by the owner on May 6, 2020. It is now read-only.
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.
Functionality to setup disk specific RAIDs #48
Functionality to setup disk specific RAIDs #48
Changes from all commits
c9920ef
bc66b11
cc4e674
2e8706c
82e8019
ab23803
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This
sort
can be deleted and has no effect anyway because the keys are wrong. If you want to sort by something, then uselsblk -lnpd - x size
orlsblk -lnpd -x rota
(and add| tac
to reverse the ordering).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TIL:
tac
😄There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now that I think about it, is there really a need to sort here? Do we need sorted disk order to form RAID?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think your suggestion should be added in https://github.com/kinvolk/lokomotive-kubernetes/blob/85732d1a934ddcd83fa8ea0a566f1ab3f89d0f66/packet/flatcar-linux/kubernetes/workers/cl/install.yaml.tmpl#L54-L58
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would be good to have predictable RAID devices here, so sorting is a good idea. Otherwise for every server, you will have a different disk layout. It's easier to spot potential inconsistencies/issues when the setup is predictable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should change all that in a follow up PR, this PR has already been prolonged for so long on very minuscule issues.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is no need to sort it I would say. The sorting was just done to find the smallest disk for installation - that works even if it's more complicated as it needs to be. Anyway, the
sort
invocation here is a no-op and should be deleted.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the record:
lsblk -lnpd -o name,rota | sort -h -k 4,4 --debug
complains that it cannot match the key (sorting byrota
makes no sense anyway?).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah it has to be sorted on the basis of size.
The diff would look something like this: