-
Notifications
You must be signed in to change notification settings - Fork 182
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
add topology option #354
base: main
Are you sure you want to change the base?
add topology option #354
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -24,7 +24,10 @@ type Context struct { | |
PathOverrides option.PathOverrides | ||
snapshotUnpackedPath string | ||
alert option.Alerter | ||
err error | ||
Err error | ||
DisableNodeCaches bool | ||
DisableNodeAreas bool | ||
DisableNodeDistances bool | ||
} | ||
|
||
// WithContext returns an option.Option that contains a pre-existing Context | ||
|
@@ -83,12 +86,18 @@ func New(opts ...*option.Option) *Context { | |
ctx.PathOverrides = merged.PathOverrides | ||
} | ||
|
||
if merged.TopologyOptions != nil { | ||
ctx.DisableNodeCaches = merged.TopologyOptions.DisableNodeCaches | ||
ctx.DisableNodeAreas = merged.TopologyOptions.DisableNodeAreas | ||
ctx.DisableNodeDistances = merged.TopologyOptions.DisableNodeDistances | ||
} | ||
|
||
// 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) | ||
ctx.Err = fmt.Errorf("Conflicting options: chroot %q and snapshot path %q", ctx.Chroot, ctx.SnapshotPath) | ||
} | ||
return ctx | ||
} | ||
|
@@ -102,11 +111,14 @@ func FromEnv() *Context { | |
snapRootVal := option.EnvOrDefaultSnapshotRoot() | ||
snapExclusiveVal := option.EnvOrDefaultSnapshotExclusive() | ||
return &Context{ | ||
Chroot: chrootVal, | ||
EnableTools: enableTools, | ||
SnapshotPath: snapPathVal, | ||
SnapshotRoot: snapRootVal, | ||
SnapshotExclusive: snapExclusiveVal, | ||
Chroot: chrootVal, | ||
EnableTools: enableTools, | ||
SnapshotPath: snapPathVal, | ||
SnapshotRoot: snapRootVal, | ||
SnapshotExclusive: snapExclusiveVal, | ||
DisableNodeCaches: option.EnvOrDefaultDisableNodeCaches(), | ||
DisableNodeAreas: option.EnvOrDefaultDisableNodeAreas(), | ||
DisableNodeDistances: option.EnvOrDefaultDisableNodeDistances(), | ||
Comment on lines
+119
to
+121
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm inclined to say to keep consistent with other options are handled even though we should later perhaps refactor. But I'd be consistent meantime. |
||
} | ||
} | ||
|
||
|
@@ -131,8 +143,8 @@ 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.Err != nil { | ||
return ctx.Err | ||
} | ||
if ctx.SnapshotPath == "" { | ||
// nothing to do! | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -46,32 +46,37 @@ func topologyNodes(ctx *context.Context) []*Node { | |
nodeID, err := strconv.Atoi(filename[4:]) | ||
if err != nil { | ||
ctx.Warn("failed to determine node ID: %s\n", err) | ||
ctx.Err = err | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this usecase makes sense, but I'd rather add a new function or enhance the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. sorry i did not get your point, could you give a example code please? |
||
return nodes | ||
} | ||
node.ID = nodeID | ||
cores, err := cpu.CoresForNode(ctx, nodeID) | ||
if err != nil { | ||
ctx.Warn("failed to determine cores for node: %s\n", err) | ||
ctx.Err = err | ||
return nodes | ||
} | ||
node.Cores = cores | ||
caches, err := memory.CachesForNode(ctx, nodeID) | ||
if err != nil { | ||
ctx.Warn("failed to determine caches for node: %s\n", err) | ||
ctx.Err = err | ||
return nodes | ||
} | ||
node.Caches = caches | ||
|
||
distances, err := distancesForNode(ctx, nodeID) | ||
if err != nil { | ||
ctx.Warn("failed to determine node distances for node: %s\n", err) | ||
ctx.Err = err | ||
return nodes | ||
} | ||
node.Distances = distances | ||
|
||
area, err := memory.AreaForNode(ctx, nodeID) | ||
if err != nil { | ||
ctx.Warn("failed to determine memory area for node: %s\n", err) | ||
ctx.Err = err | ||
return nodes | ||
} | ||
node.Memory = area | ||
|
@@ -82,6 +87,10 @@ func topologyNodes(ctx *context.Context) []*Node { | |
} | ||
|
||
func distancesForNode(ctx *context.Context, nodeID int) ([]int, error) { | ||
if ctx.DisableNodeDistances { | ||
ctx.Warn("Node distances disabled. Returning empty distances list.") | ||
return nil, nil | ||
} | ||
paths := linuxpath.New(ctx) | ||
path := filepath.Join( | ||
paths.SysDevicesSystemNode, | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we need this (kinda unprecedented) level of detail or can we just have a single option controlling everything?