Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Only log once (per phase) when we have to get target distro information from /etc/os-release #1424

Merged
merged 2 commits into from
Nov 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 28 additions & 4 deletions internal/fsutil/os_detection.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@ package fsutil
import (
"os"
"strings"
"sync"

"github.com/buildpacks/lifecycle/log"
)

type OSInfo struct {
Expand All @@ -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 {
Expand All @@ -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")
})
}
4 changes: 2 additions & 2 deletions internal/fsutil/os_detection_linux_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
})
Expand Down
4 changes: 2 additions & 2 deletions internal/fsutil/os_detection_notlinux_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
})
Expand Down
2 changes: 1 addition & 1 deletion internal/fsutil/os_detection_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
2 changes: 1 addition & 1 deletion phase/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
Expand Down
6 changes: 4 additions & 2 deletions phase/detector.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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 {
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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
}
Expand Down
2 changes: 1 addition & 1 deletion phase/generator.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
Expand Down
2 changes: 1 addition & 1 deletion platform/run_image.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
23 changes: 15 additions & 8 deletions platform/target_data.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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())
Expand All @@ -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
Expand Down
7 changes: 7 additions & 0 deletions platform/target_data_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
}
Loading