diff --git a/CHANGELOG.md b/CHANGELOG.md index e89e433e7..97e64134e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,6 +30,9 @@ CHANGELOG - Update .NET `Grpc` libraries to 2.33.1 and `Protobuf` to 3.13.0 (forked to increase the recursion limit) [#5757](https://github.com/pulumi/pulumi/pull/5757) +- Fix plugin install failures on Windows. + [#5759](https://github.com/pulumi/pulumi/pull/5759) + ## 2.13.2 (2020-11-06) - Fix a bug that was causing errors when (de)serializing custom resources. diff --git a/sdk/go/common/workspace/plugins.go b/sdk/go/common/workspace/plugins.go index 71cd9f6bb..be159c56c 100644 --- a/sdk/go/common/workspace/plugins.go +++ b/sdk/go/common/workspace/plugins.go @@ -38,6 +38,7 @@ import ( "github.com/pulumi/pulumi/sdk/v2/go/common/diag/colors" "github.com/pulumi/pulumi/sdk/v2/go/common/util/archive" "github.com/pulumi/pulumi/sdk/v2/go/common/util/contract" + "github.com/pulumi/pulumi/sdk/v2/go/common/util/fsutil" "github.com/pulumi/pulumi/sdk/v2/go/common/util/httputil" "github.com/pulumi/pulumi/sdk/v2/go/common/util/logging" "github.com/pulumi/pulumi/sdk/v2/go/common/version" @@ -89,6 +90,7 @@ type PluginInfo struct { InstallTime time.Time // the time the plugin was installed. LastUsedTime time.Time // the last time the plugin was used. ServerURL string // an optional server to use when downloading this plugin. + PluginDir string // if set, will be used as the root plugin dir instead of ~/.pulumi/plugins. } // Dir gets the expected plugin directory for this plugin. @@ -120,11 +122,36 @@ func (info PluginInfo) FileSuffix() string { // DirPath returns the directory where this plugin should be installed. func (info PluginInfo) DirPath() (string, error) { - dir, err := GetPluginDir() + var err error + dir := info.PluginDir + if dir == "" { + dir, err = GetPluginDir() + if err != nil { + return "", err + } + } + + return filepath.Join(dir, info.Dir()), nil +} + +// LockFilePath returns the full path to the plugin's lock file used during installation +// to prevent concurrent installs. +func (info PluginInfo) LockFilePath() (string, error) { + dir, err := info.DirPath() if err != nil { return "", err } - return filepath.Join(dir, info.Dir()), nil + return fmt.Sprintf("%s.lock", dir), nil +} + +// PartialFilePath returns the full path to the plugin's partial file used during installation +// to indicate installation of the plugin hasn't completed yet. +func (info PluginInfo) PartialFilePath() (string, error) { + dir, err := info.DirPath() + if err != nil { + return "", err + } + return fmt.Sprintf("%s.partial", dir), nil } // FilePath returns the full path where this plugin's primary executable should be installed. @@ -143,7 +170,14 @@ func (info PluginInfo) Delete() error { if err != nil { return err } - return os.RemoveAll(dir) + if err := os.RemoveAll(dir); err != nil { + return err + } + // Attempt to delete any leftover .partial or .lock files. + // Don't fail the operation if we can't delete these. + contract.IgnoreError(os.Remove(fmt.Sprintf("%s.partial", dir))) + contract.IgnoreError(os.Remove(fmt.Sprintf("%s.lock", dir))) + return nil } // SetFileMetadata adds extra metadata from the given file, representing this plugin's directory. @@ -231,51 +265,110 @@ func (info PluginInfo) Download() (io.ReadCloser, int64, error) { return resp.Body, resp.ContentLength, nil } -// Install installs a plugin's tarball into the cache. It validates that plugin names are in the expected format. +// installLock acquires a file lock used to prevent concurrent installs. +func (info PluginInfo) installLock() (unlock func(), err error) { + finalDir, err := info.DirPath() + if err != nil { + return nil, err + } + lockFilePath := fmt.Sprintf("%s.lock", finalDir) + + if err := os.MkdirAll(filepath.Dir(lockFilePath), 0700); err != nil { + return nil, errors.Wrap(err, "creating plugin root") + } + + mutex := fsutil.NewFileMutex(lockFilePath) + if err := mutex.Lock(); err != nil { + return nil, err + } + return func() { + contract.IgnoreError(mutex.Unlock()) + }, nil +} + +// Install installs a plugin's tarball into the cache. It validates that plugin names are in the expected format. +// Previous versions of Pulumi extracted the tarball to a temp directory first, and then renamed the temp directory +// to the final directory. The rename operation fails often enough on Windows due to aggressive virus scanners opening +// files in the temp directory. To address this, we now extract the tarball directly into the final directory, and use +// file locks to prevent concurrent installs. +// Each plugin has its own file lock, with the same name as the plugin directory, with a `.lock` suffix. +// During installation an empty file with a `.partial` suffix is created, indicating that installation is in-progress. +// The `.partial` file is deleted when installation is complete, indicating that the plugin has finished installing. +// If a failure occurs during installation, the `.partial` file will remain, indicating the plugin wasn't fully +// installed. The next time the plugin is installed, the old installation directory will be removed and replaced with +// a fresh install. func (info PluginInfo) Install(tarball io.ReadCloser) error { - // Fetch the directory into which we will expand this tarball, and create it. + defer contract.IgnoreClose(tarball) + + // Fetch the directory into which we will expand this tarball. finalDir, err := info.DirPath() if err != nil { return err } - return installPlugin(finalDir, tarball) -} -func installPlugin(finalDir string, tarball io.ReadCloser) error { - // If part of the directory tree is missing, ioutil.TempDir will return an error, so make sure the path we're going - // to create the temporary folder in actually exists. - if err := os.MkdirAll(filepath.Dir(finalDir), 0700); err != nil { - return errors.Wrap(err, "creating plugin root") - } - - tempDir, err := ioutil.TempDir(filepath.Dir(finalDir), fmt.Sprintf("%s.tmp", filepath.Base(finalDir))) + // Create a file lock file at /--.lock. + unlock, err := info.installLock() if err != nil { - return errors.Wrapf(err, "creating plugin directory %s", tempDir) + return err + } + defer unlock() + + // Cleanup any temp dirs from failed installations of this plugin from previous versions of Pulumi. + if err := cleanupTempDirs(finalDir); err != nil { + // We don't want to fail the installation if there was an error cleaning up these old temp dirs. + // Instead, log the error and continue on. + logging.V(5).Infof("Install: Error cleaning up temp dirs: %s", err.Error()) } - // If we early out of this function, try to remove the temp folder we created. - defer func() { - contract.IgnoreError(os.RemoveAll(tempDir)) - }() - - // Uncompress the plugin. We do this inside a function so that the `defer`'s to close files - // happen before we later try to rename the directory. Otherwise, the open file handles cause - // issues on Windows. - err = (func() error { - defer contract.IgnoreClose(tarball) - tarballBytes, err := ioutil.ReadAll(tarball) - if err != nil { - return err - } - - return archive.UnTGZ(tarballBytes, tempDir) - })() + // Get the partial file path (e.g. /--.partial). + partialFilePath, err := info.PartialFilePath() if err != nil { return err } + // Check whether the directory exists while we were waiting on the lock. + _, finalDirStatErr := os.Stat(finalDir) + if finalDirStatErr == nil { + _, partialFileStatErr := os.Stat(partialFilePath) + if partialFileStatErr != nil { + if os.IsNotExist(partialFileStatErr) { + // finalDir exists and there's no partial file, so the plugin is already installed. + return nil + } + return partialFileStatErr + } + + // The partial file exists, meaning a previous attempt at installing the plugin failed. + // Delete finalDir so we can try installing again. There's no need to delete the partial + // file since we'd just be recreating it again below anyway. + if err := os.RemoveAll(finalDir); err != nil { + return err + } + } else if !os.IsNotExist(finalDirStatErr) { + return finalDirStatErr + } + + // Create an empty partial file to indicate installation is in-progress. + if err := ioutil.WriteFile(partialFilePath, nil, 0600); err != nil { + return err + } + + // Create the final directory. + if err := os.MkdirAll(finalDir, 0700); err != nil { + return err + } + + // Uncompress the plugin. + tarballBytes, err := ioutil.ReadAll(tarball) + if err != nil { + return err + } + if err := archive.UnTGZ(tarballBytes, finalDir); err != nil { + return err + } + // Install dependencies, if needed. - proj, err := LoadPluginProject(filepath.Join(tempDir, "PulumiPlugin.yaml")) + proj, err := LoadPluginProject(filepath.Join(finalDir, "PulumiPlugin.yaml")) if err != nil && !os.IsNotExist(err) { return errors.Wrap(err, "loading PulumiPlugin.yaml") } @@ -287,34 +380,39 @@ func installPlugin(finalDir string, tarball io.ReadCloser) error { // TODO[pulumi/pulumi#1334]: move to the language plugins so we don't have to hard code here. switch runtime { case "nodejs": - if _, err := npm.Install(tempDir, nil, os.Stderr); err != nil { + if _, err := npm.Install(finalDir, nil, os.Stderr); err != nil { return errors.Wrap(err, "installing plugin dependencies") } case "python": - if err := python.InstallDependencies(tempDir, false /*showOutput*/, nil /*saveProj*/); err != nil { + if err := python.InstallDependencies(finalDir, false /*showOutput*/, nil /*saveProj*/); err != nil { return errors.Wrap(err, "installing plugin dependencies") } } } - // If two calls to `plugin install` for the same plugin are racing, the second one will be unable to rename - // the directory. That's OK, just ignore the error. The temp directory created as part of the install will be - // cleaned up when we exit by the defer above. - fmt.Print("Moving plugin...") - logging.V(1).Infof("moving plugin from %q to %q", tempDir, finalDir) - if err := os.Rename(tempDir, finalDir); err != nil && !os.IsExist(err) { - switch err.(type) { - case *os.LinkError: - // On Windows, an Access Denied error is sometimes thrown when renaming. Work around by trying the - // second time, which seems to work fine. See https://github.com/pulumi/pulumi/issues/2695 - if err := os.Rename(tempDir, finalDir); err != nil && !os.IsExist(err) { - return errors.Wrap(err, "moving plugin") + // Installation is complete. Remove the partial file. + return os.Remove(partialFilePath) +} + +// cleanupTempDirs cleans up leftover temp dirs from failed installs with previous versions of Pulumi. +func cleanupTempDirs(finalDir string) error { + dir := filepath.Dir(finalDir) + + infos, err := ioutil.ReadDir(dir) + if err != nil { + return err + } + + for _, info := range infos { + // Temp dirs have a suffix of `.tmpXXXXXX` (where `XXXXXX`) is a random number, + // from ioutil.TempFile. + if info.IsDir() && installingPluginRegexp.MatchString(info.Name()) { + path := filepath.Join(dir, info.Name()) + if err := os.RemoveAll(path); err != nil { + return errors.Wrapf(err, "cleaning up temp dir %s", path) } - default: - return errors.Wrap(err, "moving plugin") } } - fmt.Println(" done.") return nil } @@ -355,7 +453,12 @@ func HasPlugin(plug PluginInfo) bool { if err == nil { _, err := os.Stat(dir) if err == nil { - return true + partialFilePath, err := plug.PartialFilePath() + if err == nil { + if _, err := os.Stat(partialFilePath); os.IsNotExist(err) { + return true + } + } } } return false @@ -433,6 +536,10 @@ func GetPlugins() ([]PluginInfo, error) { if err != nil { return nil, err } + return getPlugins(dir) +} + +func getPlugins(dir string) ([]PluginInfo, error) { files, err := ioutil.ReadDir(dir) if err != nil { if os.IsNotExist(err) { @@ -451,7 +558,14 @@ func GetPlugins() ([]PluginInfo, error) { Kind: kind, Version: &version, } - if err = plugin.SetFileMetadata(filepath.Join(dir, file.Name())); err != nil { + path := filepath.Join(dir, file.Name()) + if _, err := os.Stat(fmt.Sprintf("%s.partial", path)); err == nil { + // Skip it if the partial file exists, meaning the plugin is not fully installed. + continue + } else if !os.IsNotExist(err) { + return nil, err + } + if err = plugin.SetFileMetadata(path); err != nil { return nil, err } plugins = append(plugins, plugin) @@ -671,9 +785,9 @@ var pluginRegexp = regexp.MustCompile( "(?P[a-zA-Z0-9-]*[a-zA-Z0-9])-" + // NAME "v(?P.*)$") // VERSION -// installingPluginRegexp matches the name of folders for plugins which are being installed. During installation -// we extract plugins to a folder with a suffix of `.tmpXXXXXX` (where `XXXXXX`) is a random number, from -// ioutil.TempFile. We should ignore these plugins as they have not yet been successfully installed. +// installingPluginRegexp matches the name of temporary folders. Previous versions of Pulumi first extracted +// plugins to a temporary folder with a suffix of `.tmpXXXXXX` (where `XXXXXX`) is a random number, from +// ioutil.TempFile. We should ignore these folders. var installingPluginRegexp = regexp.MustCompile(`\.tmp[0-9]+$`) // tryPlugin returns true if a file is a plugin, and extracts information about it. diff --git a/sdk/go/common/workspace/plugins_install_test.go b/sdk/go/common/workspace/plugins_install_test.go index 41138bd4c..7fab35fb8 100644 --- a/sdk/go/common/workspace/plugins_install_test.go +++ b/sdk/go/common/workspace/plugins_install_test.go @@ -20,11 +20,15 @@ import ( "archive/tar" "bytes" "compress/gzip" + "fmt" + "io" "io/ioutil" "os" "path/filepath" + "sync" "testing" + "github.com/blang/semver" "github.com/stretchr/testify/assert" ) @@ -58,48 +62,199 @@ func createTGZ(files map[string][]byte) ([]byte, error) { return buffer.Bytes(), nil } +func prepareTestDir(t *testing.T, files map[string][]byte) (string, io.ReadCloser, PluginInfo) { + if files == nil { + files = map[string][]byte{} + } + + // Add plugin binary to included files. + files["pulumi-resource-test"] = nil + + tgz, err := createTGZ(files) + assert.NoError(t, err) + tarball := ioutil.NopCloser(bytes.NewReader(tgz)) + + dir, err := ioutil.TempDir("", "plugins-test-dir") + assert.NoError(t, err) + + v1 := semver.MustParse("0.1.0") + plugin := PluginInfo{ + Name: "test", + Kind: ResourcePlugin, + Version: &v1, + PluginDir: dir, + } + + return dir, tarball, plugin +} + +func assertPluginInstalled(t *testing.T, dir string, plugin PluginInfo) { + info, err := os.Stat(filepath.Join(dir, plugin.Dir())) + assert.NoError(t, err) + assert.True(t, info.IsDir()) + + info, err = os.Stat(filepath.Join(dir, plugin.Dir(), plugin.File())) + assert.NoError(t, err) + assert.False(t, info.IsDir()) + + info, err = os.Stat(filepath.Join(dir, plugin.Dir()+".partial")) + assert.Error(t, err) + assert.True(t, os.IsNotExist(err)) + + assert.True(t, HasPlugin(plugin)) + + has, err := HasPluginGTE(plugin) + assert.NoError(t, err) + assert.True(t, has) + + plugins, err := getPlugins(dir) + assert.NoError(t, err) + assert.Equal(t, 1, len(plugins)) + assert.Equal(t, plugin.Name, plugins[0].Name) + assert.Equal(t, plugin.Kind, plugins[0].Kind) + assert.Equal(t, *plugin.Version, *plugins[0].Version) +} + +func testDeletePlugin(t *testing.T, dir string, plugin PluginInfo) { + err := plugin.Delete() + assert.NoError(t, err) + + paths := []string{ + filepath.Join(dir, plugin.Dir()), + filepath.Join(dir, plugin.Dir()+".partial"), + filepath.Join(dir, plugin.Dir()+".lock"), + } + for _, path := range paths { + _, err := os.Stat(path) + assert.Error(t, err) + assert.True(t, os.IsNotExist(err)) + } +} + func testPluginInstall(t *testing.T, expectedDir string, files map[string][]byte) { // Skip during short test runs since this test involves downloading dependencies. if testing.Short() { t.Skip("Skipped in short test run") } - tgz, err := createTGZ(files) + dir, tarball, plugin := prepareTestDir(t, files) + defer os.RemoveAll(dir) + + err := plugin.Install(tarball) assert.NoError(t, err) - finalDirRoot, err := ioutil.TempDir("", "final-dir") - assert.NoError(t, err) - defer os.RemoveAll(finalDirRoot) - finalDir := filepath.Join(finalDirRoot, "final") + assertPluginInstalled(t, dir, plugin) - err = installPlugin(finalDir, ioutil.NopCloser(bytes.NewReader(tgz))) - assert.NoError(t, err) - - info, err := os.Stat(filepath.Join(finalDir, expectedDir)) + info, err := os.Stat(filepath.Join(dir, plugin.Dir(), expectedDir)) assert.NoError(t, err) assert.True(t, info.IsDir()) + + testDeletePlugin(t, dir, plugin) } func TestInstallNoDeps(t *testing.T) { name := "foo.txt" content := []byte("hello\n") - tgz, err := createTGZ(map[string][]byte{name: content}) + dir, tarball, plugin := prepareTestDir(t, map[string][]byte{name: content}) + defer os.RemoveAll(dir) + + err := plugin.Install(tarball) assert.NoError(t, err) - finalDirRoot, err := ioutil.TempDir("", "final-dir") - assert.NoError(t, err) - defer os.RemoveAll(finalDirRoot) - finalDir := filepath.Join(finalDirRoot, "final") + assertPluginInstalled(t, dir, plugin) - err = installPlugin(finalDir, ioutil.NopCloser(bytes.NewReader(tgz))) - assert.NoError(t, err) - - info, err := os.Stat(filepath.Join(finalDir, name)) - assert.NoError(t, err) - assert.False(t, info.IsDir()) - - b, err := ioutil.ReadFile(filepath.Join(finalDir, name)) + b, err := ioutil.ReadFile(filepath.Join(dir, plugin.Dir(), name)) assert.NoError(t, err) assert.Equal(t, content, b) + + testDeletePlugin(t, dir, plugin) +} + +func TestConcurrentInstalls(t *testing.T) { + name := "foo.txt" + content := []byte("hello\n") + + dir, tarball, plugin := prepareTestDir(t, map[string][]byte{name: content}) + defer os.RemoveAll(dir) + + assertSuccess := func() { + assertPluginInstalled(t, dir, plugin) + + b, err := ioutil.ReadFile(filepath.Join(dir, plugin.Dir(), name)) + assert.NoError(t, err) + assert.Equal(t, content, b) + } + + // Run several installs concurrently. + const iterations = 12 + var wg sync.WaitGroup + for i := 0; i < iterations; i++ { + wg.Add(1) + go func() { + defer wg.Done() + + err := plugin.Install(tarball) + assert.NoError(t, err) + + assertSuccess() + }() + } + wg.Wait() + + assertSuccess() + + testDeletePlugin(t, dir, plugin) +} + +func TestInstallCleansOldFiles(t *testing.T) { + dir, tarball, plugin := prepareTestDir(t, nil) + defer os.RemoveAll(dir) + + // Leftover temp dirs. + tempDir1, err := ioutil.TempDir(dir, fmt.Sprintf("%s.tmp", plugin.Dir())) + assert.NoError(t, err) + tempDir2, err := ioutil.TempDir(dir, fmt.Sprintf("%s.tmp", plugin.Dir())) + assert.NoError(t, err) + tempDir3, err := ioutil.TempDir(dir, fmt.Sprintf("%s.tmp", plugin.Dir())) + assert.NoError(t, err) + + // Leftover partial file. + partialPath := filepath.Join(dir, plugin.Dir()+".partial") + err = ioutil.WriteFile(partialPath, nil, 0600) + assert.NoError(t, err) + + err = plugin.Install(tarball) + assert.NoError(t, err) + + assertPluginInstalled(t, dir, plugin) + + // Verify leftover files were removed. + for _, path := range []string{tempDir1, tempDir2, tempDir3, partialPath} { + _, err := os.Stat(path) + assert.Error(t, err) + assert.True(t, os.IsNotExist(err)) + } + + testDeletePlugin(t, dir, plugin) +} + +func TestGetPluginsSkipsPartial(t *testing.T) { + dir, tarball, plugin := prepareTestDir(t, nil) + defer os.RemoveAll(dir) + + err := plugin.Install(tarball) + assert.NoError(t, err) + + err = ioutil.WriteFile(filepath.Join(dir, plugin.Dir()+".partial"), nil, 0600) + assert.NoError(t, err) + + assert.False(t, HasPlugin(plugin)) + + has, err := HasPluginGTE(plugin) + assert.Error(t, err) + assert.False(t, has) + + plugins, err := getPlugins(dir) + assert.Equal(t, 0, len(plugins)) }