Only log once (per phase) when we have to get target distro information 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>
This commit is contained in:
Natalie Arellano 2024-11-15 11:08:29 -05:00 committed by GitHub
parent 0690d133ca
commit 21811fa4dc
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
10 changed files with 62 additions and 22 deletions

View File

@ -3,6 +3,9 @@ package fsutil
import (
"os"
"strings"
"sync"
"github.com/buildpacks/lifecycle/log"
)
type OSInfo struct {
@ -14,12 +17,18 @@ 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
@ -27,12 +36,14 @@ func (d *Detect) HasSystemdFile() bool {
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")
})
}

View File

@ -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")
})

View File

@ -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)
})
})

View File

@ -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() {

View File

@ -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,
}

View File

@ -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
}

View File

@ -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,
}

View File

@ -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,

View File

@ -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 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() {
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
}
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

View File

@ -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
}