diff --git a/pkg/baseboard/baseboard.go b/pkg/baseboard/baseboard.go index f503194c..93461678 100644 --- a/pkg/baseboard/baseboard.go +++ b/pkg/baseboard/baseboard.go @@ -57,7 +57,14 @@ func (i *Info) String() string { // New returns a pointer to an Info struct containing information about the // host's baseboard func New(opts ...*option.Option) (*Info, error) { - ctx := context.New(opts...) + return NewWithContext(context.New(opts...)) +} + +// New returns a pointer to an Info struct containing information about the +// host's baseboard, reusing a given context. +// Use this function when you want to consume this package from another, +// ensuring the two see a coherent set of resources. +func NewWithContext(ctx *context.Context) (*Info, error) { info := &Info{ctx: ctx} if err := ctx.Do(info.load); err != nil { return nil, err diff --git a/pkg/bios/bios.go b/pkg/bios/bios.go index 85a7c64b..b5abfcf3 100644 --- a/pkg/bios/bios.go +++ b/pkg/bios/bios.go @@ -50,7 +50,14 @@ func (i *Info) String() string { // New returns a pointer to a Info struct containing information // about the host's BIOS func New(opts ...*option.Option) (*Info, error) { - ctx := context.New(opts...) + return NewWithContext(context.New(opts...)) +} + +// New returns a pointer to a Info struct containing information +// about the host's BIOS, reusing a given context. +// Use this function when you want to consume this package from another, +// ensuring the two see a coherent set of resources. +func NewWithContext(ctx *context.Context) (*Info, error) { info := &Info{ctx: ctx} if err := ctx.Do(info.load); err != nil { return nil, err diff --git a/pkg/block/block.go b/pkg/block/block.go index 7a2e4e38..d8ce2ffe 100644 --- a/pkg/block/block.go +++ b/pkg/block/block.go @@ -134,7 +134,14 @@ type Info struct { // New returns a pointer to an Info struct that describes the block storage // resources of the host system. func New(opts ...*option.Option) (*Info, error) { - ctx := context.New(opts...) + return NewWithContext(context.New(opts...)) +} + +// New returns a pointer to an Info struct that describes the block storage +// resources of the host system, reusing a given context. +// Use this function when you want to consume this package from another, +// ensuring the two see a coherent set of resources. +func NewWithContext(ctx *context.Context) (*Info, error) { info := &Info{ctx: ctx} if err := ctx.Do(info.load); err != nil { return nil, err diff --git a/pkg/chassis/chassis.go b/pkg/chassis/chassis.go index e11f0447..e610fbd8 100644 --- a/pkg/chassis/chassis.go +++ b/pkg/chassis/chassis.go @@ -94,7 +94,14 @@ func (i *Info) String() string { // New returns a pointer to a Info struct containing information // about the host's chassis func New(opts ...*option.Option) (*Info, error) { - ctx := context.New(opts...) + return NewWithContext(context.New(opts...)) +} + +// New returns a pointer to a Info struct containing information +// about the host's chassis, reusing a given context. +// Use this function when you want to consume this package from another, +// ensuring the two see a coherent set of resources. +func NewWithContext(ctx *context.Context) (*Info, error) { info := &Info{ctx: ctx} if err := ctx.Do(info.load); err != nil { return nil, err diff --git a/pkg/context/context.go b/pkg/context/context.go index e5ac3471..9a44ec4a 100644 --- a/pkg/context/context.go +++ b/pkg/context/context.go @@ -7,6 +7,8 @@ package context import ( + "fmt" + "github.com/jaypipes/ghw/pkg/option" "github.com/jaypipes/ghw/pkg/snapshot" ) @@ -22,6 +24,8 @@ type Context struct { PathOverrides option.PathOverrides snapshotUnpackedPath string alert option.Alerter + refCount int + err error } // New returns a Context struct pointer that has had various options set on it @@ -53,10 +57,17 @@ func New(opts ...*option.Option) *Context { ctx.PathOverrides = merged.PathOverrides } + // New is not allowed to return error - it would break the established API. + // so the only way out is to actually do the checks here and record the error, + // and return it later, at the earliest possible occasion, in Setup() + if ctx.SnapshotPath != "" && ctx.Chroot != option.DefaultChroot { + // The env/client code supplied a value, but we are will overwrite it when unpacking shapshots! + ctx.err = fmt.Errorf("Conflicting options: chroot %q and snapshot path %q", ctx.Chroot, ctx.SnapshotPath) + } return ctx } -// FromEnv returns an Option that has been populated from the environs or +// FromEnv returns a Context that has been populated from the environs or // default options values func FromEnv() *Context { chrootVal := option.EnvOrDefaultChroot() @@ -89,8 +100,17 @@ func (ctx *Context) Do(fn func() error) error { // You should call `Setup` just once. It is safe to call `Setup` if you don't make // use of optional extra features - `Setup` will do nothing. func (ctx *Context) Setup() error { + if ctx.err != nil { + return ctx.err + } + if ctx.RefCount() > 1 { + // someone else came before and fixed things already! + ctx.incRefCount() + return nil + } if ctx.SnapshotPath == "" { - // nothing to do! + // nothing to do + ctx.incRefCount() return nil } @@ -108,26 +128,57 @@ func (ctx *Context) Setup() error { } _, err = snapshot.UnpackInto(ctx.SnapshotPath, root, flags) } - if err != nil { - return err + if err == nil { + ctx.Chroot = root + ctx.incRefCount() + return nil } - ctx.Chroot = root - return nil + // not ready: must try again + return err } // Teardown releases any resource acquired by Setup. // You should always call `Teardown` if you called `Setup` to free any resources // acquired by `Setup`. Check `Do` for more automated management. func (ctx *Context) Teardown() error { + if ctx.RefCount() > 1 { + // too early, the last one will do the cleaning + ctx.decRefCount() + return nil + } if ctx.snapshotUnpackedPath == "" { // if the client code provided the unpack directory, // then it is also in charge of the cleanup. + ctx.decRefCount() return nil } - return snapshot.Cleanup(ctx.snapshotUnpackedPath) + err := snapshot.Cleanup(ctx.snapshotUnpackedPath) + if err == nil { + ctx.decRefCount() + return nil + } + // else need to try again later + return err } func (ctx *Context) Warn(msg string, args ...interface{}) { ctx.alert.Printf("WARNING: "+msg, args...) } + +func (ctx *Context) IsReady() bool { + return ctx.refCount > 0 +} + +// RefCount() must be used only in testing code +func (ctx *Context) RefCount() int { + return ctx.refCount +} + +func (ctx *Context) incRefCount() { + ctx.refCount++ +} + +func (ctx *Context) decRefCount() { + ctx.refCount-- +} diff --git a/pkg/context/context_test.go b/pkg/context/context_test.go index e12d3f74..5cb92e1b 100644 --- a/pkg/context/context_test.go +++ b/pkg/context/context_test.go @@ -40,3 +40,94 @@ func TestSnapshotContext(t *testing.T) { t.Fatalf("Expected the uncompressed dir to be deleted: %s", uncompressedDir) } } + +// nolint: gocyclo +func TestContextReadiness(t *testing.T) { + ctx := context.New() + if ctx.IsReady() { + t.Fatalf("context ready before Setup()") + } + + ctx.Do(func() error { + if !ctx.IsReady() { + t.Fatalf("context NOT ready inside Do()") + } + return nil + }) + + if ctx.IsReady() { + t.Fatalf("context ready after Teardown()") + } +} + +// nolint: gocyclo +func TestContextReadinessNested(t *testing.T) { + ctx := context.New() + if ctx.IsReady() { + t.Fatalf("context ready before Setup()") + } + + ctx.Do(func() error { + if !ctx.IsReady() { + t.Fatalf("context NOT ready inside outer Do()") + } + ctx.Do(func() error { + if !ctx.IsReady() { + t.Fatalf("context NOT ready inside inner Do()") + } + return nil + }) + if !ctx.IsReady() { + t.Fatalf("context NOT ready after inner Do()") + } + return nil + }) + + if ctx.IsReady() { + t.Fatalf("context ready after Teardown() - refcount = %d", ctx.RefCount()) + } +} + +// nolint: gocyclo +func TestContextReadinessDeeplyNested(t *testing.T) { + // we don't expect more nesting than this atm + ctx := context.New() + if ctx.IsReady() { + t.Fatalf("context ready before Setup()") + } + if ctx.RefCount() != 0 { + t.Fatalf("context refcount unexpected value %d", ctx.RefCount()) + } + + ctx.Do(func() error { + if !ctx.IsReady() { + t.Fatalf("context NOT ready inside outer Do()") + } + ctx.Do(func() error { + if !ctx.IsReady() { + t.Fatalf("context NOT ready inside middle Do()") + } + ctx.Do(func() error { + if !ctx.IsReady() { + t.Fatalf("context NOT ready inside inner Do()") + } + return nil + }) + if !ctx.IsReady() { + t.Fatalf("context NOT ready after inner Do()") + } + return nil + }) + if !ctx.IsReady() { + t.Fatalf("context NOT ready after middle Do()") + } + return nil + }) + + if ctx.IsReady() { + t.Fatalf("context ready after Teardown() - refcount = %d", ctx.RefCount()) + } + if ctx.RefCount() != 0 { + t.Fatalf("context refcount unexpected value %d", ctx.RefCount()) + } +} diff --git a/pkg/cpu/cpu.go b/pkg/cpu/cpu.go index 2fa0cd2d..48644b60 100644 --- a/pkg/cpu/cpu.go +++ b/pkg/cpu/cpu.go @@ -117,7 +117,14 @@ type Info struct { // New returns a pointer to an Info struct that contains information about the // CPUs on the host system func New(opts ...*option.Option) (*Info, error) { - ctx := context.New(opts...) + return NewWithContext(context.New(opts...)) +} + +// New returns a pointer to an Info struct that contains information about the +// CPUs on the host system, reusing a given context. +// Use this function when you want to consume this package from another, +// ensuring the two see a coherent set of resources. +func NewWithContext(ctx *context.Context) (*Info, error) { info := &Info{ctx: ctx} if err := ctx.Do(info.load); err != nil { return nil, err diff --git a/pkg/gpu/gpu.go b/pkg/gpu/gpu.go index 65864c7e..e14e3efb 100644 --- a/pkg/gpu/gpu.go +++ b/pkg/gpu/gpu.go @@ -56,7 +56,14 @@ type Info struct { // New returns a pointer to an Info struct that contains information about the // graphics cards on the host system func New(opts ...*option.Option) (*Info, error) { - ctx := context.New(opts...) + return NewWithContext(context.New(opts...)) +} + +// New returns a pointer to an Info struct that contains information about the +// graphics cards on the host system, reusing a given context. +// Use this function when you want to consume this package from another, +// ensuring the two see a coherent set of resources. +func NewWithContext(ctx *context.Context) (*Info, error) { info := &Info{ctx: ctx} if err := ctx.Do(info.load); err != nil { return nil, err diff --git a/pkg/memory/memory.go b/pkg/memory/memory.go index 93605d2e..54cd19f0 100644 --- a/pkg/memory/memory.go +++ b/pkg/memory/memory.go @@ -34,8 +34,17 @@ type Info struct { Modules []*Module `json:"modules"` } +// New returns a pointer to an Info struct containing information about the +// host's memory. func New(opts ...*option.Option) (*Info, error) { - ctx := context.New(opts...) + return NewWithContext(context.New(opts...)) +} + +// New returns a pointer to an Info struct containing information about the +// host's memory, reusing a given context. +// Use this function when you want to consume this package from another, +// ensuring the two see a coherent set of resources. +func NewWithContext(ctx *context.Context) (*Info, error) { info := &Info{ctx: ctx} if err := ctx.Do(info.load); err != nil { return nil, err diff --git a/pkg/net/net.go b/pkg/net/net.go index 8994d112..d225c79d 100644 --- a/pkg/net/net.go +++ b/pkg/net/net.go @@ -49,7 +49,14 @@ type Info struct { // New returns a pointer to an Info struct that contains information about the // network interface controllers (NICs) on the host system func New(opts ...*option.Option) (*Info, error) { - ctx := context.New(opts...) + return NewWithContext(context.New(opts...)) +} + +// New returns a pointer to an Info struct that contains information about the +// network interface controllers (NICs) on the host system, reusing a given context. +// Use this function when you want to consume this package from another, +// ensuring the two see a coherent set of resources. +func NewWithContext(ctx *context.Context) (*Info, error) { info := &Info{ctx: ctx} if err := ctx.Do(info.load); err != nil { return nil, err diff --git a/pkg/option/option.go b/pkg/option/option.go index b2db7188..90e57658 100644 --- a/pkg/option/option.go +++ b/pkg/option/option.go @@ -14,7 +14,10 @@ import ( ) const ( - defaultChroot = "/" + DefaultChroot = "/" +) + +const ( envKeyChroot = "GHW_CHROOT" envKeyDisableWarnings = "GHW_DISABLE_WARNINGS" envKeyDisableTools = "GHW_DISABLE_TOOLS" @@ -57,7 +60,7 @@ func EnvOrDefaultChroot() string { if val, exists := os.LookupEnv(envKeyChroot); exists { return val } - return defaultChroot + return DefaultChroot } // EnvOrDefaultSnapshotPath returns the value of the GHW_SNAPSHOT_PATH environs variable diff --git a/pkg/pci/pci.go b/pkg/pci/pci.go index c8176e68..36f54243 100644 --- a/pkg/pci/pci.go +++ b/pkg/pci/pci.go @@ -160,18 +160,22 @@ func New(opts ...*option.Option) (*Info, error) { func NewWithContext(ctx *context.Context) (*Info, error) { // by default we don't report NUMA information; // we will only if are sure we are running on NUMA architecture - arch := topology.ARCHITECTURE_SMP - topo, err := topology.NewWithContext(ctx) - if err == nil { - arch = topo.Architecture - } else { - ctx.Warn("error detecting system topology: %v", err) - } info := &Info{ - arch: arch, + arch: topology.ARCHITECTURE_SMP, ctx: ctx, } - if err := ctx.Do(info.load); err != nil { + // we do this trick because we need to make sure ctx.Setup() gets + // a chance to run before any subordinate package is created reusing + // our context. + if err := ctx.Do(func() error { + topo, err := topology.NewWithContext(ctx) + if err == nil { + info.arch = topo.Architecture + } else { + ctx.Warn("error detecting system topology: %v", err) + } + return info.load() + }); err != nil { return nil, err } return info, nil diff --git a/pkg/product/product.go b/pkg/product/product.go index 6a2e1ee0..2daa763b 100644 --- a/pkg/product/product.go +++ b/pkg/product/product.go @@ -73,7 +73,14 @@ func (i *Info) String() string { // New returns a pointer to a Info struct containing information // about the host's product func New(opts ...*option.Option) (*Info, error) { - ctx := context.New(opts...) + return NewWithContext(context.New(opts...)) +} + +// New returns a pointer to a Info struct containing information +// about the host's product, reusing a given context. +// Use this function when you want to consume this package from another, +// ensuring the two see a coherent set of resources. +func NewWithContext(ctx *context.Context) (*Info, error) { info := &Info{ctx: ctx} if err := ctx.Do(info.load); err != nil { return nil, err