Skip to content

Commit

Permalink
fix breaking changes for cspc
Browse files Browse the repository at this point in the history 
Signed-off-by: shubham <shubham.bajpai@mayadata.io>
  • Loading branch information
@shubham14bajpai
shubham14bajpai committed Mar 9, 2020
1 parent 695a521 commit 84b450c
Show file tree
Hide file tree
Showing 3 changed files with 167 additions and 69 deletions.
44 changes: 21 additions & 23 deletions pkg/webhook/bd_replacement.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -146,20 +146,20 @@ func getCommonPoolSpecs(cspcNew, cspcOld *cstor.CStorPoolCluster, kubeClient kub
// validateRaidGroupChanges returns error when user removes or add block
// devices(for other than strip type) to existing raid group or else it will
// return nil
func validateRaidGroupChanges(oldRg, newRg *cstor.RaidGroup) error {
// // return error when block devices are removed from new raid group
// if len(newRg.BlockDevices) < len(oldRg.BlockDevices) {
// return errors.Errorf("removing block device from %s raid group is not valid operation",
// oldRg.Type)
// }
// // return error when block device are added to new raid group other than
// // stripe
// if cstor.PoolType(oldRg.Type) != cstor.PoolStriped &&
// len(newRg.BlockDevices) > len(oldRg.BlockDevices) {
// return errors.Errorf("adding block devices to existing %s raid group is "+
// "not valid operation",
// oldRg.Type)
// }
func validateRaidGroupChanges(oldRg, newRg *cstor.RaidGroup, oldRgType string) error {
// return error when block devices are removed from new raid group
if len(newRg.BlockDevices) < len(oldRg.BlockDevices) {
return errors.Errorf("removing block device from %s raid group is not valid operation",
oldRgType)
}
// return error when block device are added to new raid group other than
// stripe
if cstor.PoolType(oldRgType) != cstor.PoolStriped &&
len(newRg.BlockDevices) > len(oldRg.BlockDevices) {
return errors.Errorf("adding block devices to existing %s raid group is "+
"not valid operation",
oldRgType)
}
return nil
}

Expand All @@ -170,18 +170,16 @@ func (bdr *BlockDeviceReplacement) IsPoolSpecChangeValid(oldPoolSpec, newPoolSpe
for _, oldRg := range oldPoolSpec.DataRaidGroups {
oldRg := oldRg // pin it
isRaidGroupExist := false
// if oldRg.Type == "" {
// oldRg.Type = oldPoolSpec.PoolConfig.DefaultRaidGroupType
// }
oldRgType := oldPoolSpec.PoolConfig.DataRaidGroupType
for _, newRg := range newPoolSpec.DataRaidGroups {
newRg := newRg // pin it
if IsRaidGroupCommon(oldRg, newRg) {
isRaidGroupExist = true
if err := validateRaidGroupChanges(&oldRg, &newRg); err != nil {
if err := validateRaidGroupChanges(&oldRg, &newRg, oldRgType); err != nil {
return false, fmt.Sprintf("raid group validation failed: %v", err)
}
if IsBlockDeviceReplacementCase(&oldRg, &newRg) {
if ok, msg := bdr.IsBDReplacementValid(&newRg, &oldRg); !ok {
if ok, msg := bdr.IsBDReplacementValid(&newRg, &oldRg, oldRgType); !ok {
return false, msg
}
newBD := GetNewBDFromRaidGroups(&newRg, &oldRg)
Expand Down Expand Up @@ -245,11 +243,11 @@ func GetNumberOfDiskReplaced(newRG, oldRG *cstor.RaidGroup) int {
}

// IsBDReplacementValid validates for BD replacement.
func (bdr *BlockDeviceReplacement) IsBDReplacementValid(newRG, oldRG *cstor.RaidGroup) (bool, string) {
func (bdr *BlockDeviceReplacement) IsBDReplacementValid(newRG, oldRG *cstor.RaidGroup, oldRgType string) (bool, string) {

// if oldRG.Type == string(cstor.PoolStriped) {
// return false, "cannot replace blockdevice in stripe raid group"
// }
if oldRgType == string(cstor.PoolStriped) {
return false, "cannot replace blockdevice in stripe raid group"
}

// Not more than 1 bd should be replaced in a raid group.
if IsMoreThanOneDiskReplaced(newRG, oldRG) {
Expand Down
70 changes: 70 additions & 0 deletions pkg/webhook/bd_replacement_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -696,3 +696,73 @@ func TestBlockDeviceReplacement_IsNewBDPresentOnCurrentCSPC(t *testing.T) {
})
}
}

func TestValidateRaidGroupChanges(t *testing.T) {
tests := map[string]struct {
oldRG *cstor.RaidGroup
newRG *cstor.RaidGroup
oldRgType string
expectedError bool
}{
"removing block devices": {
oldRG: &cstor.RaidGroup{
BlockDevices: []cstor.CStorPoolClusterBlockDevice{
{BlockDeviceName: "bd-1"},
{BlockDeviceName: "bd-2"},
},
},
newRG: &cstor.RaidGroup{
BlockDevices: []cstor.CStorPoolClusterBlockDevice{
{BlockDeviceName: "bd-1"},
},
},
expectedError: true,
},
"adding block devices for raid groups": {
oldRG: &cstor.RaidGroup{
BlockDevices: []cstor.CStorPoolClusterBlockDevice{
{BlockDeviceName: "bd-1"},
{BlockDeviceName: "bd-2"},
},
},
newRG: &cstor.RaidGroup{
BlockDevices: []cstor.CStorPoolClusterBlockDevice{
{BlockDeviceName: "bd-1"},
{BlockDeviceName: "bd-2"},
{BlockDeviceName: "bd-3"},
},
},
oldRgType: "raidz",
expectedError: true,
},
"adding block devices for stripe raid groups": {
oldRG: &cstor.RaidGroup{
BlockDevices: []cstor.CStorPoolClusterBlockDevice{
{BlockDeviceName: "bd-1"},
{BlockDeviceName: "bd-2"},
},
},
newRG: &cstor.RaidGroup{
BlockDevices: []cstor.CStorPoolClusterBlockDevice{
{BlockDeviceName: "bd-1"},
{BlockDeviceName: "bd-2"},
{BlockDeviceName: "bd-3"},
},
},
oldRgType: "stripe",
expectedError: false,
},
}
for name, test := range tests {
name, test := name, test
t.Run(name, func(t *testing.T) {
err := validateRaidGroupChanges(test.oldRG, test.newRG, test.oldRgType)
if test.expectedError && err == nil {
t.Errorf("test %s failed expectedError to be error but got nil", name)
}
if !test.expectedError && err != nil {
t.Errorf("test %s failed expectedError not to be error but got error %v", name, err)
}
})
}
}
122 changes: 76 additions & 46 deletions pkg/webhook/cspc.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ type PoolValidator struct {
namespace string
nodeName string
cspcName string
clientset clientset.Interface
}

// Builder is the builder object for Builder
Expand Down Expand Up @@ -78,6 +79,13 @@ func (b *Builder) withPoolNamespace() *Builder {
return b
}

// withClientset sets the clientset field of poolValidator with provided
// values
func (b *Builder) withClientset(c clientset.Interface) *Builder {
b.object.clientset = c
return b
}

// withPoolNodeName sets the node name field of poolValidator with provided
// values
func (b *Builder) withPoolNodeName(nodeName string) *Builder {
Expand Down Expand Up @@ -175,7 +183,8 @@ func (wh *webhook) cspcValidation(cspc *cstor.CStorPoolCluster) (bool, string) {

buildPoolValidator := NewBuilder().
withPoolNamespace().
withCSPCName(cspc.Name)
withCSPCName(cspc.Name).
withClientset(wh.clientset)
for _, pool := range cspc.Spec.Pools {
pool := pool // pin it
nodeName, err := GetNodeFromLabelSelector(pool.NodeSelector, wh.kubeClient)
Expand Down Expand Up @@ -233,50 +242,71 @@ func (poolValidator *PoolValidator) poolSpecValidation() (bool, string) {
}
// TODO : Add validation for pool config
// Pool config will require mutating webhooks also.
// for _, raidGroup := range poolValidator.poolSpec.DataRaidGroups {
// raidGroup := raidGroup // pin it
// ok, msg := poolValidator.raidGroupValidation(&raidGroup)
// if !ok {
// return false, msg
// }
// }
ok, msg := poolValidator.poolConfigValidation(poolValidator.poolSpec.PoolConfig)
if !ok {
return false, msg
}
for _, raidGroup := range poolValidator.poolSpec.DataRaidGroups {
raidGroup := raidGroup // pin it
ok, msg := poolValidator.raidGroupValidation(&raidGroup, poolValidator.poolSpec.PoolConfig.DataRaidGroupType)
if !ok {
return false, msg
}
}

return true, ""
}

var (
// SupportedPRaidType is a map holding the supported raid configurations
// Value of the keys --
// 1. In case of striped this is the minimum number of disk required.
// 2. In all other cases this is the exact number of disks required.
SupportedPRaidType = map[cstor.PoolType]int{
cstor.PoolStriped: 1,
cstor.PoolMirrored: 2,
cstor.PoolRaidz: 3,
cstor.PoolRaidz2: 6,
}
)

func (poolValidator *PoolValidator) poolConfigValidation(
poolConfig cstor.PoolConfig) (bool, string) {
if poolConfig.DataRaidGroupType == "" {
return false, fmt.Sprintf("missing dataRaidGroupType")
}
if _, ok := SupportedPRaidType[cstor.PoolType(poolConfig.DataRaidGroupType)]; !ok {
return false, fmt.Sprintf("unsupported raid type '%s' specified", poolConfig.DataRaidGroupType)
}
return true, ""
}

// func (poolValidator *PoolValidator) raidGroupValidation(
// raidGroup *cstor.RaidGroup) (bool, string) {
// if raidGroup.Type == "" &&
// poolValidator.poolSpec.PoolConfig.DefaultRaidGroupType == "" {
// return false, fmt.Sprintf("any one type at raid group or default raid group type be specified ")
// }
// if _, ok := cstor.SupportedPRaidType[cstor.PoolType(raidGroup.Type)]; !ok {
// return false, fmt.Sprintf("unsupported raid type '%s' specified", cstor.PoolType(raidGroup.Type))
// }

// if len(raidGroup.BlockDevices) == 0 {
// return false, fmt.Sprintf("number of block devices honouring raid type should be specified")
// }

// if raidGroup.Type != string(cstor.PoolStriped) {
// if len(raidGroup.BlockDevices) != cstor.SupportedPRaidType[cstor.PoolType(raidGroup.Type)] {
// return false, fmt.Sprintf("number of block devices honouring raid type should be specified")
// }
// } else {
// if len(raidGroup.BlockDevices) < cstor.SupportedPRaidType[cstor.PoolType(raidGroup.Type)] {
// return false, fmt.Sprintf("number of block devices honouring raid type should be specified")
// }
// }

// for _, bd := range raidGroup.BlockDevices {
// bd := bd
// ok, msg := poolValidator.blockDeviceValidation(&bd)
// if !ok {
// return false, msg
// }
// }
// return true, ""
// }
func (poolValidator *PoolValidator) raidGroupValidation(
raidGroup *cstor.RaidGroup, rgType string) (bool, string) {

if len(raidGroup.BlockDevices) == 0 {
return false, fmt.Sprintf("number of block devices honouring raid type should be specified")
}

if rgType != string(cstor.PoolStriped) {
if len(raidGroup.BlockDevices) != SupportedPRaidType[cstor.PoolType(rgType)] {
return false, fmt.Sprintf("number of block devices honouring raid type should be specified")
}
} else {
if len(raidGroup.BlockDevices) < SupportedPRaidType[cstor.PoolType(rgType)] {
return false, fmt.Sprintf("number of block devices honouring raid type should be specified")
}
}

for _, bd := range raidGroup.BlockDevices {
bd := bd
ok, msg := poolValidator.blockDeviceValidation(&bd)
if !ok {
return false, msg
}
}
return true, ""
}

func validateBlockDevice(bd *openebsapis.BlockDevice, nodeName string) error {
if bd.Status.State != "Active" {
Expand All @@ -302,11 +332,11 @@ func validateBlockDevice(bd *openebsapis.BlockDevice, nodeName string) error {
// 1. block device name shouldn't be empty.
// 2. If block device has claim it verifies whether claim is created by this CSPC
func (poolValidator *PoolValidator) blockDeviceValidation(
bd *cstor.CStorPoolClusterBlockDevice, clientset clientset.Interface) (bool, string) {
bd *cstor.CStorPoolClusterBlockDevice) (bool, string) {
if bd.BlockDeviceName == "" {
return false, fmt.Sprint("block device name cannot be empty")
}
bdObj, err := clientset.OpenebsV1alpha1().BlockDevices(poolValidator.namespace).
bdObj, err := poolValidator.clientset.OpenebsV1alpha1().BlockDevices(poolValidator.namespace).
Get(bd.BlockDeviceName, metav1.GetOptions{})
if err != nil {
return false, fmt.Sprintf(
Expand All @@ -331,16 +361,16 @@ func (poolValidator *PoolValidator) blockDeviceValidation(
// TODO: Need to check how NDM
if bdObj.Spec.ClaimRef != nil {
bdcName := bdObj.Spec.ClaimRef.Name
if err := poolValidator.blockDeviceClaimValidation(bdcName, bdObj.Name, clientset); err != nil {
if err := poolValidator.blockDeviceClaimValidation(bdcName, bdObj.Name); err != nil {
return false, fmt.Sprintf("error: %v", err)
}
}
}
return true, ""
}

func (poolValidator *PoolValidator) blockDeviceClaimValidation(bdcName, bdName string, clientset clientset.Interface) error {
bdcObject, err := clientset.OpenebsV1alpha1().BlockDeviceClaims(poolValidator.namespace).
func (poolValidator *PoolValidator) blockDeviceClaimValidation(bdcName, bdName string) error {
bdcObject, err := poolValidator.clientset.OpenebsV1alpha1().BlockDeviceClaims(poolValidator.namespace).
Get(bdcName, metav1.GetOptions{})
if err != nil {
return errors.Wrapf(err,
Expand Down

0 comments on commit 84b450c

Please sign in to comment.