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

snapshot: initial support to collect data pertaining to network interfaces #227

Merged
merged 2 commits into from
Feb 23, 2021
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
12 changes: 12 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -1278,6 +1278,18 @@ memory:
total_usable_bytes: 25263415296
```

## Calling external programs

By default ghw may call external programs, for example `ethtool` to learn about hardware capabilities.
In some rare circumstances it may be useful to opt out from this behaviour and rely only on the data
provided by pseudo-filesystems, like sysfs.
The most common use case is when we want to consume a snapshot from ghw. In these cases the information
provided by tools will be most likely inconsistent with the data from the snapshot - they will run on
a different host!
To make ghw not calling external tools, set the environs variable `GHW_DISABLE_TOOLS` to any value,
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/To make ghw not calling external tools/To prevent ghw from calling external tools/

or, programmatically, check the `WithDisableTools` function.
The default behaviour of ghw is to call external tools when available.

## Developers

Contributions to `ghw` are welcomed! Fork the repo on GitHub and submit a pull
Expand Down
1 change: 1 addition & 0 deletions alias.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ var (
WithNullAlterter = option.WithNullAlerter
// match the existing environ variable to minimize surprises
WithDisableWarnings = option.WithNullAlerter
WithDisableTools = option.WithDisableTools
)

type SnapshotOptions = option.SnapshotOptions
Expand Down
7 changes: 7 additions & 0 deletions pkg/context/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
// context when calling internal discovery methods
type Context struct {
Chroot string
EnableTools bool
SnapshotPath string
SnapshotRoot string
SnapshotExclusive bool
Expand Down Expand Up @@ -42,18 +43,24 @@ func New(opts ...*option.Option) *Context {
ctx.alert = merged.Alerter
}

if merged.EnableTools != nil {
ctx.EnableTools = *merged.EnableTools
}

return ctx
}

// FromEnv returns an Option that has been populated from the environs or
// default options values
func FromEnv() *Context {
chrootVal := option.EnvOrDefaultChroot()
enableTools := option.EnvOrDefaultTools()
snapPathVal := option.EnvOrDefaultSnapshotPath()
snapRootVal := option.EnvOrDefaultSnapshotRoot()
snapExclusiveVal := option.EnvOrDefaultSnapshotExclusive()
return &Context{
Chroot: chrootVal,
EnableTools: enableTools,
SnapshotPath: snapPathVal,
SnapshotRoot: snapRootVal,
SnapshotExclusive: snapExclusiveVal,
Expand Down
12 changes: 8 additions & 4 deletions pkg/net/net_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,14 @@ func nics(ctx *context.Context) []*NIC {
return nics
}

etInstalled := ethtoolInstalled()
if !etInstalled {
ctx.Warn(_WARN_ETHTOOL_NOT_INSTALLED)
etAvailable := ctx.EnableTools
if etAvailable {
if etInstalled := ethtoolInstalled(); !etInstalled {
ctx.Warn(_WARN_ETHTOOL_NOT_INSTALLED)
etAvailable = false
}
}

for _, file := range files {
filename := file.Name()
// Ignore loopback...
Expand All @@ -62,7 +66,7 @@ func nics(ctx *context.Context) []*NIC {

mac := netDeviceMacAddress(paths, filename)
nic.MacAddress = mac
if etInstalled {
if etAvailable {
nic.Capabilities = netDeviceCapabilities(ctx, filename)
} else {
nic.Capabilities = []*NICCapability{}
Expand Down
29 changes: 29 additions & 0 deletions pkg/option/option.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ const (
defaultChroot = "/"
envKeyChroot = "GHW_CHROOT"
envKeyDisableWarnings = "GHW_DISABLE_WARNINGS"
envKeyDisableTools = "GHW_DISABLE_TOOLS"
envKeySnapshotPath = "GHW_SNAPSHOT_PATH"
envKeySnapshotRoot = "GHW_SNAPSHOT_ROOT"
envKeySnapshotExclusive = "GHW_SNAPSHOT_EXCLUSIVE"
Expand Down Expand Up @@ -95,6 +96,17 @@ func EnvOrDefaultSnapshotPreserve() bool {
return false
}

// EnvOrDefaultTools return true if ghw should use external tools to augment the data collected
// from sysfs. Most users want to do this most of time, so this is enabled by default.
// Users consuming snapshots may want to opt out, thus they can set the GHW_DISABLE_TOOLS
// environs variable to any value to make ghw skip calling external tools even if they are available.
func EnvOrDefaultTools() bool {
if _, exists := os.LookupEnv(envKeyDisableTools); exists {
return false
}
return true
}

// Option is used to represent optionally-configured settings. Each field is a
// pointer to some concrete value so that we can tell when something has been
// set or left unset.
Expand All @@ -113,6 +125,10 @@ type Option struct {

// Alerter contains the target for ghw warnings
Alerter Alerter

// EnableTools optionally request ghw to not call any external program to learn
// about the hardware. The default is to use such tools if available.
EnableTools *bool
}

// SnapshotOptions contains options for handling of ghw snapshots
Expand Down Expand Up @@ -161,6 +177,12 @@ func WithNullAlerter() *Option {
}
}

// WithDisableTools sets enables or prohibts ghw to call external tools to discover hardware capabilities.
func WithDisableTools() *Option {
false_ := false
return &Option{EnableTools: &false_}
}

// There is intentionally no Option related to GHW_SNAPSHOT_PRESERVE because we see that as
// a debug/troubleshoot aid more something users wants to do regularly.
// Hence we allow that only via the environment variable for the time being.
Expand All @@ -177,6 +199,9 @@ func Merge(opts ...*Option) *Option {
if opt.Alerter != nil {
merged.Alerter = opt.Alerter
}
if opt.EnableTools != nil {
merged.EnableTools = opt.EnableTools
}
}
// Set the default value if missing from mergeOpts
if merged.Chroot == nil {
Expand All @@ -194,5 +219,9 @@ func Merge(opts ...*Option) *Option {
Exclusive: EnvOrDefaultSnapshotExclusive(),
}
}
if merged.EnableTools == nil {
enabled := EnvOrDefaultTools()
merged.EnableTools = &enabled
}
return merged
}
26 changes: 25 additions & 1 deletion pkg/option/option_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ func TestOption(t *testing.T) {
option.WithChroot("/my/chroot/dir/2"),
},
merged: &option.Option{
Chroot: stringPtr("/my/chroot/dir/2"),
Chroot: stringPtr("/my/chroot/dir/2"),
EnableTools: boolPtr(true),
},
},
{
Expand Down Expand Up @@ -106,6 +107,17 @@ func TestOption(t *testing.T) {
},
},
},
{
name: "chroot and disabling tools",
opts: []*option.Option{
option.WithChroot("/my/chroot/dir"),
option.WithDisableTools(),
},
merged: &option.Option{
Chroot: stringPtr("/my/chroot/dir"),
EnableTools: boolPtr(false),
},
},
}
for _, optTCase := range optTCases {
t.Run(optTCase.name, func(t *testing.T) {
Expand All @@ -121,6 +133,10 @@ func stringPtr(s string) *string {
return &s
}

func boolPtr(b bool) *bool {
return &b
}

func optionEqual(a, b *option.Option) (string, bool) {
if a == nil || b == nil {
return "top-level", false
Expand All @@ -139,6 +155,14 @@ func optionEqual(a, b *option.Option) (string, bool) {
}
return optionSnapshotEqual(a.Snapshot, b.Snapshot)
}
if a.EnableTools != nil {
if b.EnableTools == nil {
return "enabletools ptr", false
}
if *a.EnableTools != *b.EnableTools {
return "enabletools value", false
}
}
return "", true
}

Expand Down
9 changes: 9 additions & 0 deletions pkg/snapshot/clonetree.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,15 @@ func CloneTreeInto(scratchDir string) error {
// ghw cares about. The intended usage of this function is to validate a clone tree,
// checking that the content matches the expectations.
func ExpectedCloneContent() []string {
fileSpecs := ExpectedCloneStaticContent()
fileSpecs = append(fileSpecs, ExpectedCloneNetContent()...)
return fileSpecs
}

// ExpectedCloneStaticContent return a slice of glob patterns which represent the pseudofiles
// ghw cares about, and which are independent from host specific topology or configuration,
// thus are safely represented by a static slice - e.g. they don't need to be discovered at runtime.
func ExpectedCloneStaticContent() []string {
return []string{
"/etc/mtab",
"/proc/cpuinfo",
Expand Down
62 changes: 62 additions & 0 deletions pkg/snapshot/clonetree_net.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
//
// Use and distribution licensed under the Apache license version 2.
//
// See the COPYING file in the root project directory for full text.
//

package snapshot

import (
"io/ioutil"
"os"
"path/filepath"
"strings"
)

const (
sysClassNet = "/sys/class/net"
)

// ExpectedCloneNetContent returns a slice of strings pertaning the network interfaces ghw
ffromani marked this conversation as resolved.
Show resolved Hide resolved
// cares about. We cannot use a static list because we want to filter away the virtual devices,
// which ghw doesn't concerns itself about. So we need to do some runtime discovery.
ffromani marked this conversation as resolved.
Show resolved Hide resolved
// Additionally, we want to make sure to clone the backing device data.
func ExpectedCloneNetContent() []string {
var fileSpecs []string
ifaceEntries := []string{
"addr_assign_type",
// intentionally avoid to clone "address" to avoid to leak any host-idenfifiable data.
}
entries, err := ioutil.ReadDir(sysClassNet)
if err != nil {
// we should not import context, hence we can't Warn()
return fileSpecs
}
for _, entry := range entries {
netName := entry.Name()
netPath := filepath.Join(sysClassNet, netName)
dest, err := os.Readlink(netPath)
if err != nil {
continue
}
if strings.Contains(dest, "devices/virtual/net") {
// there is no point in cloning data for virtual devices,
// becahse ghw concerns itself with HardWare.
ffromani marked this conversation as resolved.
Show resolved Hide resolved
continue
}

// so, first copy the symlink itself
fileSpecs = append(fileSpecs, netPath)

// now we have to clone the content of the actual network interface
// data related (and found into a subdir of) the backing hardware
// device
netIface := filepath.Clean(filepath.Join(sysClassNet, dest))
for _, ifaceEntry := range ifaceEntries {
fileSpecs = append(fileSpecs, filepath.Join(netIface, ifaceEntry))
}

}

return fileSpecs
}