Skip to content
This repository has been archived by the owner on Jul 6, 2023. It is now read-only.

Commit

Permalink
V3: addressing comments from John
Browse files Browse the repository at this point in the history
Note for the reviewers about changes:

- Moved multiple size checks to Build()
- Change function name from InfoBlockVolumeFromHost() -> BlockVolumeInfoFromHost()
- Discarded expandBlockVolumeExec() and called the executor directly from Exec()
- Used IDs as arguments rather than *Entry types at NewBlockVolumeExpandOperation()
- Added check for pending BHV along with existing pending BV in Build()
- Fix go vet error

Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>
  • Loading branch information
Prasanna Kumar Kalever committed Mar 31, 2020
1 parent bac488d commit 466741e
Show file tree
Hide file tree
Showing 4 changed files with 62 additions and 66 deletions.
45 changes: 3 additions & 42 deletions apps/glusterfs/app_block_volume.go
Expand Up @@ -200,52 +200,13 @@ func (a *App) BlockVolumeExpand(w http.ResponseWriter, r *http.Request) {
return
}

var blockvolume *BlockVolumeEntry
err = a.db.View(func(tx *bolt.Tx) error {
var err error
blockvolume, err = NewBlockVolumeEntryFromId(tx, id)
if err == ErrNotFound {
http.Error(w, err.Error(), http.StatusNotFound)
return err
} else if err != nil {
http.Error(w, err.Error(), http.StatusInternalServerError)
return err
}

if msg.Size == blockvolume.Info.Size {
http.Error(w, "Requested new-size is same as current block volume size, nothing to be done.",
http.StatusBadRequest)
logger.LogError("Requested new-size %v is same as current block volume size %v, nothing to be done.",
msg.Size, blockvolume.Info.Size)
return ErrInvalidRequest
} else if msg.Size < blockvolume.Info.Size {
http.Error(w, "Requested new-size is less than current block volume size, shrinking is not allowed.",
http.StatusBadRequest)
logger.LogError("Requested new-size %v is less than current block volume size %v, shrinking is not allowed.",
msg.Size, blockvolume.Info.Size)
return ErrInvalidRequest
}

volume, err := NewVolumeEntryFromId(tx, blockvolume.Info.BlockHostingVolume)
if err != nil {
return err
}

RequiredFeeSize := msg.Size - blockvolume.Info.Size
if volume.Info.BlockInfo.FreeSize < RequiredFeeSize {
http.Error(w, "Free size on block hosting volume is less than requested delta size.",
http.StatusBadRequest)
logger.LogError("Free size %v on block hosting volume is less than requested delta size %v.",
volume.Info.BlockInfo.FreeSize, RequiredFeeSize)
return ErrNoSpace
}
return nil
})
bve, err := NewBlockVolumeExpandOperation(id, a.db, msg.Size)
if err != nil {
http.Error(w, err.Error(), http.StatusInternalServerError)
logger.LogError("failed creating a new BlockVolumeExpandOperation" + err.Error())
return
}

bve := NewBlockVolumeExpandOperation(blockvolume, a.db, msg.Size)
if err := AsyncHttpOperation(a, w, r, bve); err != nil {
OperationHttpErrorf(w, err, "Failed to allocate block volume expansion: %v", err)
return
Expand Down
15 changes: 1 addition & 14 deletions apps/glusterfs/block_volume_entry.go
Expand Up @@ -320,7 +320,7 @@ func (v *BlockVolumeEntry) destroyFromHost(
return nil
}

func (v *BlockVolumeEntry) InfoBlockVolumeFromHost(executor executors.Executor,
func (v *BlockVolumeEntry) BlockVolumeInfoFromHost(executor executors.Executor,
hvname string, h string) (*executors.BlockVolumeInfo, error) {

godbc.Require(hvname != "")
Expand Down Expand Up @@ -366,19 +366,6 @@ func (v *BlockVolumeEntry) expandBlockVolumeComponents(db wdb.DB, newSize int) e
return e
}

func (v *BlockVolumeEntry) expandBlockVolumeExec(executor executors.Executor,
hvname string, newSize int, h string) error {
err := executor.BlockVolumeExpand(h, hvname, v.Info.Name, newSize)
if _, ok := err.(*executors.VolumeDoesNotExistErr); ok {
logger.LogError("Block volume %v (%v) does not exist:", v.Info.Id, v.Info.Name)
return err
} else if err != nil {
logger.LogError("Unable to Expand volume: %v", err)
return err
}
return nil
}

func (v *BlockVolumeEntry) removeComponents(db wdb.DB, keepSize bool) error {
return db.Update(func(tx *bolt.Tx) error {
// Remove volume from cluster
Expand Down
64 changes: 56 additions & 8 deletions apps/glusterfs/operations_block_volume.go
Expand Up @@ -401,10 +401,23 @@ type BlockVolumeExpandOperation struct {
}

// NewBlockVolumeExpandOperation creates a new BlockVolumeExpandOperation populated
// with the given blockvolume entry, db connection and newsize (in GB) that the
// with the given blockvolume id, db connection and newsize (in GB) that the
// blockvolume is to be expanded by.
func NewBlockVolumeExpandOperation(
bvol *BlockVolumeEntry, db wdb.DB, newSize int) *BlockVolumeExpandOperation {
func NewBlockVolumeExpandOperation(bvolId string, db wdb.DB, newSize int) (
*BlockVolumeExpandOperation, error) {

var bvol *BlockVolumeEntry
err := db.View(func(tx *bolt.Tx) error {
var err error
bvol, err = NewBlockVolumeEntryFromId(tx, bvolId)
if err != nil {
return err
}
return nil
})
if err != nil {
return nil, err
}

return &BlockVolumeExpandOperation{
OperationManager: OperationManager{
Expand All @@ -413,7 +426,7 @@ func NewBlockVolumeExpandOperation(
},
bvol: bvol,
newSize: newSize,
}
}, nil
}

// loadBlockVolumeExpandOperation returns a BlockVolumeExpandOperation populated
Expand Down Expand Up @@ -451,16 +464,43 @@ func (bve *BlockVolumeExpandOperation) ResourceUrl() string {
func (bve *BlockVolumeExpandOperation) Build() error {
return bve.db.Update(func(tx *bolt.Tx) error {
txdb := wdb.WrapTx(tx)
v, err := NewBlockVolumeEntryFromId(tx, bve.bvol.Info.Id)
bv, err := NewBlockVolumeEntryFromId(tx, bve.bvol.Info.Id)
if err != nil {
return err
}
bve.bvol = v
bve.bvol = bv
if bve.bvol.Pending.Id != "" {
logger.LogError("Pending block volume %v can not be Expanded",
bve.bvol.Info.Id)
return ErrConflict
}

v, err := NewVolumeEntryFromId(tx, bv.Info.BlockHostingVolume)
if err != nil {
return err
}
if v.Pending.Id != "" {
logger.LogError("Pending volume %v can not expand block volume %v", v.Info.Id, bve.bvol.Info.Id)
return ErrConflict
}

if bve.newSize == bv.Info.Size {
logger.LogError("Requested new-size %v is same as current block volume size %v, nothing to be done.",
bve.newSize, bv.Info.Size)
return ErrInvalidRequest
} else if bve.newSize < bv.Info.Size {
logger.LogError("Requested new-size %v is less than current block volume size %v, shrinking is not allowed.",
bve.newSize, bv.Info.Size)
return ErrInvalidRequest
}

RequiredFeeSize := bve.newSize - bv.Info.Size
if v.Info.BlockInfo.FreeSize < RequiredFeeSize {
logger.LogError("Free size %v on block hosting volume is less than requested delta size %v.",
v.Info.BlockInfo.FreeSize, RequiredFeeSize)
return ErrNoSpace
}

err = bve.bvol.expandBlockVolumeComponents(txdb, bve.newSize)
if err != nil {
return err
Expand Down Expand Up @@ -509,7 +549,15 @@ func (bve *BlockVolumeExpandOperation) Exec(executor executors.Executor) error {
logger.Info("executing expand of block volume %v in op:%v",
bve.bvol.Info.Id, bve.op.Id)
return newTryOnHosts(bvHosts).once().run(func(h string) error {
return bv.expandBlockVolumeExec(executor, hvname, bve.newSize, h)
err := executor.BlockVolumeExpand(h, hvname, bv.Info.Name, bve.newSize)
if _, ok := err.(*executors.VolumeDoesNotExistErr); ok {
logger.LogError("Block volume %v (%v) does not exist:", bv.Info.Id, bv.Info.Name)
return err
} else if err != nil {
logger.LogError("Unable to Expand volume: %v", err)
return err
}
return nil
})
}

Expand Down Expand Up @@ -579,7 +627,7 @@ func (bve *BlockVolumeExpandOperation) Clean(executor executors.Executor) error
logger.Info("executing get info of block volume %v in op:%v",
bve.bvol.Info.Id, bve.op.Id)
err = newTryOnHosts(bvHosts).once().run(func(h string) error {
bvolInfo, err := bv.InfoBlockVolumeFromHost(executor, hvname, h)
bvolInfo, err := bv.BlockVolumeInfoFromHost(executor, hvname, h)
if err != nil {
return err
}
Expand Down
4 changes: 2 additions & 2 deletions executors/cmdexec/block_volume.go
Expand Up @@ -241,7 +241,7 @@ func (c *CmdExecutor) InfoBlockVolume(host string, blockhostingvolume string,

type CliOutput struct {
BlockName string `json:"NAME"`
blockHostingVolumeNameVolume string `json:"VOLUME"`
BlockHostingVolumeName string `json:"VOLUME"`
Gbid string `json:"GBID"`
Size string `json:"SIZE"`
Ha int `json:"HA"`
Expand Down Expand Up @@ -281,7 +281,7 @@ func (c *CmdExecutor) InfoBlockVolume(host string, blockhostingvolume string,

blockVolumeInfo.BlockHosts = blockVolumeInfoExec.Portal
blockVolumeInfo.GlusterNode = host
blockVolumeInfo.GlusterVolumeName = blockVolumeInfoExec.blockHostingVolumeNameVolume
blockVolumeInfo.GlusterVolumeName = blockVolumeInfoExec.BlockHostingVolumeName
blockVolumeInfo.Hacount = blockVolumeInfoExec.Ha
blockVolumeInfo.Iqn = "iqn.2016-12.org.gluster-block:" + blockVolumeInfoExec.Gbid
blockVolumeInfo.Name = blockVolumeInfoExec.BlockName
Expand Down

0 comments on commit 466741e

Please sign in to comment.