Skip to content

Commit 63677cf

Browse files
evilhamstermanhellt
evilhamsterman
and
hellt
authored
Fix "clab config not persistent after redeploy" srl-labs#1685 (srl-labs#2642)
* Add tests for expected outcomes * Enforce and suppress should be mutually incompatible * Fix failing tests * swap to kv log * remove unused receiver --------- Co-authored-by: hellt <[email protected]>
1 parent 3735d83 commit 63677cf

File tree

3 files changed

+157
-55
lines changed

3 files changed

+157
-55
lines changed

‎nodes/default_node.go

Lines changed: 33 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -350,41 +350,49 @@ func (d *DefaultNode) VerifyStartupConfig(topoDir string) error {
350350

351351
// GenerateConfig generates configuration for the nodes
352352
// out of the template `t` based on the node configuration and saves the result to dst.
353+
// If the config file is already present in the node dir
354+
// we do not regenerate the config unless EnforceStartupConfig is explicitly set to true and startup-config points to a file
355+
// this will persist the changes that users make to a running config when booted from some startup config
353356
func (d *DefaultNode) GenerateConfig(dst, t string) error {
354-
// If the config file is already present in the node dir
355-
// we do not regenerate the config unless EnforceStartupConfig is explicitly set to true and startup-config points to a file
356-
// this will persist the changes that users make to a running config when booted from some startup config
357+
// Check for incompatible options
358+
if d.Cfg.EnforceStartupConfig && d.Cfg.SuppressStartupConfig {
359+
return ErrIncompatibleOptions
360+
}
361+
if d.Cfg.EnforceStartupConfig && d.Cfg.StartupConfig == "" {
362+
return ErrNoStartupConfig
363+
}
364+
357365
if d.Cfg.SuppressStartupConfig {
358-
log.Infof("Startup config generation for '%s' node suppressed", d.Cfg.ShortName)
359-
return nil
360-
} else if d.Cfg.EnforceStartupConfig {
361-
log.Infof("Startup config for '%s' node enforced: '%s'", d.Cfg.ShortName, dst)
362-
// continue with config generation
363-
} else if utils.FileExists(dst) && d.Cfg.StartupConfig == "" {
364-
log.Infof("config file '%s' for node '%s' already exists and will not be generated/reset", dst, d.Cfg.ShortName)
366+
log.Info("Startup config generation suppressed", "node", d.Cfg.ShortName)
365367
return nil
366368
}
367369

368-
log.Debug("Generating config", "node", d.Cfg.ShortName, "file", d.Cfg.StartupConfig)
370+
if !d.Cfg.EnforceStartupConfig && utils.FileExists(dst) {
371+
log.Debug("Existing config found", "node", d.Cfg.ShortName, "path", dst)
372+
return nil
373+
} else {
369374

370-
cfgBuf, err := utils.SubstituteEnvsAndTemplate(strings.NewReader(t), d.Cfg)
371-
if err != nil {
372-
return err
373-
}
374-
log.Debugf("node '%s' generated config: %s", d.Cfg.ShortName, cfgBuf.String())
375+
log.Debug("Generating config", "node", d.Cfg.ShortName, "file", d.Cfg.StartupConfig)
375376

376-
f, err := os.Create(dst)
377-
if err != nil {
378-
return err
379-
}
377+
cfgBuf, err := utils.SubstituteEnvsAndTemplate(strings.NewReader(t), d.Cfg)
378+
if err != nil {
379+
return err
380+
}
381+
log.Debug("Generated config", "node", d.Cfg.ShortName, "content", cfgBuf.String())
380382

381-
_, err = f.Write(cfgBuf.Bytes())
382-
if err != nil {
383-
f.Close()
384-
return err
383+
f, err := os.Create(dst)
384+
if err != nil {
385+
return err
386+
}
387+
defer f.Close()
388+
389+
_, err = f.Write(cfgBuf.Bytes())
390+
if err != nil {
391+
return err
392+
}
385393
}
386394

387-
return f.Close()
395+
return nil
388396
}
389397

390398
// NodeOverwrites is an interface that every node implements.

‎nodes/default_node_test.go

Lines changed: 120 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package nodes
22

33
import (
4+
"errors"
45
"fmt"
56
"os"
67
"path/filepath"
@@ -13,54 +14,143 @@ import (
1314
)
1415

1516
func TestGenerateConfigs(t *testing.T) {
17+
defCfg := "default config"
18+
oldCfg := "old config"
19+
newCfg := "new config"
20+
1621
tests := map[string]struct {
17-
node *DefaultNode
18-
err error
19-
exists bool
20-
out string
22+
cfg *types.NodeConfig
23+
err error
24+
preExists bool
25+
postExists bool
26+
template string
27+
out string
2128
}{
22-
"suppress-true": {
23-
node: &DefaultNode{
24-
Cfg: &types.NodeConfig{
25-
SuppressStartupConfig: true,
26-
ShortName: "suppress",
27-
},
29+
"suppress-true-first-start": {
30+
cfg: &types.NodeConfig{
31+
SuppressStartupConfig: true,
2832
},
29-
err: nil,
30-
exists: false,
31-
out: "",
33+
err: nil,
34+
preExists: false,
35+
postExists: false,
36+
template: defCfg,
3237
},
33-
"suppress-false": {
34-
node: &DefaultNode{
35-
Cfg: &types.NodeConfig{
36-
SuppressStartupConfig: false,
37-
ShortName: "configure",
38-
},
38+
"suppress-true-existing-lab": {
39+
cfg: &types.NodeConfig{
40+
SuppressStartupConfig: true,
41+
},
42+
err: nil,
43+
preExists: true,
44+
postExists: true,
45+
out: oldCfg,
46+
template: defCfg,
47+
},
48+
"suppress-false-first-start": {
49+
cfg: &types.NodeConfig{
50+
SuppressStartupConfig: false,
51+
},
52+
err: nil,
53+
preExists: false,
54+
postExists: true,
55+
out: defCfg,
56+
template: defCfg,
57+
},
58+
"suppress-false-existing-lab": {
59+
cfg: &types.NodeConfig{
60+
SuppressStartupConfig: false,
61+
},
62+
err: nil,
63+
preExists: true,
64+
postExists: true,
65+
out: oldCfg,
66+
template: defCfg,
67+
},
68+
"startup-config-first-start": {
69+
cfg: &types.NodeConfig{
70+
StartupConfig: "other",
71+
},
72+
err: nil,
73+
preExists: false,
74+
postExists: true,
75+
out: newCfg,
76+
template: newCfg,
77+
},
78+
"startup-config-existing-lab": {
79+
cfg: &types.NodeConfig{
80+
StartupConfig: "other",
3981
},
40-
err: nil,
41-
exists: true,
42-
out: "foo",
82+
err: nil,
83+
preExists: true,
84+
postExists: true,
85+
out: oldCfg,
86+
template: newCfg,
87+
},
88+
"enforce-startup-config-first-start": {
89+
cfg: &types.NodeConfig{
90+
StartupConfig: "other",
91+
EnforceStartupConfig: true,
92+
},
93+
err: nil,
94+
preExists: false,
95+
postExists: true,
96+
out: newCfg,
97+
template: newCfg,
98+
},
99+
"enforce-startup-config-existing-lab": {
100+
cfg: &types.NodeConfig{
101+
StartupConfig: "other",
102+
EnforceStartupConfig: true,
103+
},
104+
err: nil,
105+
preExists: true,
106+
postExists: true,
107+
out: newCfg,
108+
template: newCfg,
109+
},
110+
"enforce-startup-config-no-startup": {
111+
cfg: &types.NodeConfig{
112+
EnforceStartupConfig: true,
113+
},
114+
err: ErrNoStartupConfig,
115+
},
116+
"enforce-and-suppress-startup-config": {
117+
cfg: &types.NodeConfig{
118+
EnforceStartupConfig: true,
119+
SuppressStartupConfig: true,
120+
},
121+
err: ErrIncompatibleOptions,
43122
},
44123
}
45124
for name, tc := range tests {
46125
t.Run(name, func(tt *testing.T) {
126+
node := &DefaultNode{Cfg: tc.cfg}
47127
dstFolder := tt.TempDir()
48128
dstFile := filepath.Join(dstFolder, "config")
49-
err := tc.node.GenerateConfig(dstFile, "foo")
50-
if err != tc.err {
51-
t.Errorf("got %v, wanted %v", err, tc.err)
129+
130+
if tc.preExists {
131+
err := os.WriteFile(dstFile, []byte(oldCfg), 0666)
132+
if err != nil {
133+
tt.Errorf("Could not write existing config: %v", err)
134+
}
52135
}
53-
if tc.exists {
136+
137+
err := node.GenerateConfig(dstFile, tc.template)
138+
if tc.err != nil {
139+
if !errors.Is(err, tc.err) {
140+
tt.Errorf("got: %v, wanted: %v", err, tc.err)
141+
}
142+
}
143+
if tc.postExists {
54144
cnt, err := os.ReadFile(dstFile)
55145
if err != nil {
56-
t.Fatal(err)
146+
tt.Fatal(err)
57147
}
58148
if string(cnt) != tc.out {
59-
t.Errorf("got %v, wanted %v", string(cnt), tc.out)
149+
tt.Errorf("got %v, wanted %v", string(cnt), tc.out)
60150
}
61151
} else {
62152
if _, err := os.Stat(dstFile); err == nil {
63-
t.Errorf("config file created")
153+
tt.Errorf("config file created")
64154
}
65155
}
66156
})
@@ -235,7 +325,7 @@ func TestInterfacesAliases(t *testing.T) { // skipcq: GO-R1005
235325
}
236326

237327
for name, tc := range tests {
238-
t.Run(name, func(tt *testing.T) {
328+
t.Run(name, func(*testing.T) {
239329
foundError := false
240330
tc.node.OverwriteNode = tc.node
241331
tc.node.InterfaceMappedPrefix = "eth"

‎nodes/node.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,10 @@ var (
3838
ErrCommandExecError = errors.New("command execution error")
3939
// ErrContainersNotFound indicated that for a given node no containers where found in the runtime.
4040
ErrContainersNotFound = errors.New("containers not found")
41+
// ErrNoStartupConfig indicates that we are supposed to enforce a startup config but none are provided
42+
ErrNoStartupConfig = errors.New("no startup-config provided")
43+
// ErrIncompatibleOptions for options that are mutually exclusive
44+
ErrIncompatibleOptions = errors.New("incompatible options")
4145
)
4246

4347
// SetNonDefaultRuntimePerKind sets a non default runtime for kinds that requires that (see cvx).

0 commit comments

Comments
 (0)