From d9a8fcef25d61e417b3298feb9ccc002e90d6b0b Mon Sep 17 00:00:00 2001 From: Stefan Kurek Date: Thu, 28 Jul 2022 18:15:00 -0400 Subject: [PATCH] Updater properly installs and rollbacks JMX Jar. Also making sure that we use the backed up file permissions when rolling back a file that no longer exists in the install directory --- updater/internal/action/file_action.go | 9 +- updater/internal/action/file_action_test.go | 20 +- updater/internal/file/file.go | 35 +++- updater/internal/file/file_test.go | 53 ++++- updater/internal/install/install.go | 62 +++++- updater/internal/install/install_test.go | 209 +++++++++++++++++--- updater/internal/path/path.go | 16 ++ updater/internal/path/path_test.go | 12 ++ updater/internal/rollback/rollback.go | 38 +++- updater/internal/rollback/rollback_test.go | 6 +- updater/internal/service/service_darwin.go | 2 +- updater/internal/service/service_linux.go | 2 +- 12 files changed, 396 insertions(+), 68 deletions(-) diff --git a/updater/internal/action/file_action.go b/updater/internal/action/file_action.go index 90c95df3c..ddf548656 100644 --- a/updater/internal/action/file_action.go +++ b/updater/internal/action/file_action.go @@ -21,7 +21,6 @@ import ( "path/filepath" "github.com/observiq/observiq-otel-collector/updater/internal/file" - "github.com/observiq/observiq-otel-collector/updater/internal/path" "go.uber.org/zap" ) @@ -42,10 +41,10 @@ var _ RollbackableAction = (*CopyFileAction)(nil) var _ fmt.Stringer = (*CopyFileAction)(nil) // NewCopyFileAction creates a new CopyFileAction that indicates a file was copied from -// fromPathRel into toPath. installDir is specified for rollback purposes. +// fromPathRel into toPath. backupDir is specified for rollback purposes. // NOTE: This action MUST be created BEFORE the action actually takes place; This allows // for previous existence of the file to be recorded. -func NewCopyFileAction(logger *zap.Logger, fromPathRel, toPath, installDir string) (*CopyFileAction, error) { +func NewCopyFileAction(logger *zap.Logger, fromPathRel, toPath, backupDir string) (*CopyFileAction, error) { fileExists := true _, err := os.Stat(toPath) switch { @@ -60,7 +59,7 @@ func NewCopyFileAction(logger *zap.Logger, fromPathRel, toPath, installDir strin ToPath: toPath, // The file will be created if it doesn't already exist FileCreated: !fileExists, - backupDir: path.BackupDir(installDir), + backupDir: backupDir, logger: logger.Named("copy-file-action"), }, nil } @@ -76,7 +75,7 @@ func (c CopyFileAction) Rollback() error { // join the relative path to the backup directory to get the location of the backup path backupFilePath := filepath.Join(c.backupDir, c.FromPathRel) - if err := file.CopyFile(c.logger.Named("copy-file"), backupFilePath, c.ToPath, true); err != nil { + if err := file.CopyFile(c.logger.Named("copy-file"), backupFilePath, c.ToPath, true, true); err != nil { return fmt.Errorf("failed to copy file: %w", err) } diff --git a/updater/internal/action/file_action_test.go b/updater/internal/action/file_action_test.go index 119b50cc7..1d384f133 100644 --- a/updater/internal/action/file_action_test.go +++ b/updater/internal/action/file_action_test.go @@ -19,6 +19,7 @@ import ( "path/filepath" "testing" + "github.com/observiq/observiq-otel-collector/updater/internal/path" "github.com/stretchr/testify/require" "go.uber.org/zap/zaptest" ) @@ -27,17 +28,18 @@ func TestNewCopyFileAction(t *testing.T) { t.Run("out file does not exist", func(t *testing.T) { scratchDir := t.TempDir() testInstallDir := filepath.Join("testdata", "copyfileaction") + backupDir := path.BackupDir(testInstallDir) outFile := filepath.Join(scratchDir, "test.txt") inFile := filepath.Join(testInstallDir, "latest", "test.txt") - a, err := NewCopyFileAction(zaptest.NewLogger(t), inFile, outFile, testInstallDir) + a, err := NewCopyFileAction(zaptest.NewLogger(t), inFile, outFile, backupDir) require.NoError(t, err) require.Equal(t, &CopyFileAction{ FromPathRel: inFile, ToPath: outFile, FileCreated: true, - backupDir: filepath.Join(testInstallDir, "tmp", "rollback"), + backupDir: backupDir, logger: a.logger, }, a) }) @@ -45,6 +47,7 @@ func TestNewCopyFileAction(t *testing.T) { t.Run("out file exists", func(t *testing.T) { scratchDir := t.TempDir() testInstallDir := filepath.Join("testdata", "copyfileaction") + backupDir := path.BackupDir(testInstallDir) outFile := filepath.Join(scratchDir, "test.txt") inFile := filepath.Join(testInstallDir, "latest", "test.txt") @@ -52,14 +55,14 @@ func TestNewCopyFileAction(t *testing.T) { require.NoError(t, err) require.NoError(t, f.Close()) - a, err := NewCopyFileAction(zaptest.NewLogger(t), inFile, outFile, testInstallDir) + a, err := NewCopyFileAction(zaptest.NewLogger(t), inFile, outFile, backupDir) require.NoError(t, err) require.Equal(t, &CopyFileAction{ FromPathRel: inFile, ToPath: outFile, FileCreated: false, - backupDir: filepath.Join(testInstallDir, "tmp", "rollback"), + backupDir: backupDir, logger: a.logger, }, a) }) @@ -69,10 +72,11 @@ func TestCopyFileActionRollback(t *testing.T) { t.Run("deletes out file if it does not exist", func(t *testing.T) { scratchDir := t.TempDir() testInstallDir := filepath.Join("testdata", "copyfileaction") + backupDir := path.BackupDir(testInstallDir) outFile := filepath.Join(scratchDir, "test.txt") inFile := filepath.Join(testInstallDir, "tmp", "latest", "test.txt") - a, err := NewCopyFileAction(zaptest.NewLogger(t), inFile, outFile, testInstallDir) + a, err := NewCopyFileAction(zaptest.NewLogger(t), inFile, outFile, backupDir) require.NoError(t, err) inBytes, err := os.ReadFile(inFile) @@ -90,6 +94,7 @@ func TestCopyFileActionRollback(t *testing.T) { t.Run("Rolls back out file when it exists", func(t *testing.T) { scratchDir := t.TempDir() testInstallDir := filepath.Join("testdata", "copyfileaction") + backupDir := path.BackupDir(testInstallDir) outFile := filepath.Join(scratchDir, "test.txt") inFileRel := "test.txt" inFile := filepath.Join(testInstallDir, "tmp", "latest", inFileRel) @@ -101,7 +106,7 @@ func TestCopyFileActionRollback(t *testing.T) { err = os.WriteFile(outFile, originalBytes, 0600) require.NoError(t, err) - a, err := NewCopyFileAction(zaptest.NewLogger(t), inFileRel, outFile, testInstallDir) + a, err := NewCopyFileAction(zaptest.NewLogger(t), inFileRel, outFile, backupDir) require.NoError(t, err) // Overwrite original file with latest file @@ -125,6 +130,7 @@ func TestCopyFileActionRollback(t *testing.T) { t.Run("Fails if backup file doesn't exist", func(t *testing.T) { scratchDir := t.TempDir() testInstallDir := filepath.Join("testdata", "copyfileaction") + backupDir := path.BackupDir(testInstallDir) outFile := filepath.Join(scratchDir, "test.txt") inFile := filepath.Join(testInstallDir, "tmp", "latest", "not_in_backup.txt") originalFile := filepath.Join(testInstallDir, "tmp", "rollback", "test.txt") @@ -136,7 +142,7 @@ func TestCopyFileActionRollback(t *testing.T) { err = os.WriteFile(outFile, originalBytes, 0600) require.NoError(t, err) - a, err := NewCopyFileAction(zaptest.NewLogger(t), inFile, outFile, testInstallDir) + a, err := NewCopyFileAction(zaptest.NewLogger(t), inFile, outFile, backupDir) require.NoError(t, err) // Overwrite original file with latest file diff --git a/updater/internal/file/file.go b/updater/internal/file/file.go index fcc06585e..0dca7df95 100644 --- a/updater/internal/file/file.go +++ b/updater/internal/file/file.go @@ -26,7 +26,7 @@ import ( // CopyFile copies the file from pathIn to pathOut. // If the file does not exist, it is created. If the file does exist, it is truncated before writing. -func CopyFile(logger *zap.Logger, pathIn, pathOut string, overwrite bool) error { +func CopyFile(logger *zap.Logger, pathIn, pathOut string, overwrite bool, useInFilePermBackup bool) error { pathInClean := filepath.Clean(pathIn) // Open the input file for reading. @@ -43,21 +43,42 @@ func CopyFile(logger *zap.Logger, pathIn, pathOut string, overwrite bool) error pathOutClean := filepath.Clean(pathOut) fileMode := fs.FileMode(0600) - flags := os.O_CREATE | os.O_WRONLY if overwrite { // If we are OK to overwrite, we will truncate the file on open flags |= os.O_TRUNC - // Save old file's permissions and delete it first if it exists - fileOutInfo, _ := os.Stat(pathOutClean) - if fileOutInfo != nil { - fileMode = fileOutInfo.Mode() + // Try to save old file's permissions + outFileInfo, _ := os.Stat(pathOutClean) + if outFileInfo != nil { + fileMode = outFileInfo.Mode() + } else if useInFilePermBackup { + // Use the new file's permissions as a backup and don't fail on error (best chance for rollback) + inFileInfo, err := inFile.Stat() + if err != nil { + logger.Error("failed to retrieve fileinfo for input file", zap.Error(err)) + } + if inFileInfo != nil { + fileMode = inFileInfo.Mode() + } + } + + // Remove old file to prevent issues with mac + if err = os.Remove(pathOutClean); err != nil { + logger.Debug("Failed to remove output file", zap.Error(err)) } - _ = os.Remove(pathOutClean) } else { // This flag will make OpenFile error if the file already exists flags |= os.O_EXCL + + // Use the new file's permissions and fail if there's an issue (want to fail for backup) + inFileInfo, err := inFile.Stat() + if err != nil { + return fmt.Errorf("failed to retrive fileinfo for input file: %w", err) + } + if inFileInfo != nil { + fileMode = inFileInfo.Mode() + } } // Open the output file, creating it if it does not exist and truncating it. diff --git a/updater/internal/file/file_test.go b/updater/internal/file/file_test.go index 5dc4c4cd3..12dda94a6 100644 --- a/updater/internal/file/file_test.go +++ b/updater/internal/file/file_test.go @@ -32,7 +32,7 @@ func TestCopyFile(t *testing.T) { inFile := filepath.Join("testdata", "test.txt") outFile := filepath.Join(tmpDir, "test.txt") - err := CopyFile(zaptest.NewLogger(t), inFile, outFile, true) + err := CopyFile(zaptest.NewLogger(t), inFile, outFile, true, false) require.NoError(t, err) require.FileExists(t, outFile) @@ -52,6 +52,34 @@ func TestCopyFile(t *testing.T) { } }) + t.Run("Copies file when output does not exist and uses new permissions when argument set", func(t *testing.T) { + tmpDir := t.TempDir() + + inFile := filepath.Join("testdata", "test.txt") + outFile := filepath.Join(tmpDir, "test.txt") + + err := CopyFile(zaptest.NewLogger(t), inFile, outFile, true, true) + require.NoError(t, err) + require.FileExists(t, outFile) + + contentsIn, err := os.ReadFile(inFile) + require.NoError(t, err) + + contentsOut, err := os.ReadFile(outFile) + require.NoError(t, err) + + require.Equal(t, contentsIn, contentsOut) + + fio, err := os.Stat(outFile) + require.NoError(t, err) + fii, err := os.Stat(outFile) + require.NoError(t, err) + // file mode on windows acts unlike unix, we'll only check for this on linux/darwin + if runtime.GOOS != "windows" { + require.Equal(t, fii.Mode(), fio.Mode()) + } + }) + t.Run("Copies file when output already exists", func(t *testing.T) { tmpDir := t.TempDir() @@ -64,7 +92,10 @@ func TestCopyFile(t *testing.T) { err = os.WriteFile(outFile, []byte("This is a file that already exists"), 0640) require.NoError(t, err) - err = CopyFile(zaptest.NewLogger(t), inFile, outFile, true) + fioOrig, err := os.Stat(outFile) + require.NoError(t, err) + + err = CopyFile(zaptest.NewLogger(t), inFile, outFile, true, false) require.NoError(t, err) require.FileExists(t, outFile) @@ -72,11 +103,11 @@ func TestCopyFile(t *testing.T) { require.NoError(t, err) require.Equal(t, contentsIn, contentsOut) - fi, err := os.Stat(outFile) + fio, err := os.Stat(outFile) require.NoError(t, err) // file mode on windows acts unlike unix, we'll only check for this on linux/darwin if runtime.GOOS != "windows" { - require.Equal(t, fs.FileMode(0640), fi.Mode()) + require.Equal(t, fioOrig.Mode(), fio.Mode()) } }) @@ -86,7 +117,7 @@ func TestCopyFile(t *testing.T) { inFile := filepath.Join("testdata", "does-not-exist.txt") outFile := filepath.Join(tmpDir, "test.txt") - err := CopyFile(zaptest.NewLogger(t), inFile, outFile, true) + err := CopyFile(zaptest.NewLogger(t), inFile, outFile, true, false) require.ErrorContains(t, err, "failed to open input file") require.NoFileExists(t, outFile) }) @@ -100,7 +131,7 @@ func TestCopyFile(t *testing.T) { err := os.WriteFile(outFile, []byte("This is a file that already exists"), 0600) require.NoError(t, err) - err = CopyFile(zaptest.NewLogger(t), inFile, outFile, true) + err = CopyFile(zaptest.NewLogger(t), inFile, outFile, true, false) require.ErrorContains(t, err, "failed to open input file") require.FileExists(t, outFile) @@ -118,7 +149,7 @@ func TestCopyFile(t *testing.T) { err := os.WriteFile(outFile, []byte("This is a file that already exists"), 0640) require.NoError(t, err) - err = CopyFile(zaptest.NewLogger(t), inFile, outFile, false) + err = CopyFile(zaptest.NewLogger(t), inFile, outFile, false, false) require.ErrorContains(t, err, "failed to open output file") require.FileExists(t, outFile) @@ -140,7 +171,7 @@ func TestCopyFile(t *testing.T) { inFile := filepath.Join("testdata", "test.txt") outFile := filepath.Join(tmpDir, "test.txt") - err := CopyFile(zaptest.NewLogger(t), inFile, outFile, false) + err := CopyFile(zaptest.NewLogger(t), inFile, outFile, false, false) require.NoError(t, err) require.FileExists(t, outFile) @@ -152,11 +183,13 @@ func TestCopyFile(t *testing.T) { require.Equal(t, contentsIn, contentsOut) - fi, err := os.Stat(outFile) + fio, err := os.Stat(outFile) + require.NoError(t, err) + fii, err := os.Stat(outFile) require.NoError(t, err) // file mode on windows acts unlike unix, we'll only check for this on linux/darwin if runtime.GOOS != "windows" { - require.Equal(t, fs.FileMode(0600), fi.Mode()) + require.Equal(t, fii.Mode(), fio.Mode()) } }) } diff --git a/updater/internal/install/install.go b/updater/internal/install/install.go index be11e1f1e..b07bf290f 100644 --- a/updater/internal/install/install.go +++ b/updater/internal/install/install.go @@ -15,6 +15,7 @@ package install import ( + "errors" "fmt" "io/fs" "os" @@ -33,6 +34,7 @@ import ( type Installer struct { latestDir string installDir string + backupDir string svc service.Service logger *zap.Logger } @@ -44,6 +46,7 @@ func NewInstaller(logger *zap.Logger, installDir string) *Installer { latestDir: path.LatestDir(installDir), svc: service.NewService(namedLogger, installDir), installDir: installDir, + backupDir: path.BackupDir(installDir), logger: namedLogger, } } @@ -58,9 +61,27 @@ func (i Installer) Install(rb rollback.ActionAppender) error { rb.AppendAction(action.NewServiceStopAction(i.svc)) i.logger.Debug("Service stopped") + // If JMX jar exists outside of install directory, make sure that gets backed up + jarPath := path.SpecialJMXJarFile(i.installDir) + jarDirPath := path.SpecialJarDir(i.installDir) + latestJarPath := path.LatestJMXJarFile(i.latestDir) + _, err := os.Stat(jarPath) + switch { + case err == nil: + if err := installFile(i.logger, latestJarPath, jarDirPath, i.backupDir, rb); err != nil { + return fmt.Errorf("failed to install JMX jar from latest directory: %w", err) + } + // Just log this error as the worst case is that there will be two jars copied over + if err = os.Remove(latestJarPath); err != nil { + i.logger.Warn("Failed to remove JMX jar from latest directory", zap.Error(err)) + } + case !errors.Is(err, os.ErrNotExist): + return fmt.Errorf("failed determine where currently installed JMX jar is: %w", err) + } + // install files that go to installDirPath to their correct location, // excluding any config files (logging.yaml, config.yaml, manager.yaml) - if err := installFiles(i.logger, i.latestDir, i.installDir, rb); err != nil { + if err := installFiles(i.logger, i.latestDir, i.installDir, i.backupDir, rb); err != nil { return fmt.Errorf("failed to install new files: %w", err) } i.logger.Debug("Install artifacts copied") @@ -84,7 +105,7 @@ func (i Installer) Install(rb rollback.ActionAppender) error { // installFiles moves the file tree rooted at inputPath to installDir, // skipping configuration files. Appends CopyFileAction-s to the Rollbacker as it copies file. -func installFiles(logger *zap.Logger, inputPath, installDir string, rb rollback.ActionAppender) error { +func installFiles(logger *zap.Logger, inputPath, installDir, backupDir string, rb rollback.ActionAppender) error { err := filepath.WalkDir(inputPath, func(inPath string, d fs.DirEntry, err error) error { switch { case err != nil: @@ -116,7 +137,7 @@ func installFiles(logger *zap.Logger, inputPath, installDir string, rb rollback. // We create the action record here, because we want to record whether the file exists or not before // we open the file (which will end up creating the file). - cfa, err := action.NewCopyFileAction(logger, relPath, outPath, installDir) + cfa, err := action.NewCopyFileAction(logger, relPath, outPath, backupDir) if err != nil { return fmt.Errorf("failed to create copy file action: %w", err) } @@ -126,7 +147,7 @@ func installFiles(logger *zap.Logger, inputPath, installDir string, rb rollback. // and we will want to roll that back if that is the case. rb.AppendAction(cfa) - if err := file.CopyFile(logger.Named("copy-file"), inPath, outPath, true); err != nil { + if err := file.CopyFile(logger.Named("copy-file"), inPath, outPath, true, false); err != nil { return fmt.Errorf("failed to copy file: %w", err) } @@ -140,6 +161,39 @@ func installFiles(logger *zap.Logger, inputPath, installDir string, rb rollback. return nil } +// installFile moves new file to output path. +// Appends CopyFileAction-s to the Rollbacker as it copies file. +func installFile(logger *zap.Logger, inPath, installDirPath, backupDirPath string, rb rollback.ActionAppender) error { + baseInPath := filepath.Base(inPath) + + // use the relative path to get the outPath (where we should write the file), and + // to get the out directory (which we will create if it does not exist). + outPath := filepath.Join(installDirPath, baseInPath) + outDir := filepath.Dir(outPath) + + if err := os.MkdirAll(outDir, 0750); err != nil { + return fmt.Errorf("failed to create dir: %w", err) + } + + // We create the action record here, because we want to record whether the file exists or not before + // we open the file (which will end up creating the file). + cfa, err := action.NewCopyFileAction(logger, baseInPath, outPath, backupDirPath) + if err != nil { + return fmt.Errorf("failed to create copy file action: %w", err) + } + + // Record that we are performing copying the file. + // We record before we actually do the action here because the file may be partially written, + // and we will want to roll that back if that is the case. + rb.AppendAction(cfa) + + if err := file.CopyFile(logger.Named("copy-file"), inPath, outPath, true, false); err != nil { + return fmt.Errorf("failed to copy file: %w", err) + } + + return nil +} + // skipConfigFiles returns true if the given path is a special config file. // These files should not be overwritten. func skipConfigFiles(path string) bool { diff --git a/updater/internal/install/install_test.go b/updater/internal/install/install_test.go index e3322d42a..9ce86eb0d 100644 --- a/updater/internal/install/install_test.go +++ b/updater/internal/install/install_test.go @@ -38,15 +38,122 @@ func TestInstallArtifacts(t *testing.T) { installer := &Installer{ latestDir: filepath.Join("testdata", "example-install"), installDir: outDir, + backupDir: filepath.Join("testdata", "rollback"), svc: svc, logger: zaptest.NewLogger(t), } + latestJarPath := filepath.Join(installer.latestDir, "opentelemetry-java-contrib-jmx-metrics.jar") + _, err := os.Create(latestJarPath) + require.NoError(t, err) + err = os.WriteFile(latestJarPath, []byte("# The new jar file"), 0660) + require.NoError(t, err) + + outDirConfig := filepath.Join(outDir, "config.yaml") + outDirLogging := filepath.Join(outDir, "logging.yaml") + outDirManager := filepath.Join(outDir, "manager.yaml") + + err = os.WriteFile(outDirConfig, []byte("# The original config file"), 0600) + require.NoError(t, err) + err = os.WriteFile(outDirLogging, []byte("# The original logging file"), 0600) + require.NoError(t, err) + err = os.WriteFile(outDirManager, []byte("# The original manager file"), 0600) + require.NoError(t, err) + + svc.On("Stop").Once().Return(nil) + svc.On("Update").Once().Return(nil) + svc.On("Start").Once().Return(nil) + + actions := []action.RollbackableAction{} + rb.On("AppendAction", mock.Anything).Run(func(args mock.Arguments) { + action := args.Get(0).(action.RollbackableAction) + actions = append(actions, action) + }) + + err = installer.Install(rb) + require.NoError(t, err) + + contentsEqual(t, outDirConfig, "# The original config file") + contentsEqual(t, outDirManager, "# The original manager file") + contentsEqual(t, outDirLogging, "# The original logging file") + + require.FileExists(t, filepath.Join(outDir, "opentelemetry-java-contrib-jmx-metrics.jar")) + require.FileExists(t, filepath.Join(outDir, "test.txt")) + require.DirExists(t, filepath.Join(outDir, "test-folder")) + require.FileExists(t, filepath.Join(outDir, "test-folder", "another-test.txt")) + + contentsEqual(t, filepath.Join(outDir, "opentelemetry-java-contrib-jmx-metrics.jar"), "# The new jar file") + contentsEqual(t, filepath.Join(outDir, "test.txt"), "This is a test file\n") + contentsEqual(t, filepath.Join(outDir, "test-folder", "another-test.txt"), "This is a nested text file\n") + + copyTestTxtAction, err := action.NewCopyFileAction( + installer.logger, + filepath.Join("test.txt"), + filepath.Join(installer.installDir, "test.txt"), + installer.backupDir, + ) + require.NoError(t, err) + copyTestTxtAction.FileCreated = true + + copyJarAction, err := action.NewCopyFileAction( + installer.logger, + filepath.Join("opentelemetry-java-contrib-jmx-metrics.jar"), + filepath.Join(installer.installDir, "opentelemetry-java-contrib-jmx-metrics.jar"), + installer.backupDir, + ) + require.NoError(t, err) + copyJarAction.FileCreated = true + + copyNestedTestTxtAction, err := action.NewCopyFileAction( + installer.logger, + filepath.Join("test-folder", "another-test.txt"), + filepath.Join(installer.installDir, "test-folder", "another-test.txt"), + installer.backupDir, + ) + require.NoError(t, err) + copyNestedTestTxtAction.FileCreated = true + + require.Equal(t, len(actions), 6) + require.Contains(t, actions, action.NewServiceStopAction(svc)) + require.Contains(t, actions, copyJarAction) + require.Contains(t, actions, copyNestedTestTxtAction) + require.Contains(t, actions, copyTestTxtAction) + require.Contains(t, actions, action.NewServiceUpdateAction(installer.logger, installer.installDir)) + require.Contains(t, actions, action.NewServiceStartAction(svc)) + }) + + t.Run("Installs artifacts correctly when linux jmx jar", func(t *testing.T) { + jarDir := t.TempDir() + specialJarPath := filepath.Join(jarDir, "opentelemetry-java-contrib-jmx-metrics.jar") + _, err := os.Create(specialJarPath) + require.NoError(t, err) + err = os.WriteFile(specialJarPath, []byte("# The original jar file"), 0600) + require.NoError(t, err) + outDir := filepath.Join(jarDir, "installdir") + os.MkdirAll(outDir, 0700) + + svc := mocks.NewService(t) + rb := rb_mocks.NewActionAppender(t) + + installer := &Installer{ + latestDir: filepath.Join("testdata", "example-install"), + installDir: outDir, + backupDir: filepath.Join("testdata", "rollback"), + svc: svc, + logger: zaptest.NewLogger(t), + } + + latestJarPath := filepath.Join(installer.latestDir, "opentelemetry-java-contrib-jmx-metrics.jar") + _, err = os.Create(latestJarPath) + require.NoError(t, err) + err = os.WriteFile(latestJarPath, []byte("# The new jar file"), 0660) + require.NoError(t, err) + outDirConfig := filepath.Join(outDir, "config.yaml") outDirLogging := filepath.Join(outDir, "logging.yaml") outDirManager := filepath.Join(outDir, "manager.yaml") - err := os.WriteFile(outDirConfig, []byte("# The original config file"), 0600) + err = os.WriteFile(outDirConfig, []byte("# The original config file"), 0600) require.NoError(t, err) err = os.WriteFile(outDirLogging, []byte("# The original logging file"), 0600) require.NoError(t, err) @@ -70,10 +177,12 @@ func TestInstallArtifacts(t *testing.T) { contentsEqual(t, outDirManager, "# The original manager file") contentsEqual(t, outDirLogging, "# The original logging file") + require.FileExists(t, filepath.Join(jarDir, "opentelemetry-java-contrib-jmx-metrics.jar")) require.FileExists(t, filepath.Join(outDir, "test.txt")) require.DirExists(t, filepath.Join(outDir, "test-folder")) require.FileExists(t, filepath.Join(outDir, "test-folder", "another-test.txt")) + contentsEqual(t, filepath.Join(jarDir, "opentelemetry-java-contrib-jmx-metrics.jar"), "# The new jar file") contentsEqual(t, filepath.Join(outDir, "test.txt"), "This is a test file\n") contentsEqual(t, filepath.Join(outDir, "test-folder", "another-test.txt"), "This is a nested text file\n") @@ -81,27 +190,36 @@ func TestInstallArtifacts(t *testing.T) { installer.logger, filepath.Join("test.txt"), filepath.Join(installer.installDir, "test.txt"), - installer.installDir, + installer.backupDir, ) require.NoError(t, err) copyTestTxtAction.FileCreated = true + copyJarAction, err := action.NewCopyFileAction( + installer.logger, + filepath.Join("opentelemetry-java-contrib-jmx-metrics.jar"), + filepath.Join(jarDir, "opentelemetry-java-contrib-jmx-metrics.jar"), + installer.backupDir, + ) + require.NoError(t, err) + copyJarAction.FileCreated = false + copyNestedTestTxtAction, err := action.NewCopyFileAction( installer.logger, filepath.Join("test-folder", "another-test.txt"), filepath.Join(installer.installDir, "test-folder", "another-test.txt"), - installer.installDir, + installer.backupDir, ) require.NoError(t, err) copyNestedTestTxtAction.FileCreated = true - require.Equal(t, []action.RollbackableAction{ - action.NewServiceStopAction(svc), - copyNestedTestTxtAction, - copyTestTxtAction, - action.NewServiceUpdateAction(installer.logger, installer.installDir), - action.NewServiceStartAction(svc), - }, actions) + require.Equal(t, len(actions), 6) + require.Contains(t, actions, action.NewServiceStopAction(svc)) + require.Contains(t, actions, copyJarAction) + require.Contains(t, actions, copyNestedTestTxtAction) + require.Contains(t, actions, copyTestTxtAction) + require.Contains(t, actions, action.NewServiceUpdateAction(installer.logger, installer.installDir)) + require.Contains(t, actions, action.NewServiceStartAction(svc)) }) t.Run("Stop fails", func(t *testing.T) { @@ -128,10 +246,17 @@ func TestInstallArtifacts(t *testing.T) { installer := &Installer{ latestDir: filepath.Join("testdata", "example-install"), installDir: outDir, + backupDir: filepath.Join("testdata", "rollback"), svc: svc, logger: zaptest.NewLogger(t), } + latestJarPath := filepath.Join(installer.latestDir, "opentelemetry-java-contrib-jmx-metrics.jar") + _, err := os.Create(latestJarPath) + require.NoError(t, err) + err = os.WriteFile(latestJarPath, []byte("# The new jar file"), 0660) + require.NoError(t, err) + svc.On("Stop").Once().Return(nil) svc.On("Update").Once().Return(errors.New("uninstall failed")) @@ -141,13 +266,13 @@ func TestInstallArtifacts(t *testing.T) { actions = append(actions, action) }) - err := installer.Install(rb) + err = installer.Install(rb) require.ErrorContains(t, err, "failed to update service") copyTestTxtAction, err := action.NewCopyFileAction( installer.logger, filepath.Join("test.txt"), filepath.Join(installer.installDir, "test.txt"), - installer.installDir, + installer.backupDir, ) require.NoError(t, err) copyTestTxtAction.FileCreated = true @@ -156,16 +281,25 @@ func TestInstallArtifacts(t *testing.T) { installer.logger, filepath.Join("test-folder", "another-test.txt"), filepath.Join(installer.installDir, "test-folder", "another-test.txt"), - installer.installDir, + installer.backupDir, ) require.NoError(t, err) copyNestedTestTxtAction.FileCreated = true - require.Equal(t, []action.RollbackableAction{ - action.NewServiceStopAction(svc), - copyNestedTestTxtAction, - copyTestTxtAction, - }, actions) + copyJarAction, err := action.NewCopyFileAction( + installer.logger, + filepath.Join("opentelemetry-java-contrib-jmx-metrics.jar"), + filepath.Join(installer.installDir, "opentelemetry-java-contrib-jmx-metrics.jar"), + installer.backupDir, + ) + require.NoError(t, err) + copyJarAction.FileCreated = true + + require.Equal(t, len(actions), 4) + require.Contains(t, actions, action.NewServiceStopAction(svc)) + require.Contains(t, actions, copyJarAction) + require.Contains(t, actions, copyNestedTestTxtAction) + require.Contains(t, actions, copyTestTxtAction) }) t.Run("Start fails", func(t *testing.T) { @@ -175,10 +309,17 @@ func TestInstallArtifacts(t *testing.T) { installer := &Installer{ latestDir: filepath.Join("testdata", "example-install"), installDir: outDir, + backupDir: filepath.Join("testdata", "rollback"), svc: svc, logger: zaptest.NewLogger(t), } + latestJarPath := filepath.Join(installer.latestDir, "opentelemetry-java-contrib-jmx-metrics.jar") + _, err := os.Create(latestJarPath) + require.NoError(t, err) + err = os.WriteFile(latestJarPath, []byte("# The new jar file"), 0660) + require.NoError(t, err) + svc.On("Stop").Once().Return(nil) svc.On("Update").Once().Return(nil) svc.On("Start").Once().Return(errors.New("start failed")) @@ -189,14 +330,14 @@ func TestInstallArtifacts(t *testing.T) { actions = append(actions, action) }) - err := installer.Install(rb) + err = installer.Install(rb) require.ErrorContains(t, err, "failed to start service") copyTestTxtAction, err := action.NewCopyFileAction( installer.logger, filepath.Join("test.txt"), filepath.Join(installer.installDir, "test.txt"), - installer.installDir, + installer.backupDir, ) require.NoError(t, err) copyTestTxtAction.FileCreated = true @@ -205,17 +346,26 @@ func TestInstallArtifacts(t *testing.T) { installer.logger, filepath.Join("test-folder", "another-test.txt"), filepath.Join(installer.installDir, "test-folder", "another-test.txt"), - installer.installDir, + installer.backupDir, ) require.NoError(t, err) copyNestedTestTxtAction.FileCreated = true - require.Equal(t, []action.RollbackableAction{ - action.NewServiceStopAction(svc), - copyNestedTestTxtAction, - copyTestTxtAction, - action.NewServiceUpdateAction(installer.logger, installer.installDir), - }, actions) + copyJarAction, err := action.NewCopyFileAction( + installer.logger, + filepath.Join("opentelemetry-java-contrib-jmx-metrics.jar"), + filepath.Join(installer.installDir, "opentelemetry-java-contrib-jmx-metrics.jar"), + installer.backupDir, + ) + require.NoError(t, err) + copyJarAction.FileCreated = true + + require.Equal(t, len(actions), 5) + require.Contains(t, actions, action.NewServiceStopAction(svc)) + require.Contains(t, actions, copyJarAction) + require.Contains(t, actions, copyNestedTestTxtAction) + require.Contains(t, actions, copyTestTxtAction) + require.Contains(t, actions, action.NewServiceUpdateAction(installer.logger, installer.installDir)) }) t.Run("Latest dir does not exist", func(t *testing.T) { @@ -240,9 +390,8 @@ func TestInstallArtifacts(t *testing.T) { err := installer.Install(rb) require.ErrorContains(t, err, "failed to install new files") - require.Equal(t, []action.RollbackableAction{ - action.NewServiceStopAction(svc), - }, actions) + require.Equal(t, len(actions), 1) + require.Contains(t, actions, action.NewServiceStopAction(svc)) }) } diff --git a/updater/internal/path/path.go b/updater/internal/path/path.go index 509a76040..bd2d91dc0 100644 --- a/updater/internal/path/path.go +++ b/updater/internal/path/path.go @@ -36,6 +36,12 @@ func ServiceFileDir(installDir string) string { return filepath.Join(installDir, "install") } +// SpecialJarDir gets the directory where linux and darwin installs put the JMX jar +// Keeping this relative for now so we don't have to deal with /opt in tests +func SpecialJarDir(installDir string) string { + return filepath.Join(installDir, "..") +} + // BackupServiceFile returns the full path to the backup service file func BackupServiceFile(installDir string) string { return filepath.Join(BackupDir(installDir), "backup.service") @@ -45,3 +51,13 @@ func BackupServiceFile(installDir string) string { func LogFile(installDir string) string { return filepath.Join(installDir, "log", "updater.log") } + +// LatestJMXJarFile returns the full path to the latest JMX jar to be installed +func LatestJMXJarFile(latestDir string) string { + return filepath.Join(latestDir, "opentelemetry-java-contrib-jmx-metrics.jar") +} + +// SpecialJMXJarFile returns the full path to the JMX Jar on linux and darwin installs +func SpecialJMXJarFile(installDir string) string { + return filepath.Join(SpecialJarDir(installDir), "opentelemetry-java-contrib-jmx-metrics.jar") +} diff --git a/updater/internal/path/path_test.go b/updater/internal/path/path_test.go index 538c11e97..7992c2a64 100644 --- a/updater/internal/path/path_test.go +++ b/updater/internal/path/path_test.go @@ -37,6 +37,10 @@ func TestServiceFileDir(t *testing.T) { require.Equal(t, filepath.Join("install", "install"), ServiceFileDir("install")) } +func TestSpecialJarDir(t *testing.T) { + require.Equal(t, filepath.Join("install", ".."), SpecialJarDir("install")) +} + func TestBackupServiceFile(t *testing.T) { require.Equal(t, filepath.Join("install", "tmp", "rollback", "backup.service"), BackupServiceFile("install")) } @@ -44,3 +48,11 @@ func TestBackupServiceFile(t *testing.T) { func TestLogFile(t *testing.T) { require.Equal(t, filepath.Join("install", "log", "updater.log"), LogFile("install")) } + +func TestLatestJMXJarFile(t *testing.T) { + require.Equal(t, filepath.Join("latest", "opentelemetry-java-contrib-jmx-metrics.jar"), LatestJMXJarFile("latest")) +} + +func TestSpecialJMXJarFile(t *testing.T) { + require.Equal(t, filepath.Join("install", "..", "opentelemetry-java-contrib-jmx-metrics.jar"), SpecialJMXJarFile("install")) +} diff --git a/updater/internal/rollback/rollback.go b/updater/internal/rollback/rollback.go index 4c2e6578a..3f4743e16 100644 --- a/updater/internal/rollback/rollback.go +++ b/updater/internal/rollback/rollback.go @@ -15,6 +15,7 @@ package rollback import ( + "errors" "fmt" "io/fs" "os" @@ -74,6 +75,18 @@ func (r Rollbacker) Backup() error { return fmt.Errorf("failed to copy files to backup dir: %w", err) } + // If JMX jar exists outside of install directory, make sure that gets backed up + jarPath := path.SpecialJMXJarFile(r.installDir) + _, err := os.Stat(jarPath) + switch { + case err == nil: + if err := backupFile(r.logger, jarPath, r.backupDir); err != nil { + return fmt.Errorf("failed to copy JMX jar to jar backup dir: %w", err) + } + case !errors.Is(err, os.ErrNotExist): + return fmt.Errorf("failed determine where currently installed JMX jar is: %w", err) + } + // Backup the service configuration so we can reload it in case of rollback if err := r.originalSvc.Backup(); err != nil { return fmt.Errorf("failed to backup service configuration: %w", err) @@ -107,7 +120,7 @@ func backupFiles(logger *zap.Logger, installDir, outputPath string) error { fullPath, absErr := filepath.Abs(inPath) if absErr != nil { - return fmt.Errorf("failed to determine absolute path of file: %w", err) + return fmt.Errorf("failed to determine absolute path of file: %w", absErr) } switch { @@ -140,7 +153,7 @@ func backupFiles(logger *zap.Logger, installDir, outputPath string) error { } // Fail if copying the input file to the output file would fail - if err := file.CopyFile(logger.Named("copy-file"), inPath, outPath, false); err != nil { + if err := file.CopyFile(logger.Named("copy-file"), inPath, outPath, false, false); err != nil { return fmt.Errorf("failed to copy file: %w", err) } @@ -153,3 +166,24 @@ func backupFiles(logger *zap.Logger, installDir, outputPath string) error { return nil } + +// backupFile copies original file to output path +func backupFile(logger *zap.Logger, inPath, outputDirPath string) error { + baseInPath := filepath.Base(inPath) + + // use the relative path to get the outPath (where we should write the file), and + // to get the out directory (which we will create if it does not exist). + outPath := filepath.Join(outputDirPath, baseInPath) + outDir := filepath.Dir(outPath) + + if err := os.MkdirAll(outDir, 0750); err != nil { + return fmt.Errorf("failed to create dir: %w", err) + } + + // Fail if copying the input file to the output file would fail + if err := file.CopyFile(logger.Named("copy-file"), inPath, outPath, false, false); err != nil { + return fmt.Errorf("failed to copy file: %w", err) + } + + return nil +} diff --git a/updater/internal/rollback/rollback_test.go b/updater/internal/rollback/rollback_test.go index 384c61359..31896103c 100644 --- a/updater/internal/rollback/rollback_test.go +++ b/updater/internal/rollback/rollback_test.go @@ -47,6 +47,7 @@ func TestRollbackerBackup(t *testing.T) { err := rb.Backup() require.NoError(t, err) + require.FileExists(t, filepath.Join(outDir, "opentelemetry-java-contrib-jmx-metrics.jar")) require.FileExists(t, filepath.Join(outDir, "some-file.txt")) require.FileExists(t, filepath.Join(outDir, "plugins-dir", "plugin.txt")) require.NoDirExists(t, filepath.Join(outDir, "tmp-dir")) @@ -78,7 +79,9 @@ func TestRollbackerBackup(t *testing.T) { svc := service_mocks.NewService(t) svc.On("Backup").Return(nil) - err := os.WriteFile(leftoverFile, []byte("leftover file"), 0600) + err := os.MkdirAll(outDir, 0750) + require.NoError(t, err) + err = os.WriteFile(leftoverFile, []byte("leftover file"), 0600) require.NoError(t, err) rb := &Rollbacker{ @@ -91,6 +94,7 @@ func TestRollbackerBackup(t *testing.T) { err = rb.Backup() require.NoError(t, err) + require.FileExists(t, filepath.Join(outDir, "opentelemetry-java-contrib-jmx-metrics.jar")) require.FileExists(t, filepath.Join(outDir, "some-file.txt")) require.FileExists(t, filepath.Join(outDir, "plugins-dir", "plugin.txt")) require.NoDirExists(t, filepath.Join(outDir, "tmp-dir")) diff --git a/updater/internal/service/service_darwin.go b/updater/internal/service/service_darwin.go index 908b97ef7..badd8d9b3 100644 --- a/updater/internal/service/service_darwin.go +++ b/updater/internal/service/service_darwin.go @@ -140,7 +140,7 @@ func (d darwinService) Update() error { } func (d darwinService) Backup() error { - if err := file.CopyFile(d.logger.Named("copy-file"), d.installedServiceFilePath, path.BackupServiceFile(d.installDir), false); err != nil { + if err := file.CopyFile(d.logger.Named("copy-file"), d.installedServiceFilePath, path.BackupServiceFile(d.installDir), false, false); err != nil { return fmt.Errorf("failed to copy service file: %w", err) } diff --git a/updater/internal/service/service_linux.go b/updater/internal/service/service_linux.go index 2538f79d0..cd271e7ff 100644 --- a/updater/internal/service/service_linux.go +++ b/updater/internal/service/service_linux.go @@ -165,7 +165,7 @@ func (l linuxService) Update() error { } func (l linuxService) Backup() error { - if err := file.CopyFile(l.logger.Named("copy-file"), l.installedServiceFilePath, path.BackupServiceFile(l.installDir), false); err != nil { + if err := file.CopyFile(l.logger.Named("copy-file"), l.installedServiceFilePath, path.BackupServiceFile(l.installDir), false, false); err != nil { return fmt.Errorf("failed to copy service file: %w", err) }