Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Failing to detect SSDs in copyDir should not be a fatal error. #3653

Merged
merged 6 commits into from
Oct 30, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 7 additions & 8 deletions internal/pkg/agent/application/upgrade/upgrade.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,6 @@ import (
"runtime"
"strings"

"github.com/jaypipes/ghw"

"github.com/otiai10/copy"
"go.elastic.co/apm"

Expand Down Expand Up @@ -386,13 +384,14 @@ func copyDir(l *logger.Logger, from, to string, ignoreErrs bool) error {
}
}

block, err := ghw.Block()
if err != nil {
return fmt.Errorf("ghw.Block() returned error: %w", err)
}

// Try to detect if we are running with SSDs. If we are increase the copy concurrency,
// otherwise fall back to the default.
copyConcurrency := 1
if install.HasAllSSDs(*block) {
hasSSDs, detectHWErr := install.HasAllSSDs()
if detectHWErr != nil {
l.Infow("Could not determine block storage type, disabling copy concurrency", "error.message", detectHWErr)
}
if hasSSDs {
copyConcurrency = runtime.NumCPU() * 4
}

Expand Down
2 changes: 1 addition & 1 deletion internal/pkg/agent/cmd/install.go
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ func installCmd(streams *cli.IOStreams, cmd *cobra.Command) error {

cfgFile := paths.ConfigFile()
if status != install.PackageInstall {
err = install.Install(cfgFile, topPath, progBar)
err = install.Install(cfgFile, topPath, progBar, streams)
if err != nil {
return fmt.Errorf("error installing package: %w", err)
}
Expand Down
42 changes: 30 additions & 12 deletions internal/pkg/agent/install/install.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,22 +11,22 @@ import (
"runtime"
"strings"

"github.com/kardianos/service"

"github.com/jaypipes/ghw"
"github.com/kardianos/service"
"github.com/otiai10/copy"
"github.com/schollz/progressbar/v3"

"github.com/elastic/elastic-agent/internal/pkg/agent/application/paths"
"github.com/elastic/elastic-agent/internal/pkg/agent/errors"
"github.com/elastic/elastic-agent/internal/pkg/cli"
)

const (
darwin = "darwin"
)

// Install installs Elastic Agent persistently on the system including creating and starting its service.
func Install(cfgFile, topPath string, pt *progressbar.ProgressBar) error {
func Install(cfgFile, topPath string, pt *progressbar.ProgressBar, streams *cli.IOStreams) error {
dir, err := findDirectory()
if err != nil {
return errors.New(err, "failed to discover the source directory for installation", errors.TypeFilesystem)
Expand Down Expand Up @@ -62,15 +62,18 @@ func Install(cfgFile, topPath string, pt *progressbar.ProgressBar) error {
}

// copy source into install path
block, err := ghw.Block()
if err != nil {
return fmt.Errorf("ghw.Block() returned error: %w", err)
}

//
// Try to detect if we are running with SSDs. If we are increase the copy concurrency,
// otherwise fall back to the default.
copyConcurrency := 1
if HasAllSSDs(*block) {
hasSSDs, detectHWErr := HasAllSSDs()
if detectHWErr != nil {
fmt.Fprintf(streams.Out, "Could not determine block hardware type, disabling copy concurrency: %s\n", detectHWErr)
}
if hasSSDs {
copyConcurrency = runtime.NumCPU() * 4
}

pt.Describe("Copying install files")
err = copy.Copy(dir, topPath, copy.Options{
OnSymlink: func(_ string) copy.SymlinkAction {
Expand All @@ -84,7 +87,8 @@ func Install(cfgFile, topPath string, pt *progressbar.ProgressBar) error {
return errors.New(
err,
fmt.Sprintf("failed to copy source directory (%s) to destination (%s)", dir, topPath),
errors.M("source", dir), errors.M("destination", topPath))
errors.M("source", dir), errors.M("destination", topPath),
)
}
pt.Describe("Successfully copied files")

Expand Down Expand Up @@ -263,8 +267,22 @@ func verifyDirectory(dir string) error {
}

// HasAllSSDs returns true if the host we are on uses SSDs for
// all its persistent storage; false otherwise or on error
func HasAllSSDs(block ghw.BlockInfo) bool {
// all its persistent storage; false otherwise. Returns any error
// encountered detecting the hardware type for informational purposes.
// Errors from this function are not fatal. Note that errors may be
// returned on some Mac hardware configurations as the ghw package
// does not fully support MacOS.
func HasAllSSDs() (bool, error) {
block, err := ghw.Block()
if err != nil {
return false, err
}

return hasAllSSDs(*block), nil
}

// Internal version of HasAllSSDs for testing.
func hasAllSSDs(block ghw.BlockInfo) bool {
for _, disk := range block.Disks {
switch disk.DriveType {
case ghw.DRIVE_TYPE_FDD, ghw.DRIVE_TYPE_ODD:
Expand Down
2 changes: 1 addition & 1 deletion internal/pkg/agent/install/install_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ func TestHasAllSSDs(t *testing.T) {

for name, test := range cases {
t.Run(name, func(t *testing.T) {
actual := HasAllSSDs(test.block)
actual := hasAllSSDs(test.block)
require.Equal(t, test.expected, actual)
})
}
Expand Down
Loading