From 387cd0f7be70e90711e9a472d5a96cf443366444 Mon Sep 17 00:00:00 2001 From: Jonathon Anderson Date: Fri, 14 Apr 2023 18:34:03 -0600 Subject: [PATCH] Refactor BaseConf Read and Parse Also refactor New and Get to return pointers to match BaseConf methods. This makes calling the methods immediately on the return values of the constructors easier. Also move config.ConfigFile to buildconfig.go.in. ConfigFile is still used by wwctl as a default config file to reference; but it is removed from other locations now. Removed Persist, as nothing called it. Signed-off-by: Jonathon Anderson --- Makefile | 9 +- .../app/wwctl/container/list/main_test.go | 2 +- internal/app/wwctl/node/add/main_test.go | 2 +- internal/app/wwctl/node/list/main_test.go | 2 +- internal/app/wwctl/profile/list/main_test.go | 2 +- internal/app/wwctl/root.go | 6 +- internal/pkg/config/buildconfig.go.in | 2 + internal/pkg/config/root.go | 85 +++++++------------ 8 files changed, 42 insertions(+), 68 deletions(-) diff --git a/Makefile b/Makefile index a9fc7dd2..1abd6869 100644 --- a/Makefile +++ b/Makefile @@ -151,7 +151,7 @@ vet: go vet ./... test-it: - go test -v ./... -ldflags="-X 'github.com/hpcng/warewulf/internal/pkg/config.ConfigFile=$(shell pwd)/etc/warewulf.conf'" + go test -v ./... # Generate test coverage test-cover: ## Run test coverage and generate html report @@ -219,13 +219,12 @@ init: wwctl: $(WWCTL_DEPS) @echo Building "$@" @cd cmd/wwctl; GOOS=linux go build -mod vendor -tags "$(WW_GO_BUILD_TAGS)" \ - -ldflags "-X 'github.com/hpcng/warewulf/internal/pkg/config.ConfigFile=$(SYSCONFDIR)/warewulf/warewulf.conf'" \ -o ../../wwctl wwclient: $(WWCLIENT_DEPS) @echo Building "$@" - @cd cmd/wwclient; CGO_ENABLED=0 GOOS=linux go build -mod vendor -a -ldflags "-extldflags -static \ - -X 'github.com/hpcng/warewulf/internal/pkg/config.ConfigFile=/etc/warewulf/warewulf.conf'" -o ../../wwclient + @cd cmd/wwclient; CGO_ENABLED=0 GOOS=linux go build -mod vendor -a -ldflags "-extldflags -static" \ + -o ../../wwclient man_pages: wwctl @install -d man_pages @@ -235,7 +234,7 @@ man_pages: wwctl @cd man_pages; for i in wwctl*1 *.5; do gzip --force $$i; echo -n "$$i "; done; echo update_configuration: vendor cmd/update_configuration/update_configuration.go - cd cmd/update_configuration && go build -ldflags="-X 'github.com/hpcng/warewulf/internal/pkg/config.ConfigFile=./etc/warewulf.conf'\ + cd cmd/update_configuration && go build \ -X 'github.com/hpcng/warewulf/internal/pkg/node.ConfigFile=./etc/nodes.conf'"\ -mod vendor -tags "$(WW_GO_BUILD_TAGS)" -o ../../update_configuration diff --git a/internal/app/wwctl/container/list/main_test.go b/internal/app/wwctl/container/list/main_test.go index 2d5afc3a..ab3c880f 100644 --- a/internal/app/wwctl/container/list/main_test.go +++ b/internal/app/wwctl/container/list/main_test.go @@ -56,7 +56,7 @@ WW_INTERNAL: 0 ` conf := warewulfconf.Get() - err := conf.Read([]byte(conf_yml)) + err := conf.Parse([]byte(conf_yml)) assert.NoError(t, err) warewulfd.SetNoDaemon() for _, tt := range tests { diff --git a/internal/app/wwctl/node/add/main_test.go b/internal/app/wwctl/node/add/main_test.go index 95b14873..f0c7e7df 100644 --- a/internal/app/wwctl/node/add/main_test.go +++ b/internal/app/wwctl/node/add/main_test.go @@ -243,7 +243,7 @@ nodes: _, warewulfConfErr = tempWarewulfConf.Write([]byte(conf_yml)) assert.NoError(t, warewulfConfErr) assert.NoError(t, tempWarewulfConf.Sync()) - warewulfconf.ConfigFile = tempWarewulfConf.Name() + warewulfconf.New().Read(tempWarewulfConf.Name()) nodes_yml := `WW_INTERNAL: 43` tempNodeConf, nodesConfErr := os.CreateTemp("", "nodes.conf-") diff --git a/internal/app/wwctl/node/list/main_test.go b/internal/app/wwctl/node/list/main_test.go index 4178f2b0..ebd1ac3b 100644 --- a/internal/app/wwctl/node/list/main_test.go +++ b/internal/app/wwctl/node/list/main_test.go @@ -42,7 +42,7 @@ nodes: _, warewulfConfErr = tempWarewulfConf.Write([]byte(conf_yml)) assert.NoError(t, warewulfConfErr) assert.NoError(t, tempWarewulfConf.Sync()) - warewulfconf.ConfigFile = tempWarewulfConf.Name() + warewulfconf.New().Read(tempWarewulfConf.Name()) tempNodeConf, nodesConfErr := os.CreateTemp("", "nodes.conf-") assert.NoError(t, nodesConfErr) diff --git a/internal/app/wwctl/profile/list/main_test.go b/internal/app/wwctl/profile/list/main_test.go index 4f4efad3..00f6144f 100644 --- a/internal/app/wwctl/profile/list/main_test.go +++ b/internal/app/wwctl/profile/list/main_test.go @@ -43,7 +43,7 @@ nodes: _, warewulfConfErr = tempWarewulfConf.Write([]byte(conf_yml)) assert.NoError(t, warewulfConfErr) assert.NoError(t, tempWarewulfConf.Sync()) - warewulfconf.ConfigFile = tempWarewulfConf.Name() + warewulfconf.New().Read(tempWarewulfConf.Name()) tempNodeConf, nodesConfErr := os.CreateTemp("", "nodes.conf-") assert.NoError(t, nodesConfErr) diff --git a/internal/app/wwctl/root.go b/internal/app/wwctl/root.go index 4d40ccde..282615b2 100644 --- a/internal/app/wwctl/root.go +++ b/internal/app/wwctl/root.go @@ -79,11 +79,11 @@ func rootPersistentPreRunE(cmd *cobra.Command, args []string) (err error) { conf := warewulfconf.Get() if !AllowEmptyConf && !conf.InitializedFromFile() { if WarewulfConfArg != "" { - err = conf.ReadConf(WarewulfConfArg) + err = conf.Read(WarewulfConfArg) } else if os.Getenv("WAREWULFCONF") != "" { - err = conf.ReadConf(os.Getenv("WAREWULFCONF")) + err = conf.Read(os.Getenv("WAREWULFCONF")) } else { - err = conf.ReadConf(warewulfconf.ConfigFile) + err = conf.Read(warewulfconf.ConfigFile) } } else { err = conf.SetDynamicDefaults() diff --git a/internal/pkg/config/buildconfig.go.in b/internal/pkg/config/buildconfig.go.in index eae1071f..5316e518 100644 --- a/internal/pkg/config/buildconfig.go.in +++ b/internal/pkg/config/buildconfig.go.in @@ -1,5 +1,7 @@ package config +var ConfigFile = "@SYSCONFDIR@/warewulf/warewulf.conf" + type BuildConfig struct { Bindir string `default:"@BINDIR@"` Sysconfdir string `default:"@SYSCONFDIR@"` diff --git a/internal/pkg/config/root.go b/internal/pkg/config/root.go index 374f7d7c..89e418e0 100644 --- a/internal/pkg/config/root.go +++ b/internal/pkg/config/root.go @@ -21,10 +21,7 @@ import ( ) -var ( - cachedConf RootConf - ConfigFile string -) +var cachedConf RootConf // RootConf is the main Warewulf configuration structure. It stores @@ -50,65 +47,67 @@ type RootConf struct { } -// New returns a [RootConf] initialized with empty values. -func New() (conf RootConf) { - conf.Warewulf = new(WarewulfConf) - conf.Dhcp = new(DhcpConf) - conf.Tftp = new(TftpConf) - conf.Nfs = new(NfsConf) - conf.Paths = new(BuildConfig) - _ = defaults.Set(&conf) - return +// New caches and returns a new [RootConf] initialized with empty +// values, clearing replacing any previously cached value. +func New() (*RootConf) { + cachedConf = RootConf{} + cachedConf.fromFile = false + cachedConf.Warewulf = new(WarewulfConf) + cachedConf.Dhcp = new(DhcpConf) + cachedConf.Tftp = new(TftpConf) + cachedConf.Nfs = new(NfsConf) + cachedConf.Paths = new(BuildConfig) + if err := defaults.Set(&cachedConf); err != nil { + panic(err) + } + return &cachedConf } -// Get returns a [RootConf] which may have been cached from a previous -// call. -func Get() (RootConf) { +// Get returns a previously cached [RootConf] if it exists, or returns +// a new RootConf. +func Get() (*RootConf) { // NOTE: This function can be called before any log level is set // so using wwlog.Verbose or wwlog.Debug won't work if reflect.ValueOf(cachedConf).IsZero() { - cachedConf = New() - cachedConf.fromFile = false + cachedConf = *New() } - return cachedConf + return &cachedConf } // ReadConf populates [RootConf] with the values from a configuration // file. -func (conf *RootConf) ReadConf(confFileName string) (err error) { +func (conf *RootConf) Read(confFileName string) (error) { wwlog.Debug("Reading warewulf.conf from: %s", confFileName) - fileHandle, err := os.ReadFile(confFileName) + data, err := os.ReadFile(confFileName) if err != nil { return err } - return conf.Read(fileHandle) + return conf.Parse(data) } // Read populates [RootConf] with the values from a yaml document. -func (conf *RootConf) Read(data []byte) (err error) { +func (conf *RootConf) Parse(data []byte) (error) { // ipxe binaries are merged not overwritten, store defaults separate defIpxe := make(map[string]string) for k, v := range conf.Tftp.IpxeBinaries { defIpxe[k] = v delete(conf.Tftp.IpxeBinaries, k) } - err = yaml.Unmarshal(data, &conf) - if err != nil { - return + if err := yaml.Unmarshal(data, &conf); err != nil { + return err } - err = conf.SetDynamicDefaults() - if err != nil { - return + if err := conf.SetDynamicDefaults(); err != nil { + return err } if len(conf.Tftp.IpxeBinaries) == 0 { conf.Tftp.IpxeBinaries = defIpxe } cachedConf = *conf cachedConf.fromFile = true - return + return nil } @@ -185,29 +184,3 @@ func (conf *RootConf) SetDynamicDefaults() (err error) { func (conf *RootConf) InitializedFromFile() bool { return conf.fromFile } - - -// Persist writes [RootConf] to a file as a yaml document. -func (controller *RootConf) Persist() error { - - out, err := yaml.Marshal(controller) - if err != nil { - return err - } - - file, err := os.OpenFile(ConfigFile, os.O_RDWR|os.O_CREATE|os.O_TRUNC, 0644) - if err != nil { - wwlog.Error("%s", err) - os.Exit(1) - } - - defer file.Close() - - _, err = file.WriteString(string(out)+"\n") - if err != nil { - wwlog.Error("Unable to write to warewulf.conf") - return err - } - - return nil -}