feat(webhook): add cspc expansion validations #46
Conversation
Signed-off-by: shubham <[email protected]>
| newRaidGroups: []cstor.RaidGroup{}, | ||
| } | ||
| // build raidGroups by identifying common raidGroups in old and new | ||
| for _, oldRg := range oldPoolSpec.DataRaidGroups { |
There was a problem hiding this comment.
We need to get raid groups of WriteCacheRaidGroups
lly in other places also we need to check the repetition of blockdevices in writeCacheRaidGroup in getDuplicateBlockDeviceList function.
Signed-off-by: shubham <[email protected]>
pkg/webhook/cspc_operations.go
Outdated
| for _, newRg := range newPoolSpec.WriteCacheRaidGroups { | ||
| if IsRaidGroupCommon(oldRg, newRg) { | ||
| isRaidGroupExist = true | ||
| rgs["data"].oldRaidGroups = append(rgs["data"].oldRaidGroups, oldRg) |
There was a problem hiding this comment.
Key should be writeCache in other places also
| // 2.2 Validate changes for blockdevice replacement scenarios(openebs/openebs#2846). | ||
| // 3. Validate vertical pool expansions if there are any new raidgroups or blockdevices added. | ||
| func (pOps *PoolOperations) ArePoolSpecChangesValid(oldPoolSpec, newPoolSpec *cstor.PoolSpec) (bool, string) { | ||
| newToOldBd := make(map[string]string) |
There was a problem hiding this comment.
We need to add one check for raidGroupType also
if oldPoolSpec.PoolConfig.DataRaidGroupType != newPoolSpec.PoolConfig.DataRaidGroupType || oldPoolSpec.PoolConfig.WriteCacheRaidGroupType != newPoolSpec.PoolConfig.WriteCacheRaidGroupType {
return false, fmt.Sprintf("raidgroup can't be modified")
}There was a problem hiding this comment.
@shubham14bajpai sorry for bad reviews above check should be updated because there might be chances that user can add writecache raidgroup after pool creation at that time we need to allow expansion
if oldPoolSpec.PoolConfig.DataRaidGroupType != newPoolSpec.PoolConfig.DataRaidGroupType || (oldPoolSpec.PoolConfig.WriteCacheRaidGroupType != "" && oldPoolSpec.PoolConfig.WriteCacheRaidGroupType != newPoolSpec.PoolConfig.WriteCacheRaidGroupType) {
return false, fmt.Sprintf("raidgroup can't be modified")
}| _, ok := bdcObject.GetAnnotations()[types.PredecessorBDLabelKey] | ||
| if ok { | ||
| return true, errors.Errorf("replacement is still in progress for bd %s", v.BlockDeviceName) | ||
| if bdcObject != nil { |
There was a problem hiding this comment.
This can be improved by returning a typed error -- like BDC does not exists.
And BDC does not exist -- should be done instead.
Philosophy is -- if err is nil -- object should be not nil -- code looks more cleaner.
Commenting for tracking. It can be enhanced in a new PR.
There was a problem hiding this comment.
Anyways -- thanks. Nice Catch.
| // in stripe only block devices are added). Following are the validations: | ||
| // 1. New blockdevice shouldn't be claimed by any other CSPC (or) third party. | ||
| // 2. New blockdevice shouldn't be the replacing blockdevice. | ||
| func (pOps *PoolOperations) validatePoolExpansion( |
There was a problem hiding this comment.
func (pOps *PoolOperations) validatePoolExpansion(
newPoolSpec *cstor.PoolSpec, commonRaidGroups map[string]*raidGroups) {
var bds []string
for _, rgs := range commonRaidGroups {
if rgs.rgType == "stripe" {
bds = append(bds, getNewBDsFromStripeSpec(rgs.oldRaidGroups[0],
rgs.newRaidGroups[0]))
} else {
newRgs := getExpandedRaidGroups(newPoolSpec, rgs.oldRaidGroups)
bds = getBDsFromRaidGroups(newRgs)
}
}
err := pOps.validateNewBDs(bds, pOps.NewCSPC)
if err != nil {
return err
}
return nil
}Check will above fit into it by considering caller code comment also
Signed-off-by: shubham <[email protected]>
| // 2.2 Validate changes for blockdevice replacement scenarios(openebs/openebs#2846). | ||
| // 3. Validate vertical pool expansions if there are any new raidgroups or blockdevices added. | ||
| func (pOps *PoolOperations) ArePoolSpecChangesValid(oldPoolSpec, newPoolSpec *cstor.PoolSpec) (bool, string) { | ||
| newToOldBd := make(map[string]string) |
There was a problem hiding this comment.
@shubham14bajpai sorry for bad reviews above check should be updated because there might be chances that user can add writecache raidgroup after pool creation at that time we need to allow expansion
if oldPoolSpec.PoolConfig.DataRaidGroupType != newPoolSpec.PoolConfig.DataRaidGroupType || (oldPoolSpec.PoolConfig.WriteCacheRaidGroupType != "" && oldPoolSpec.PoolConfig.WriteCacheRaidGroupType != newPoolSpec.PoolConfig.WriteCacheRaidGroupType) {
return false, fmt.Sprintf("raidgroup can't be modified")
}Signed-off-by: shubham <[email protected]>
Signed-off-by: shubham [email protected]
This PR adds webhook validations for cspc expansion.
Refer: openebs-archive/maya#1588
TODO:
add unit test for new functions