From 1b30be28e252db1ed57ce7425b7f9899eed1e0a1 Mon Sep 17 00:00:00 2001 From: mitchell Date: Tue, 20 Aug 2024 15:58:12 -0400 Subject: [PATCH 1/5] If two artifacts provide the same file, re-link or re-copy that file from the existing artifact after the other was undeployed/uninstalled. --- internal/smartlink/smartlink.go | 13 ++++- pkg/runtime/depot.go | 95 ++++++++++++++++++++++++++++----- 2 files changed, 94 insertions(+), 14 deletions(-) diff --git a/internal/smartlink/smartlink.go b/internal/smartlink/smartlink.go index 50f8916641..c3eca28464 100644 --- a/internal/smartlink/smartlink.go +++ b/internal/smartlink/smartlink.go @@ -78,7 +78,7 @@ func Link(src, dest string) error { // UnlinkContents will unlink the contents of src to dest if the links exist // WARNING: on windows smartlinks are hard links, and relating hard links back to their source is non-trivial, so instead // we just delete the target path. If the user modified the target in any way their changes will be lost. -func UnlinkContents(src, dest string) error { +func UnlinkContents(src, dest string, ignorePaths ...string) error { if !fileutils.DirExists(dest) { return errs.New("dest dir does not exist: %s", dest) } @@ -89,6 +89,11 @@ func UnlinkContents(src, dest string) error { return errs.Wrap(err, "Could not resolve src and dest paths") } + ignore := make(map[string]bool) + for _, path := range ignorePaths { + ignore[path] = true + } + entries, err := os.ReadDir(src) if err != nil { return errs.Wrap(err, "Reading dir %s failed", dest) @@ -101,8 +106,12 @@ func UnlinkContents(src, dest string) error { continue } + if _, yes := ignore[destPath]; yes { + continue + } + if fileutils.IsDir(destPath) { - if err := UnlinkContents(srcPath, destPath); err != nil { + if err := UnlinkContents(srcPath, destPath, ignorePaths...); err != nil { return err // Not wrapping here cause it'd just repeat the same error due to the recursion } } else { diff --git a/pkg/runtime/depot.go b/pkg/runtime/depot.go index a776f339e6..39f80eaf01 100644 --- a/pkg/runtime/depot.go +++ b/pkg/runtime/depot.go @@ -28,9 +28,10 @@ type depotConfig struct { } type deployment struct { - Type deploymentType `json:"type"` - Path string `json:"path"` - Files []string `json:"files"` + Type deploymentType `json:"type"` + Path string `json:"path"` + Files []string `json:"files"` + InstallDir string `json:"installDir"` } type deploymentType string @@ -187,9 +188,10 @@ func (d *depot) DeployViaLink(id strfmt.UUID, relativeSrc, absoluteDest string) d.config.Deployments[id] = []deployment{} } d.config.Deployments[id] = append(d.config.Deployments[id], deployment{ - Type: deploymentTypeLink, - Path: absoluteDest, - Files: files.RelativePaths(), + Type: deploymentTypeLink, + Path: absoluteDest, + Files: files.RelativePaths(), + InstallDir: relativeSrc, }) return nil @@ -243,9 +245,10 @@ func (d *depot) DeployViaCopy(id strfmt.UUID, relativeSrc, absoluteDest string) d.config.Deployments[id] = []deployment{} } d.config.Deployments[id] = append(d.config.Deployments[id], deployment{ - Type: deploymentTypeCopy, - Path: absoluteDest, - Files: files.RelativePaths(), + Type: deploymentTypeCopy, + Path: absoluteDest, + Files: files.RelativePaths(), + InstallDir: relativeSrc, }) return nil @@ -270,16 +273,44 @@ func (d *depot) Undeploy(id strfmt.UUID, relativeSrc, path string) error { if !ok { return errs.New("deployment for %s not found in depot", id) } - deploy := sliceutils.Filter(deployments, func(d deployment) bool { return d.Path == path }) - if len(deploy) != 1 { + deployments = sliceutils.Filter(deployments, func(d deployment) bool { return d.Path == path }) + if len(deployments) != 1 { return errs.New("no deployment found for %s in depot", path) } + // Determine if there are any files provided by another artifact that will need to be re-linked + // or re-copied after this artifact is uninstalled. + relinks, err := d.getSharedFilesToReLink(id, relativeSrc, path) + if err != nil { + return errs.Wrap(err, "failed to get shared files") + } + sharedFiles := make([]string, 0) + for file := range relinks { + sharedFiles = append(sharedFiles, file) + } + // Perform uninstall based on deployment type - if err := smartlink.UnlinkContents(filepath.Join(d.Path(id), relativeSrc), path); err != nil { + if err := smartlink.UnlinkContents(filepath.Join(d.Path(id), relativeSrc), path, sharedFiles...); err != nil { return errs.Wrap(err, "failed to unlink artifact") } + // Re-link or re-copy any files provided by other artifacts. + for sharedFile, relinkSrc := range relinks { + if err := os.Remove(sharedFile); err != nil { + return errs.Wrap(err, "failed to remove file") + } + switch deployments[0].Type { + case deploymentTypeLink: + if err := smartlink.Link(relinkSrc, sharedFile); err != nil { + return errs.Wrap(err, "failed to relink file") + } + case deploymentTypeCopy: + if err := fileutils.CopyFile(relinkSrc, sharedFile); err != nil { + return errs.Wrap(err, "failed to re-copy file") + } + } + } + // Write changes to config d.config.Deployments[id] = sliceutils.Filter(d.config.Deployments[id], func(d deployment) bool { return d.Path != path }) @@ -300,6 +331,46 @@ func (d *depot) validateVolume(absoluteDest string) error { return nil } +// getSharedFilesToReLink returns a map of deployed files to re-link to (or re-copy from) another +// artifact that provides those files. The key is the deployed file path and the value is the +// source path from another artifact. +func (d *depot) getSharedFilesToReLink(id strfmt.UUID, relativeSrc, path string) (map[string]string, error) { + // Map of deployed paths to other sources that provides those paths. + relink := make(map[string]string, 0) + + // Get a listing of all files deployed by this artifact. + deployedDir := filepath.Join(d.Path(id), relativeSrc) + deployedFiles, err := fileutils.ListDirSimple(deployedDir, false) + if err != nil { + return nil, errs.Wrap(err, "failed to list depot files for artifact") + } + + // For each of those files, find another artifact (if any) that deploys its own copy. + for _, deployedFile := range deployedFiles { + relativeDeployedFile := deployedFile[len(deployedDir)+1:] + for artifactId, artifactDeployments := range d.config.Deployments { + if artifactId == id { + continue + } + + for _, deployment := range artifactDeployments { + for _, relativeDeploymentFile := range deployment.Files { + if relativeDeployedFile == relativeDeploymentFile { + // We'll want to relink this other artifact's copy after undeploying the currently deployed version. + deployedFile := filepath.Join(path, relativeDeployedFile) + newSrc := filepath.Join(d.Path(artifactId), deployment.InstallDir, relativeDeployedFile) + logging.Debug("More than one artifact provides '%s'", relativeDeployedFile) + logging.Debug("Will relink %s to %s", deployedFile, newSrc) + relink[deployedFile] = newSrc + } + } + } + } + } + + return relink, nil +} + // Save will write config changes to disk (ie. links between depot artifacts and runtimes that use it). // It will also delete any stale artifacts which are not used by any runtime. func (d *depot) Save() error { From 4eda2f27078b4dd7c00822231868ac8862f2b02e Mon Sep 17 00:00:00 2001 From: mitchell Date: Wed, 21 Aug 2024 17:23:05 -0400 Subject: [PATCH 2/5] Iterate over declared deployment files, not what's on disk. Also address PR feedback. --- pkg/runtime/depot.go | 67 ++++++++++++++++++++------------------------ 1 file changed, 30 insertions(+), 37 deletions(-) diff --git a/pkg/runtime/depot.go b/pkg/runtime/depot.go index 39f80eaf01..e5a1cc2d69 100644 --- a/pkg/runtime/depot.go +++ b/pkg/runtime/depot.go @@ -28,10 +28,10 @@ type depotConfig struct { } type deployment struct { - Type deploymentType `json:"type"` - Path string `json:"path"` - Files []string `json:"files"` - InstallDir string `json:"installDir"` + Type deploymentType `json:"type"` + Path string `json:"path"` + Files []string `json:"files"` + RelativeSrc string `json:"relativeSrc"` } type deploymentType string @@ -188,10 +188,10 @@ func (d *depot) DeployViaLink(id strfmt.UUID, relativeSrc, absoluteDest string) d.config.Deployments[id] = []deployment{} } d.config.Deployments[id] = append(d.config.Deployments[id], deployment{ - Type: deploymentTypeLink, - Path: absoluteDest, - Files: files.RelativePaths(), - InstallDir: relativeSrc, + Type: deploymentTypeLink, + Path: absoluteDest, + Files: files.RelativePaths(), + RelativeSrc: relativeSrc, }) return nil @@ -245,10 +245,10 @@ func (d *depot) DeployViaCopy(id strfmt.UUID, relativeSrc, absoluteDest string) d.config.Deployments[id] = []deployment{} } d.config.Deployments[id] = append(d.config.Deployments[id], deployment{ - Type: deploymentTypeCopy, - Path: absoluteDest, - Files: files.RelativePaths(), - InstallDir: relativeSrc, + Type: deploymentTypeCopy, + Path: absoluteDest, + Files: files.RelativePaths(), + RelativeSrc: relativeSrc, }) return nil @@ -277,15 +277,16 @@ func (d *depot) Undeploy(id strfmt.UUID, relativeSrc, path string) error { if len(deployments) != 1 { return errs.New("no deployment found for %s in depot", path) } + deploy := deployments[0] // Determine if there are any files provided by another artifact that will need to be re-linked // or re-copied after this artifact is uninstalled. - relinks, err := d.getSharedFilesToReLink(id, relativeSrc, path) + redeploys, err := d.getSharedFilesToRedeploy(id, deploy, path) if err != nil { return errs.Wrap(err, "failed to get shared files") } sharedFiles := make([]string, 0) - for file := range relinks { + for file := range redeploys { sharedFiles = append(sharedFiles, file) } @@ -295,11 +296,11 @@ func (d *depot) Undeploy(id strfmt.UUID, relativeSrc, path string) error { } // Re-link or re-copy any files provided by other artifacts. - for sharedFile, relinkSrc := range relinks { + for sharedFile, relinkSrc := range redeploys { if err := os.Remove(sharedFile); err != nil { return errs.Wrap(err, "failed to remove file") } - switch deployments[0].Type { + switch deploy.Type { case deploymentTypeLink: if err := smartlink.Link(relinkSrc, sharedFile); err != nil { return errs.Wrap(err, "failed to relink file") @@ -331,44 +332,36 @@ func (d *depot) validateVolume(absoluteDest string) error { return nil } -// getSharedFilesToReLink returns a map of deployed files to re-link to (or re-copy from) another +// getSharedFilesToRedeploy returns a map of deployed files to re-link to (or re-copy from) another // artifact that provides those files. The key is the deployed file path and the value is the // source path from another artifact. -func (d *depot) getSharedFilesToReLink(id strfmt.UUID, relativeSrc, path string) (map[string]string, error) { +func (d *depot) getSharedFilesToRedeploy(id strfmt.UUID, deploy deployment, path string) (map[string]string, error) { // Map of deployed paths to other sources that provides those paths. - relink := make(map[string]string, 0) + redeploy := make(map[string]string, 0) - // Get a listing of all files deployed by this artifact. - deployedDir := filepath.Join(d.Path(id), relativeSrc) - deployedFiles, err := fileutils.ListDirSimple(deployedDir, false) - if err != nil { - return nil, errs.Wrap(err, "failed to list depot files for artifact") - } - - // For each of those files, find another artifact (if any) that deploys its own copy. - for _, deployedFile := range deployedFiles { - relativeDeployedFile := deployedFile[len(deployedDir)+1:] + // For each file deployed by this artifact, find another artifact (if any) that deploys its own copy. + for _, relativeDeployedFile := range deploy.Files { + deployedFile := filepath.Join(path, relativeDeployedFile) for artifactId, artifactDeployments := range d.config.Deployments { if artifactId == id { continue } for _, deployment := range artifactDeployments { - for _, relativeDeploymentFile := range deployment.Files { - if relativeDeployedFile == relativeDeploymentFile { - // We'll want to relink this other artifact's copy after undeploying the currently deployed version. - deployedFile := filepath.Join(path, relativeDeployedFile) - newSrc := filepath.Join(d.Path(artifactId), deployment.InstallDir, relativeDeployedFile) + for _, fileToDeploy := range deployment.Files { + if relativeDeployedFile == fileToDeploy { + // We'll want to redeploy this from other artifact's copy after undeploying the currently deployed version. + newSrc := filepath.Join(d.Path(artifactId), deployment.RelativeSrc, relativeDeployedFile) logging.Debug("More than one artifact provides '%s'", relativeDeployedFile) - logging.Debug("Will relink %s to %s", deployedFile, newSrc) - relink[deployedFile] = newSrc + logging.Debug("Will redeploy '%s' to '%s'", newSrc, deployedFile) + redeploy[deployedFile] = newSrc } } } } } - return relink, nil + return redeploy, nil } // Save will write config changes to disk (ie. links between depot artifacts and runtimes that use it). From fc84551f9f1b5324779acf88291e57edca071c31 Mon Sep 17 00:00:00 2001 From: mitchell Date: Thu, 22 Aug 2024 11:16:11 -0400 Subject: [PATCH 3/5] Smartlink.Link() should create directories as needed. We no longer have to avoid removing files, which could have removed needed directories. --- internal/smartlink/smartlink.go | 19 ++++++++----------- pkg/runtime/depot.go | 20 +++++--------------- 2 files changed, 13 insertions(+), 26 deletions(-) diff --git a/internal/smartlink/smartlink.go b/internal/smartlink/smartlink.go index c3eca28464..245b725d7c 100644 --- a/internal/smartlink/smartlink.go +++ b/internal/smartlink/smartlink.go @@ -62,6 +62,12 @@ func Link(src, dest string) error { return nil } + if destDir := filepath.Dir(dest); !fileutils.DirExists(destDir) { + if err := os.MkdirAll(destDir, 0755); err != nil { + return errs.Wrap(err, "could not create directory %s", destDir) + } + } + // Multiple artifacts can supply the same file. We do not have a better solution for this at the moment other than // favouring the first one encountered. if fileutils.TargetExists(dest) { @@ -78,7 +84,7 @@ func Link(src, dest string) error { // UnlinkContents will unlink the contents of src to dest if the links exist // WARNING: on windows smartlinks are hard links, and relating hard links back to their source is non-trivial, so instead // we just delete the target path. If the user modified the target in any way their changes will be lost. -func UnlinkContents(src, dest string, ignorePaths ...string) error { +func UnlinkContents(src, dest string) error { if !fileutils.DirExists(dest) { return errs.New("dest dir does not exist: %s", dest) } @@ -89,11 +95,6 @@ func UnlinkContents(src, dest string, ignorePaths ...string) error { return errs.Wrap(err, "Could not resolve src and dest paths") } - ignore := make(map[string]bool) - for _, path := range ignorePaths { - ignore[path] = true - } - entries, err := os.ReadDir(src) if err != nil { return errs.Wrap(err, "Reading dir %s failed", dest) @@ -106,12 +107,8 @@ func UnlinkContents(src, dest string, ignorePaths ...string) error { continue } - if _, yes := ignore[destPath]; yes { - continue - } - if fileutils.IsDir(destPath) { - if err := UnlinkContents(srcPath, destPath, ignorePaths...); err != nil { + if err := UnlinkContents(srcPath, destPath); err != nil { return err // Not wrapping here cause it'd just repeat the same error due to the recursion } } else { diff --git a/pkg/runtime/depot.go b/pkg/runtime/depot.go index e5a1cc2d69..d556fee5aa 100644 --- a/pkg/runtime/depot.go +++ b/pkg/runtime/depot.go @@ -279,27 +279,17 @@ func (d *depot) Undeploy(id strfmt.UUID, relativeSrc, path string) error { } deploy := deployments[0] - // Determine if there are any files provided by another artifact that will need to be re-linked - // or re-copied after this artifact is uninstalled. - redeploys, err := d.getSharedFilesToRedeploy(id, deploy, path) - if err != nil { - return errs.Wrap(err, "failed to get shared files") - } - sharedFiles := make([]string, 0) - for file := range redeploys { - sharedFiles = append(sharedFiles, file) - } - // Perform uninstall based on deployment type - if err := smartlink.UnlinkContents(filepath.Join(d.Path(id), relativeSrc), path, sharedFiles...); err != nil { + if err := smartlink.UnlinkContents(filepath.Join(d.Path(id), relativeSrc), path); err != nil { return errs.Wrap(err, "failed to unlink artifact") } // Re-link or re-copy any files provided by other artifacts. + redeploys, err := d.getSharedFilesToRedeploy(id, deploy, path) + if err != nil { + return errs.Wrap(err, "failed to get shared files") + } for sharedFile, relinkSrc := range redeploys { - if err := os.Remove(sharedFile); err != nil { - return errs.Wrap(err, "failed to remove file") - } switch deploy.Type { case deploymentTypeLink: if err := smartlink.Link(relinkSrc, sharedFile); err != nil { From 845e392430d1fadf1fb85084a088ec17442aad95 Mon Sep 17 00:00:00 2001 From: mitchell Date: Thu, 22 Aug 2024 11:38:42 -0400 Subject: [PATCH 4/5] Break after finding a shared file. --- pkg/runtime/depot.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkg/runtime/depot.go b/pkg/runtime/depot.go index d556fee5aa..043deb4304 100644 --- a/pkg/runtime/depot.go +++ b/pkg/runtime/depot.go @@ -345,10 +345,12 @@ func (d *depot) getSharedFilesToRedeploy(id strfmt.UUID, deploy deployment, path logging.Debug("More than one artifact provides '%s'", relativeDeployedFile) logging.Debug("Will redeploy '%s' to '%s'", newSrc, deployedFile) redeploy[deployedFile] = newSrc + goto nextDeployedFile } } } } + nextDeployedFile: } return redeploy, nil From 3aa4976d8295da2c0d79a8f38e83b79c6e878d6d Mon Sep 17 00:00:00 2001 From: mitchell Date: Thu, 22 Aug 2024 13:12:32 -0400 Subject: [PATCH 5/5] Cleanup per PR feedback. --- internal/smartlink/smartlink.go | 9 ++++----- pkg/runtime/depot.go | 17 ++++++++++++----- 2 files changed, 16 insertions(+), 10 deletions(-) diff --git a/internal/smartlink/smartlink.go b/internal/smartlink/smartlink.go index 245b725d7c..feb5134fa5 100644 --- a/internal/smartlink/smartlink.go +++ b/internal/smartlink/smartlink.go @@ -47,7 +47,7 @@ func Link(src, dest string) error { } if fileutils.IsDir(src) { - if err := os.MkdirAll(dest, 0755); err != nil { + if err := fileutils.Mkdir(dest); err != nil { return errs.Wrap(err, "could not create directory %s", dest) } entries, err := os.ReadDir(src) @@ -62,10 +62,9 @@ func Link(src, dest string) error { return nil } - if destDir := filepath.Dir(dest); !fileutils.DirExists(destDir) { - if err := os.MkdirAll(destDir, 0755); err != nil { - return errs.Wrap(err, "could not create directory %s", destDir) - } + destDir := filepath.Dir(dest) + if err := fileutils.MkdirUnlessExists(destDir); err != nil { + return errs.Wrap(err, "could not create directory %s", destDir) } // Multiple artifacts can supply the same file. We do not have a better solution for this at the moment other than diff --git a/pkg/runtime/depot.go b/pkg/runtime/depot.go index 043deb4304..28a2c83539 100644 --- a/pkg/runtime/depot.go +++ b/pkg/runtime/depot.go @@ -337,20 +337,27 @@ func (d *depot) getSharedFilesToRedeploy(id strfmt.UUID, deploy deployment, path continue } - for _, deployment := range artifactDeployments { - for _, fileToDeploy := range deployment.Files { - if relativeDeployedFile == fileToDeploy { + findArtifact := func() bool { + for _, deployment := range artifactDeployments { + for _, fileToDeploy := range deployment.Files { + if relativeDeployedFile != fileToDeploy { + continue + } // We'll want to redeploy this from other artifact's copy after undeploying the currently deployed version. newSrc := filepath.Join(d.Path(artifactId), deployment.RelativeSrc, relativeDeployedFile) logging.Debug("More than one artifact provides '%s'", relativeDeployedFile) logging.Debug("Will redeploy '%s' to '%s'", newSrc, deployedFile) redeploy[deployedFile] = newSrc - goto nextDeployedFile + return true } } + return false + } + + if findArtifact() { + break // ignore all other copies once one is found } } - nextDeployedFile: } return redeploy, nil