Only log once (per phase) when we have to get target distro informati…

…on from /etc/os-release (#1424) * Only log once (per phase) when we have to get target distro information from /etc/os-release Signed-off-by: Natalie Arellano <narellano@vmware.com> * Save the distro information the first time we read /etc/os-release, so that we end up only reading that file once Signed-off-by: Natalie Arellano <narellano@vmware.com> --------- Signed-off-by: Natalie Arellano <narellano@vmware.com>
buildpacks · Nov 15, 2024 · 21811fa · 21811fa
1 parent 0690d13
commit 21811fa
Show file tree

Hide file tree

Showing 10 changed files with 62 additions and 22 deletions.
diff --git a/internal/fsutil/os_detection.go b/internal/fsutil/os_detection.go
@@ -3,6 +3,9 @@ package fsutil
 import (
 	"os"
 	"strings"
+	"sync"
+
+	"github.com/buildpacks/lifecycle/log"
 )
 
 type OSInfo struct {
@@ -14,25 +17,33 @@ type Detector interface {
 	HasSystemdFile() bool
 	ReadSystemdFile() (string, error)
 	GetInfo(osReleaseContents string) OSInfo
+	StoredInfo() *OSInfo
+	InfoOnce(logger log.Logger)
 }
 
-type Detect struct {
+// DefaultDetector implements Detector
+type DefaultDetector struct {
+	once sync.Once
+	info *OSInfo
 }
 
-func (d *Detect) HasSystemdFile() bool {
+// HasSystemdFile returns true if /etc/os-release exists with contents
+func (d *DefaultDetector) HasSystemdFile() bool {
 	finfo, err := os.Stat("/etc/os-release")
 	if err != nil {
 		return false
 	}
 	return !finfo.IsDir() && finfo.Size() > 0
 }
 
-func (d *Detect) ReadSystemdFile() (string, error) {
+// ReadSystemdFile returns the contents of /etc/os-release
+func (d *DefaultDetector) ReadSystemdFile() (string, error) {
 	bs, err := os.ReadFile("/etc/os-release")
 	return string(bs), err
 }
 
-func (d *Detect) GetInfo(osReleaseContents string) OSInfo {
+// GetInfo returns the OS distribution name and version from the contents of /etc/os-release
+func (d *DefaultDetector) GetInfo(osReleaseContents string) OSInfo {
 	ret := OSInfo{}
 	lines := strings.Split(osReleaseContents, "\n")
 	for _, line := range lines {
@@ -51,5 +62,18 @@ func (d *Detect) GetInfo(osReleaseContents string) OSInfo {
 			break
 		}
 	}
+	d.info = &ret // store for future use
 	return ret
 }
+
+// StoredInfo returns any OSInfo found during the last call to GetInfo
+func (d *DefaultDetector) StoredInfo() *OSInfo {
+	return d.info
+}
+
+// InfoOnce logs an info message to the provided logger, but only once in the lifetime of the receiving DefaultDetector.
+func (d *DefaultDetector) InfoOnce(logger log.Logger) {
+	d.once.Do(func() {
+		logger.Info("target distro name/version labels not found, reading /etc/os-release file")
+	})
+}
diff --git a/internal/fsutil/os_detection_linux_test.go b/internal/fsutil/os_detection_linux_test.go
@@ -20,10 +20,10 @@ func TestDetectorUnix(t *testing.T) {
 func testDetectorUnix(t *testing.T, when spec.G, it spec.S) {
 	when("we should have a file", func() {
 		it("returns true correctly", func() {
-			h.AssertEq(t, (&fsutil.Detect{}).HasSystemdFile(), true)
+			h.AssertEq(t, (&fsutil.DefaultDetector{}).HasSystemdFile(), true)
 		})
 		it("returns the file contents", func() {
-			s, err := (&fsutil.Detect{}).ReadSystemdFile()
+			s, err := (&fsutil.DefaultDetector{}).ReadSystemdFile()
 			h.AssertNil(t, err)
 			h.AssertStringContains(t, s, "NAME")
 		})

diff --git a/internal/fsutil/os_detection_notlinux_test.go b/internal/fsutil/os_detection_notlinux_test.go
@@ -20,10 +20,10 @@ func TestDetectorNonUnix(t *testing.T) {
 func testDetectorNonUnix(t *testing.T, when spec.G, it spec.S) {
 	when("we don't have a file", func() {
 		it("returns false correctly", func() {
-			h.AssertEq(t, (&fsutil.Detect{}).HasSystemdFile(), false)
+			h.AssertEq(t, (&fsutil.DefaultDetector{}).HasSystemdFile(), false)
 		})
 		it("returns an error correctly", func() {
-			_, err := (&fsutil.Detect{}).ReadSystemdFile()
+			_, err := (&fsutil.DefaultDetector{}).ReadSystemdFile()
 			h.AssertNotNil(t, err)
 		})
 	})

diff --git a/internal/fsutil/os_detection_test.go b/internal/fsutil/os_detection_test.go
@@ -15,7 +15,7 @@ func TestDetector(t *testing.T) {
 }
 
 // there's no state on this object so we can just use the same one forever
-var detect fsutil.Detect
+var detect fsutil.DefaultDetector
 
 func testDetector(t *testing.T, when spec.G, it spec.S) {
 	when("we have the contents of an os-release file", func() {

diff --git a/phase/builder.go b/phase/builder.go
@@ -149,7 +149,7 @@ func (b *Builder) getBuildInputs() buildpack.BuildInputs {
 		LayersDir:      b.LayersDir,
 		PlatformDir:    b.PlatformDir,
 		Env:            env.NewBuildEnv(os.Environ()),
-		TargetEnv:      platform.EnvVarsFor(&fsutil.Detect{}, b.AnalyzeMD.RunImageTarget(), b.Logger),
+		TargetEnv:      platform.EnvVarsFor(&fsutil.DefaultDetector{}, b.AnalyzeMD.RunImageTarget(), b.Logger),
 		Out:            b.Out,
 		Err:            b.Err,
 	}

diff --git a/phase/detector.go b/phase/detector.go
@@ -52,6 +52,7 @@ type Detector struct {
 	Runs           *sync.Map
 	AnalyzeMD      files.Analyzed
 	PlatformAPI    *api.Version
+	OSDetector     *fsutil.DefaultDetector
 
 	// If detect fails, we want to print debug statements as info level.
 	// memHandler holds all log entries; we'll iterate through them at the end of detect,
@@ -73,6 +74,7 @@ func (f *HermeticFactory) NewDetector(inputs platform.LifecycleInputs, logger lo
 		Runs:           &sync.Map{},
 		memHandler:     memHandler,
 		PlatformAPI:    f.platformAPI,
+		OSDetector:     &fsutil.DefaultDetector{},
 	}
 	var err error
 	if detector.AnalyzeMD, err = f.configHandler.ReadAnalyzed(inputs.AnalyzedPath, logger); err != nil {
@@ -198,7 +200,7 @@ func (d *Detector) detectGroup(group buildpack.Group, done []buildpack.GroupElem
 			} else {
 				for _, target := range descriptor.TargetsList() {
 					d.Logger.Debugf("Checking for match against descriptor: %s", target)
-					if platform.TargetSatisfiedForBuild(&fsutil.Detect{}, &runImageTargetInfo, target, d.Logger) {
+					if platform.TargetSatisfiedForBuild(d.OSDetector, &runImageTargetInfo, target, d.Logger) {
 						targetMatch = true
 						break
 					}
@@ -233,7 +235,7 @@ func (d *Detector) detectGroup(group buildpack.Group, done []buildpack.GroupElem
 					BuildConfigDir: d.BuildConfigDir,
 					PlatformDir:    d.PlatformDir,
 					Env:            env.NewBuildEnv(os.Environ()),
-					TargetEnv:      platform.EnvVarsFor(&fsutil.Detect{}, runImageTargetInfo, d.Logger),
+					TargetEnv:      platform.EnvVarsFor(d.OSDetector, runImageTargetInfo, d.Logger),
 				}
 				d.Runs.Store(key, d.Executor.Detect(descriptor, inputs, d.Logger)) // this is where we finally invoke bin/detect
 			}

diff --git a/phase/generator.go b/phase/generator.go
@@ -151,7 +151,7 @@ func (g *Generator) getGenerateInputs() buildpack.GenerateInputs {
 		BuildConfigDir: g.BuildConfigDir,
 		PlatformDir:    g.PlatformDir,
 		Env:            env.NewBuildEnv(os.Environ()),
-		TargetEnv:      platform.EnvVarsFor(&fsutil.Detect{}, g.AnalyzedMD.RunImageTarget(), g.Logger),
+		TargetEnv:      platform.EnvVarsFor(&fsutil.DefaultDetector{}, g.AnalyzedMD.RunImageTarget(), g.Logger),
 		Out:            g.Out,
 		Err:            g.Err,
 	}

diff --git a/platform/run_image.go b/platform/run_image.go
@@ -72,7 +72,7 @@ func byRegistry(reg string, images []string, checkReadAccess CheckReadAccess, ke
 // - stack.toml for older platforms
 // - run.toml for newer platforms, where the run image information returned is
 //   - the first set of image & mirrors that contains the platform-provided run image, or
-//   - the platform-provided run image if extensions were used and the image was not found, or
+//   - the platform-provided run image if extensions were used and the image was not found in run.toml, or
 //   - the first set of image & mirrors in run.toml
 //
 // The "platform-provided run image" is the run image "image" in analyzed.toml,

diff --git a/platform/target_data.go b/platform/target_data.go
@@ -53,7 +53,6 @@ func TargetSatisfiedForBuild(d fsutil.Detector, base *files.TargetMetadata, modu
 	}
 	// ensure we have all available data
 	if base.Distro == nil {
-		logger.Info("target distro name/version labels not found, reading /etc/os-release file")
 		GetTargetOSFromFileSystem(d, base, logger)
 	}
 	// check matches
@@ -93,13 +92,22 @@ func matches(target1, target2 string) bool {
 // GetTargetOSFromFileSystem populates the provided target metadata with information from /etc/os-release
 // if it is available.
 func GetTargetOSFromFileSystem(d fsutil.Detector, tm *files.TargetMetadata, logger log.Logger) {
-	if d.HasSystemdFile() {
-		if tm.OS == "" {
-			tm.OS = "linux"
-		}
-		if tm.Arch == "" {
-			tm.Arch = runtime.GOARCH // in a future world where we support cross platform builds, this should be removed
+	if tm.OS == "" {
+		tm.OS = "linux" // we shouldn't get here, as OS comes from the image config, and OS is always required
+	}
+	if tm.Arch == "" {
+		tm.Arch = runtime.GOARCH // in a future world where we support cross-platform builds, this should be removed
+	}
+
+	if info := d.StoredInfo(); info != nil {
+		if info.Version != "" || info.Name != "" {
+			tm.Distro = &files.OSDistro{Name: info.Name, Version: info.Version}
 		}
+		return
+	}
+
+	d.InfoOnce(logger)
+	if d.HasSystemdFile() {
 		contents, err := d.ReadSystemdFile()
 		if err != nil {
 			logger.Warnf("Encountered error trying to read /etc/os-release file: %s", err.Error())
@@ -118,7 +126,6 @@ func EnvVarsFor(d fsutil.Detector, tm files.TargetMetadata, logger log.Logger) [
 	// we should always have os & arch,
 	// if they are not populated try to get target information from the build-time base image
 	if tm.Distro == nil {
-		logger.Info("target distro name/version labels not found, reading /etc/os-release file")
 		GetTargetOSFromFileSystem(d, &tm, logger)
 	}
 	// required

diff --git a/platform/target_data_test.go b/platform/target_data_test.go
@@ -11,6 +11,7 @@ import (
 
 	"github.com/buildpacks/lifecycle/buildpack"
 	"github.com/buildpacks/lifecycle/internal/fsutil"
+	llog "github.com/buildpacks/lifecycle/log"
 	"github.com/buildpacks/lifecycle/platform"
 	"github.com/buildpacks/lifecycle/platform/files"
 	h "github.com/buildpacks/lifecycle/testhelpers"
@@ -324,3 +325,9 @@ func (d *mockDetector) GetInfo(osReleaseContents string) fsutil.OSInfo {
 		Version: "3.14",
 	}
 }
+
+func (d *mockDetector) InfoOnce(_ llog.Logger) {}
+
+func (d *mockDetector) StoredInfo() *fsutil.OSInfo {
+	return nil
+}