Skip to content

[MINOR] First shot of clean-archi for smartarray PR. - #10

Merged
vdaviot merged 7 commits into
mainfrom
pouet
Mar 10, 2025
Merged

[MINOR] First shot of clean-archi for smartarray PR.#10
vdaviot merged 7 commits into
mainfrom
pouet

Conversation

@vdaviot

@vdaviot vdaviot commented Mar 7, 2025

Copy link
Copy Markdown
Contributor

No description provided.

vdaviot added 3 commits March 7, 2025 14:29
Signed-off-by: Valentin Daviot <valentin.daviot@scality.com>
…sts not working for now.

Signed-off-by: Valentin Daviot <valentin.daviot@scality.com>
Signed-off-by: Valentin Daviot <valentin.daviot@scality.com>
Comment thread pkg/implementation/hardwareraidcontroller/smartarray.go
Comment thread pkg/implementation/blinker/ssacli.go Outdated
Comment thread pkg/utils/formatter.go Outdated
Comment thread pkg/implementation/controllergetter/ssacli.go Outdated
Comment thread pkg/implementation/controllergetter/ssacli.go Outdated
Comment on lines +290 to +313
func (s *SSACLI) getBlockDevice(devicePath string) (*BlockDevice, error) {
output, err := s.lsblk.Run([]string{
devicePath,
"--paths",
"--bytes",
"--nodeps",
"--output",
"name,rota,size,type,tran,mountpoint,fstype,parttype",
})
if err != nil {
return nil, errors.Wrap(err, "failed to get block device using lsblk")
}

blockDevices, err := ParseLSBLKOutput(output)
if err != nil {
return nil, errors.Wrap(err, "failed to parse lsblk command output")
}

if len(blockDevices) <= 0 {
return nil, errors.Errorf("block device not found: %s", devicePath)
}

return &blockDevices[0], nil
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To me ideally we should use the same function as for rhel8

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMHO this should come in the next PR, because it will require a new package (something like drivestatusgetter or something)

I have two things to address following the merge:

  • That ☝️
  • As we're now using empty methods returning errors, SoftwareRaidController doesn't make sense, so I need to harmonize the declarations

Which I'd like to do in a separate PR

Comment thread pkg/implementation/physicaldrivegetter/ssacli.go Outdated
Comment on lines +345 to +354
// isBlockDeviceUsed checks if a block device is used.
// If the device is mounted or has a filesystem type, it is considered used.
// Otherwise, it is considered unassigned good.
func isBlockDeviceUsed(device *BlockDevice) physicaldrive.PDStatus {
if device.MountPoint != "" || device.FilesystemType != "" || device.PartitionType != "" {
return physicaldrive.PDStatusUsed
}

return physicaldrive.PDStatusUnassignedGood
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto we should mutualise it with rhel8

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added to todolist from above message

Comment thread pkg/implementation/physicaldrivegetter/ssacli.go Outdated
Comment thread pkg/implementation/physicaldrivegetter/ssacli.go Outdated
vdaviot added 3 commits March 10, 2025 11:26
Signed-off-by: Valentin Daviot <valentin.daviot@scality.com>
Signed-off-by: Valentin Daviot <valentin.daviot@scality.com>
Signed-off-by: Valentin Daviot <valentin.daviot@scality.com>
Comment thread pkg/implementation/hardwareraidcontroller/megaraid/logicalvolume.go Outdated
Comment thread pkg/implementation/hardwareraidcontroller/megaraid/megaraid_test.go Outdated
Comment thread pkg/domain/entities/physicaldrive/types.go Outdated
Comment thread pkg/domain/entities/physicaldrive/types.go Outdated
Comment thread pkg/implementation/physicaldrivegetter/ssacli.go Outdated
Signed-off-by: Valentin Daviot <valentin.daviot@scality.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants