diff --git a/doc/admin/deployment.md b/doc/admin/deployment.md index 1bb443c1752..e90ce3ca3aa 100644 --- a/doc/admin/deployment.md +++ b/doc/admin/deployment.md @@ -176,6 +176,13 @@ host list. devices, taking into account any specified network device class preference (ethernet or infiniband). +Note that there is currently a [bug](https://github.com/pmem/ndctl/issues/130) +in `ndctl` that prevents the NUMA affinity of an nvdimm namespace from being +correctly reported. +This prevents generation of dual engine configs using `dmg config generate` +command on CentOS7 operating systems, the issue has reportedly been fixed on +CentOS8. + #### Certificate Configuration The DAOS security framework relies on certificates to authenticate @@ -1147,3 +1154,5 @@ each storage node. [^3]: [*https://www.open-mpi.org/faq/?category=running\#mpirun-hostfile*](https://www.open-mpi.org/faq/?category=running#mpirun-hostfile) [^4]: https://github.com/daos-stack/daos/tree/master/src/control/README.md + +[^5]: https://github.com/pmem/ndctl/issues/130 diff --git a/doc/release/release_notes_v1_0.md b/doc/release/release_notes_v1_0.md index f143d3995cd..e4491532da3 100644 --- a/doc/release/release_notes_v1_0.md +++ b/doc/release/release_notes_v1_0.md @@ -274,7 +274,9 @@ that has been resolved in CentOS 8. Some DAOS nodes may not report the correct NUMA socket ID when running a "dmg storage scan." This appears to be a regression in the CentOS7.7 kernel rather than an ndctl issue. This is due to different versions of ndctl provisioning different JSON namespace details -(storage scan command reads the "numa_node" field). +(storage scan command reads the "numa_node" field). The regression also affects +the operation of the "dmg config generate" command which isn't able to detect +correct NUMA affinity for the PMem namespaces required for the config. ### PSM over OPA is not fully functional If you must evaluate DAOS on an OPA fabric, we recommend using IP over diff --git a/src/control/cmd/dmg/auto.go b/src/control/cmd/dmg/auto.go index fd3d19de68f..ab4033fd49a 100644 --- a/src/control/cmd/dmg/auto.go +++ b/src/control/cmd/dmg/auto.go @@ -42,10 +42,12 @@ type configGenCmd struct { func (cmd *configGenCmd) Execute(_ []string) error { ctx := context.Background() + cmd.log.Debugf("configGenCmd input control config: %+v", cmd.config) + req := control.ConfigGenerateReq{ NrEngines: cmd.NrEngines, MinNrSSDs: cmd.MinNrSSDs, - HostList: cmd.hostlist, + HostList: cmd.config.HostList, Client: cmd.ctlInvoker, Log: cmd.log, } @@ -68,7 +70,7 @@ func (cmd *configGenCmd) Execute(_ []string) error { resp, err := control.ConfigGenerate(ctx, req) - if resp.Errors() != nil { + if resp != nil && resp.Errors() != nil { // host level errors e.g. unresponsive daos_server process var bld strings.Builder if err := pretty.PrintResponseErrors(resp, &bld); err != nil { diff --git a/src/control/cmd/dmg/main.go b/src/control/cmd/dmg/main.go index b33df638780..acff9c21d20 100644 --- a/src/control/cmd/dmg/main.go +++ b/src/control/cmd/dmg/main.go @@ -223,8 +223,8 @@ func parseOpts(args []string, opts *cliOptions, invoker control.Invoker, log *lo } ctlCfg = control.DefaultConfig() } - if opts.ConfigPath != "" { - log.Debugf("control config loaded from %s", opts.ConfigPath) + if ctlCfg.Path != "" { + log.Debugf("control config loaded from %s", ctlCfg.Path) } if opts.Insecure { @@ -239,7 +239,9 @@ func parseOpts(args []string, opts *cliOptions, invoker control.Invoker, log *lo ctlCmd.setInvoker(invoker) if opts.HostList != "" { if hlCmd, ok := cmd.(hostListSetter); ok { - hlCmd.setHostList(strings.Split(opts.HostList, ",")) + hl := strings.Split(opts.HostList, ",") + hlCmd.setHostList(hl) + ctlCfg.HostList = hl } else { return errors.Errorf("this command does not accept a hostlist parameter (set it in %s or %s)", control.UserConfigPath(), control.SystemConfigPath()) diff --git a/src/control/lib/control/auto.go b/src/control/lib/control/auto.go index a57b3be2c4a..4a19aa04ead 100644 --- a/src/control/lib/control/auto.go +++ b/src/control/lib/control/auto.go @@ -67,7 +67,11 @@ type ( // // Returns API response and error. func ConfigGenerate(ctx context.Context, req ConfigGenerateReq) (*ConfigGenerateResp, error) { - req.Log.Debugf("ConfigGenerate called with request %v", req) + req.Log.Debugf("ConfigGenerate called with request %+v", req) + + if len(req.HostList) == 0 { + return nil, errors.New("no hosts specified") + } nd, hostErrs, err := getNetworkDetails(ctx, req) if err != nil { @@ -89,12 +93,9 @@ func ConfigGenerate(ctx context.Context, req ConfigGenerateReq) (*ConfigGenerate return nil, err } - // TODO: change Validate() to take io.Writer - // if err := cfg.Validate(&req.buf); err != nil { - // return &ConfigGenerateResp{ - // Err: errors.Wrap(err, "validation failed on auto generated config"), - // }, nil - // } + if err := cfg.Validate(req.Log); err != nil { + return nil, errors.Wrap(err, "validation failed on auto generated config") + } return &ConfigGenerateResp{ConfigOut: cfg}, nil } diff --git a/src/control/lib/control/config.go b/src/control/lib/control/config.go index b0fd2832041..7e43bd4772c 100644 --- a/src/control/lib/control/config.go +++ b/src/control/lib/control/config.go @@ -28,6 +28,7 @@ type Config struct { ControlPort int `yaml:"port"` HostList []string `yaml:"hostlist"` TransportConfig *security.TransportConfig `yaml:"transport_config"` + Path string `yaml:"-"` } // DefaultConfig returns a Config populated with default values. Only @@ -82,5 +83,7 @@ func LoadConfig(cfgPath string) (*Config, error) { if err := yaml.UnmarshalStrict(data, cfg); err != nil { return nil, err } + cfg.Path = cfgPath + return cfg, nil } diff --git a/src/control/lib/control/config_test.go b/src/control/lib/control/config_test.go index 276a389921f..e6610e46844 100644 --- a/src/control/lib/control/config_test.go +++ b/src/control/lib/control/config_test.go @@ -7,6 +7,7 @@ package control import ( + "fmt" "io/ioutil" "os" "path" @@ -21,7 +22,12 @@ import ( "github.com/daos-stack/daos/src/control/security" ) -var testCfg = DefaultConfig() +var ( + testCfg = DefaultConfig() + defCfgCmpOpts = []cmp.Option{ + cmpopts.IgnoreUnexported(security.CertificateConfig{}), + } +) func saveConfig(t *testing.T, cfg *Config, cfgPath string) { t.Helper() @@ -64,16 +70,14 @@ func TestControl_LoadSystemConfig(t *testing.T) { restore := setDirs(t, "NONE", tmpDir) defer restore(t) saveConfig(t, testCfg, SystemConfigPath()) + testCfg.Path = fmt.Sprintf("%s/%s", tmpDir, defaultConfigFile) gotCfg, err := LoadConfig("") if err != nil { t.Fatal(err) } - cmpOpts := []cmp.Option{ - cmpopts.IgnoreUnexported(security.CertificateConfig{}), - } - if diff := cmp.Diff(testCfg, gotCfg, cmpOpts...); diff != "" { + if diff := cmp.Diff(testCfg, gotCfg, defCfgCmpOpts...); diff != "" { t.Fatalf("loaded cfg doesn't match (-want, +got):\n%s\n", diff) } } @@ -85,16 +89,14 @@ func TestControl_LoadUserConfig(t *testing.T) { restore := setDirs(t, tmpDir, "NONE") defer restore(t) saveConfig(t, testCfg, UserConfigPath()) + testCfg.Path = fmt.Sprintf("%s/.%s", tmpDir, defaultConfigFile) gotCfg, err := LoadConfig("") if err != nil { t.Fatal(err) } - cmpOpts := []cmp.Option{ - cmpopts.IgnoreUnexported(security.CertificateConfig{}), - } - if diff := cmp.Diff(testCfg, gotCfg, cmpOpts...); diff != "" { + if diff := cmp.Diff(testCfg, gotCfg, defCfgCmpOpts...); diff != "" { t.Fatalf("loaded cfg doesn't match (-want, +got):\n%s\n", diff) } } @@ -107,16 +109,14 @@ func TestControl_LoadSpecifiedConfig(t *testing.T) { defer restore(t) testPath := path.Join(tmpDir, "test.yml") saveConfig(t, testCfg, testPath) + testCfg.Path = testPath gotCfg, err := LoadConfig(testPath) if err != nil { t.Fatal(err) } - cmpOpts := []cmp.Option{ - cmpopts.IgnoreUnexported(security.CertificateConfig{}), - } - if diff := cmp.Diff(testCfg, gotCfg, cmpOpts...); diff != "" { + if diff := cmp.Diff(testCfg, gotCfg, defCfgCmpOpts...); diff != "" { t.Fatalf("loaded cfg doesn't match (-want, +got):\n%s\n", diff) } }