Skip to content

Commit 89a316f

Browse files
chdeshpa-huecursoragent
chdeshpa-hue
and
cursoragent
authored
OCPBUGS-86311: fix: validate agent-config interface names match networkConfig (#10567)
* fix: validate agent-config interface names match networkConfig openshift-install agent does not cross-validate that interface names in hosts[].interfaces[] match the names used in hosts[].networkConfig. When names mismatch, the pre-network-manager-config.sh script silently fails to rename .nmconnection files at boot time, causing complete network failure for bond/VLAN/bridge topologies with no diagnostic. Add validateInterfaceNamesMatchNetworkConfig() to validateAgentHosts() that ensures every interfaces[].name exists in the networkConfig interfaces list. The error message lists valid networkConfig names to guide users toward the correct configuration. Only the agent-config.yaml path is affected; install-config.yaml derives interface names from networkConfig automatically, so names always match. Co-authored-by: Cursor <cursoragent@cursor.com> * fix: change interface name validation to warning per reviewer feedback Addresses @zaneb's review: the interface name cross-check against networkConfig is now a warning (logrus.Warnf) instead of a hard error, and runs for both agent-config.yaml and nodes-config.yaml (oc adm node-image create) paths — but never for install-config baremetal hosts where getInstallConfigDefaults generates synthetic interface names. This ensures the baremetal IPI fallback path (which generates a bogus "boot" interface name) is never affected, while giving users early visibility into potential name mismatches that could cause connectivity failures at boot time. Test coverage added for: - install-config inert path (no warning fires) - AddNodes workflow mismatch (warns) and match (no warning) - empty interface name (skipped gracefully) - malformed networkConfig YAML (no panic) - bond interfaces matching and mismatching Ref: OCPBUGS-86420 Co-authored-by: Cursor <cursoragent@cursor.com> * fix: inline interface-name warning at call site, remove struct flag Address reviewer feedback: instead of a hostsFromAgentConfig bool flag checked inside the generic validation loop, call the warning directly in Generate() at the two branches where user-authored hosts are already known (agent-config and AddNodes paths). The install-config fallback path (baremetal IPI with synthetic "boot" interface name) never reaches the warning call, so no guard is needed. Co-authored-by: Cursor <cursoragent@cursor.com> * fix: include field path in interface-name warning, restore comment Address zaneb's Jun 9 review: 1. Restore "Hosts defined in agent-config take precedence" comment that was inadvertently dropped in the previous commit. 2. Include the full field path (e.g. hosts[0].interfaces[1].name) in the warning message so users can identify which host and interface has the mismatch, matching the pattern used by validation errors elsewhere in this file. Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent 64847fc commit 89a316f

2 files changed

Lines changed: 272 additions & 7 deletions

File tree

‎pkg/asset/agent/agentconfig/agenthosts.go‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,9 @@ func (a *AgentHosts) Generate(_ context.Context, dependencies asset.Parents) err
7676
if len(a.Hosts) > 0 {
7777
// Hosts defined in agent-config take precedence
7878
logrus.Debugf("Using hosts from %s", agentConfigFilename)
79+
for i, host := range a.Hosts {
80+
warnInterfaceNamesNotInNetworkConfig(host, i)
81+
}
7982
}
8083
}
8184

@@ -92,6 +95,9 @@ func (a *AgentHosts) Generate(_ context.Context, dependencies asset.Parents) err
9295

9396
case workflow.AgentWorkflowTypeAddNodes:
9497
a.Hosts = append(a.Hosts, addNodesConfig.Config.Hosts...)
98+
for i, host := range a.Hosts {
99+
warnInterfaceNamesNotInNetworkConfig(host, i)
100+
}
95101

96102
default:
97103
return fmt.Errorf("AgentWorkflowType value not supported: %s", agentWorkflow.Workflow)
@@ -171,6 +177,42 @@ func (a *AgentHosts) validateHostInterfaces(hostPath *field.Path, host agent.Hos
171177
return allErrs
172178
}
173179

180+
func warnInterfaceNamesNotInNetworkConfig(host agent.Host, hostIdx int) {
181+
if len(host.NetworkConfig.Raw) == 0 || len(host.Interfaces) == 0 {
182+
return
183+
}
184+
185+
var netInterfaces nmStateInterface
186+
if err := yaml.Unmarshal(host.NetworkConfig.Raw, &netInterfaces); err != nil {
187+
return
188+
}
189+
190+
ncNames := make(map[string]bool, len(netInterfaces.Interfaces))
191+
var ncNameList []string
192+
for _, iface := range netInterfaces.Interfaces {
193+
if iface.Name != "" {
194+
ncNames[iface.Name] = true
195+
ncNameList = append(ncNameList, iface.Name)
196+
}
197+
}
198+
if len(ncNames) == 0 {
199+
return
200+
}
201+
202+
hostPath := field.NewPath("hosts").Index(hostIdx)
203+
for i, iface := range host.Interfaces {
204+
if iface.Name == "" {
205+
continue
206+
}
207+
if !ncNames[iface.Name] {
208+
ifacePath := hostPath.Child("interfaces").Index(i).Child("name")
209+
logrus.Warnf("%s: interface name %q not found in networkConfig interfaces %v; "+
210+
"connectivity may fail if interface names do not match at boot time",
211+
ifacePath, iface.Name, ncNameList)
212+
}
213+
}
214+
}
215+
174216
func (a *AgentHosts) validateHostRootDeviceHints(hostPath *field.Path, host agent.Host) field.ErrorList {
175217
rdhPath := hostPath.Child("rootDeviceHints")
176218
allErrs := validation.ValidateHostRootDeviceHints(&host.RootDeviceHints, rdhPath)

‎pkg/asset/agent/agentconfig/agenthosts_test.go‎

Lines changed: 230 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,30 @@ routes:
100100
next-hop-address: 192.168.111.126
101101
next-hop-interface: eth0
102102
table-id: 254
103+
`
104+
agentNetworkConfigBond = `interfaces:
105+
- name: eth0
106+
type: ethernet
107+
state: up
108+
mac-address: 28:d2:44:d2:b2:1a
109+
- name: eth1
110+
type: ethernet
111+
state: up
112+
mac-address: 28:d2:44:d2:b2:1b
113+
- name: bond0
114+
type: bond
115+
state: up
116+
link-aggregation:
117+
mode: active-backup
118+
port:
119+
- eth0
120+
- eth1
121+
ipv4:
122+
enabled: true
123+
dhcp: false
124+
address:
125+
- ip: 192.168.111.80
126+
prefix-length: 24
103127
`
104128
)
105129

@@ -174,8 +198,8 @@ func TestAgentHosts_Generate(t *testing.T) {
174198
getAgentConfigMultiHost("worker"),
175199
},
176200
expectedConfig: agentHosts().hosts(
177-
agentHost().name("test").role("master").interfaces(iface("enp3s1", "28:d2:44:d2:b2:1a")).deviceHint().networkConfig(agentNetworkConfigOne),
178-
agentHost().name("test-2").role("worker").interfaces(iface("enp3s1", "28:d2:44:d2:b2:1b")).networkConfig(agentNetworkConfigTwo)),
201+
agentHost().name("test").role("master").interfaces(iface("eth0", "28:d2:44:d2:b2:1a")).deviceHint().networkConfig(agentNetworkConfigOne),
202+
agentHost().name("test-2").role("worker").interfaces(iface("eth0", "28:d2:44:d2:b2:1b")).networkConfig(agentNetworkConfigTwo)),
179203
},
180204
{
181205
name: "multi-host-from-install-config",
@@ -329,8 +353,8 @@ func TestAgentHosts_Generate(t *testing.T) {
329353
getAgentConfigMultiHostEmbeddedRendezvousIP(),
330354
},
331355
expectedConfig: agentHosts().hosts(
332-
agentHost().name("test").role("master").interfaces(iface("enp3s1", "28:d2:44:d2:b2:1a")).deviceHint().networkConfig(agentNetworkConfigEmbeddedRendezvousIPOne),
333-
agentHost().name("test-2").role("worker").interfaces(iface("enp3s1", "28:d2:44:d2:b2:1b")).networkConfig(agentNetworkConfigEmbeddedRendezvousIPTwo)),
356+
agentHost().name("test").role("master").interfaces(iface("eth0", "28:d2:44:d2:b2:1a")).deviceHint().networkConfig(agentNetworkConfigEmbeddedRendezvousIPOne),
357+
agentHost().name("test-2").role("worker").interfaces(iface("eth0", "28:d2:44:d2:b2:1b")).networkConfig(agentNetworkConfigEmbeddedRendezvousIPTwo)),
334358
},
335359
{
336360
name: "multi-host-from-agent-config-with-arbiter",
@@ -341,8 +365,143 @@ func TestAgentHosts_Generate(t *testing.T) {
341365
getAgentConfigMultiHost("arbiter"),
342366
},
343367
expectedConfig: agentHosts().hosts(
344-
agentHost().name("test").role("master").interfaces(iface("enp3s1", "28:d2:44:d2:b2:1a")).deviceHint().networkConfig(agentNetworkConfigOne),
345-
agentHost().name("test-2").role("arbiter").interfaces(iface("enp3s1", "28:d2:44:d2:b2:1b")).networkConfig(agentNetworkConfigTwo)),
368+
agentHost().name("test").role("master").interfaces(iface("eth0", "28:d2:44:d2:b2:1a")).deviceHint().networkConfig(agentNetworkConfigOne),
369+
agentHost().name("test-2").role("arbiter").interfaces(iface("eth0", "28:d2:44:d2:b2:1b")).networkConfig(agentNetworkConfigTwo)),
370+
},
371+
{
372+
name: "interface-name-mismatch-with-networkconfig-warns-only",
373+
dependencies: []asset.Asset{
374+
&workflow.AgentWorkflow{Workflow: workflow.AgentWorkflowTypeInstall},
375+
&joiner.AddNodesConfig{},
376+
getInstallConfigSingleHost(),
377+
getAgentConfigMismatchedInterfaceName(),
378+
},
379+
expectedConfig: agentHosts().hosts(
380+
agentHost().name("test").role("master").interfaces(iface("enp3s0", "28:d2:44:d2:b2:1a")).deviceHint().networkConfig(agentNetworkConfigOne)),
381+
},
382+
{
383+
name: "interface-name-matches-networkconfig",
384+
dependencies: []asset.Asset{
385+
&workflow.AgentWorkflow{Workflow: workflow.AgentWorkflowTypeInstall},
386+
&joiner.AddNodesConfig{},
387+
getInstallConfigSingleHost(),
388+
getAgentConfigMatchingInterfaceName(),
389+
},
390+
expectedConfig: agentHosts().hosts(
391+
agentHost().name("test").role("master").interfaces(iface("eth0", "28:d2:44:d2:b2:1a")).deviceHint().networkConfig(agentNetworkConfigOne)),
392+
},
393+
{
394+
name: "bond-networkconfig-with-matching-interfaces",
395+
dependencies: []asset.Asset{
396+
&workflow.AgentWorkflow{Workflow: workflow.AgentWorkflowTypeInstall},
397+
&joiner.AddNodesConfig{},
398+
getInstallConfigSingleHost(),
399+
getAgentConfigBondMatching(),
400+
},
401+
expectedConfig: agentHosts().hosts(
402+
agentHost().name("test").role("master").interfaces(iface("eth0", "28:d2:44:d2:b2:1a"), iface("eth1", "28:d2:44:d2:b2:1b")).deviceHint().networkConfig(agentNetworkConfigBond)),
403+
},
404+
{
405+
name: "bond-networkconfig-with-mismatched-interfaces-warns-only",
406+
dependencies: []asset.Asset{
407+
&workflow.AgentWorkflow{Workflow: workflow.AgentWorkflowTypeInstall},
408+
&joiner.AddNodesConfig{},
409+
getInstallConfigSingleHost(),
410+
getAgentConfigBondMismatched(),
411+
},
412+
expectedConfig: agentHosts().hosts(
413+
agentHost().name("test").role("master").interfaces(iface("nic1", "28:d2:44:d2:b2:1a"), iface("nic2", "28:d2:44:d2:b2:1b")).deviceHint().networkConfig(agentNetworkConfigBond)),
414+
},
415+
{
416+
name: "install-config-with-networkconfig-no-warning-inert-path",
417+
dependencies: []asset.Asset{
418+
&workflow.AgentWorkflow{Workflow: workflow.AgentWorkflowTypeInstall},
419+
&joiner.AddNodesConfig{},
420+
getInstallConfigWithMismatchedNetworkConfig(),
421+
getNoHostsAgentConfig(),
422+
},
423+
expectedConfig: agentHosts().hosts(
424+
agentHost().name("test").role("master").interfaces(iface("eth0", "28:d2:44:b0:bf:01")).deviceHint().networkConfig(installNetworkConfigOne)),
425+
},
426+
{
427+
name: "add-nodes-interface-mismatch-warns-only",
428+
dependencies: []asset.Asset{
429+
&workflow.AgentWorkflow{Workflow: workflow.AgentWorkflowTypeAddNodes},
430+
&joiner.AddNodesConfig{
431+
Config: joiner.Config{
432+
Hosts: []agent.Host{
433+
{
434+
Hostname: "extra-worker-0",
435+
Role: "worker",
436+
Interfaces: []*aiv1beta1.Interface{
437+
{
438+
Name: "nic0",
439+
MacAddress: "28:d2:44:d2:b2:1a",
440+
},
441+
},
442+
NetworkConfig: aiv1beta1.NetConfig{
443+
Raw: []byte(agentNetworkConfigOne),
444+
},
445+
},
446+
},
447+
},
448+
},
449+
getNoHostsInstallConfig(),
450+
getNoHostsAgentConfig(),
451+
},
452+
expectedConfig: agentHosts().hosts(
453+
agentHost().name("extra-worker-0").role("worker").interfaces(iface("nic0", "28:d2:44:d2:b2:1a")).networkConfig(agentNetworkConfigOne)),
454+
},
455+
{
456+
name: "add-nodes-interface-matches-no-warning",
457+
dependencies: []asset.Asset{
458+
&workflow.AgentWorkflow{Workflow: workflow.AgentWorkflowTypeAddNodes},
459+
&joiner.AddNodesConfig{
460+
Config: joiner.Config{
461+
Hosts: []agent.Host{
462+
{
463+
Hostname: "extra-worker-0",
464+
Role: "worker",
465+
Interfaces: []*aiv1beta1.Interface{
466+
{
467+
Name: "eth0",
468+
MacAddress: "28:d2:44:d2:b2:1a",
469+
},
470+
},
471+
NetworkConfig: aiv1beta1.NetConfig{
472+
Raw: []byte(agentNetworkConfigOne),
473+
},
474+
},
475+
},
476+
},
477+
},
478+
getNoHostsInstallConfig(),
479+
getNoHostsAgentConfig(),
480+
},
481+
expectedConfig: agentHosts().hosts(
482+
agentHost().name("extra-worker-0").role("worker").interfaces(iface("eth0", "28:d2:44:d2:b2:1a")).networkConfig(agentNetworkConfigOne)),
483+
},
484+
{
485+
name: "agent-config-empty-interface-name-skipped",
486+
dependencies: []asset.Asset{
487+
&workflow.AgentWorkflow{Workflow: workflow.AgentWorkflowTypeInstall},
488+
&joiner.AddNodesConfig{},
489+
getInstallConfigSingleHost(),
490+
getAgentConfigEmptyInterfaceName(),
491+
},
492+
expectedConfig: agentHosts().hosts(
493+
agentHost().name("test").role("master").interfaces(iface("", "28:d2:44:d2:b2:1a")).deviceHint().networkConfig(agentNetworkConfigOne)),
494+
},
495+
{
496+
name: "agent-config-malformed-networkconfig-no-panic",
497+
dependencies: []asset.Asset{
498+
&workflow.AgentWorkflow{Workflow: workflow.AgentWorkflowTypeInstall},
499+
&joiner.AddNodesConfig{},
500+
getInstallConfigSingleHost(),
501+
getAgentConfigMalformedNetworkConfig(),
502+
},
503+
expectedConfig: agentHosts().hosts(
504+
agentHost().name("test").role("master").interfaces(iface("eth0", "28:d2:44:d2:b2:1a")).deviceHint().rawNetworkConfig("not: valid: yaml: [[")),
346505
},
347506
}
348507
for _, tc := range cases {
@@ -453,13 +612,14 @@ func getAgentConfigSingleHost() *AgentConfig {
453612

454613
func getAgentConfigMultiHost(role string) *AgentConfig {
455614
a := getAgentConfigSingleHost()
615+
a.Config.Hosts[0].Interfaces[0].Name = "eth0"
456616
a.Config.Hosts[0].NetworkConfig.Raw = []byte(agentNetworkConfigOne)
457617
host := agent.Host{
458618
Hostname: "test-2",
459619
Role: role,
460620
Interfaces: []*aiv1beta1.Interface{
461621
{
462-
Name: "enp3s1",
622+
Name: "eth0",
463623
MacAddress: "28:d2:44:d2:b2:1b",
464624
},
465625
},
@@ -528,6 +688,26 @@ func getAgentConfigInvalidInterfaces() *AgentConfig {
528688
return a
529689
}
530690

691+
func getAgentConfigBondMatching() *AgentConfig {
692+
a := getAgentConfigSingleHost()
693+
a.Config.Hosts[0].Interfaces = []*aiv1beta1.Interface{
694+
{Name: "eth0", MacAddress: "28:d2:44:d2:b2:1a"},
695+
{Name: "eth1", MacAddress: "28:d2:44:d2:b2:1b"},
696+
}
697+
a.Config.Hosts[0].NetworkConfig.Raw = unmarshalJSON([]byte(agentNetworkConfigBond))
698+
return a
699+
}
700+
701+
func getAgentConfigBondMismatched() *AgentConfig {
702+
a := getAgentConfigSingleHost()
703+
a.Config.Hosts[0].Interfaces = []*aiv1beta1.Interface{
704+
{Name: "nic1", MacAddress: "28:d2:44:d2:b2:1a"},
705+
{Name: "nic2", MacAddress: "28:d2:44:d2:b2:1b"},
706+
}
707+
a.Config.Hosts[0].NetworkConfig.Raw = unmarshalJSON([]byte(agentNetworkConfigBond))
708+
return a
709+
}
710+
531711
func getAgentConfigMissingInterfaces() *AgentConfig {
532712
a := getNoHostsAgentConfig()
533713
a.Config.Hosts = []agent.Host{
@@ -547,6 +727,20 @@ func getAgentConfigMissingInterfaces() *AgentConfig {
547727
return a
548728
}
549729

730+
func getAgentConfigMismatchedInterfaceName() *AgentConfig {
731+
a := getAgentConfigSingleHost()
732+
a.Config.Hosts[0].Interfaces[0].Name = "enp3s0"
733+
a.Config.Hosts[0].NetworkConfig.Raw = []byte(agentNetworkConfigOne)
734+
return a
735+
}
736+
737+
func getAgentConfigMatchingInterfaceName() *AgentConfig {
738+
a := getAgentConfigSingleHost()
739+
a.Config.Hosts[0].Interfaces[0].Name = "eth0"
740+
a.Config.Hosts[0].NetworkConfig.Raw = []byte(agentNetworkConfigOne)
741+
return a
742+
}
743+
550744
func getAgentConfigInvalidRendezvousIP() *AgentConfig {
551745
a := getAgentConfigMultiHost("worker")
552746
a.Config.RendezvousIP = "192.168.111.81"
@@ -728,6 +922,13 @@ func (hb *HostBuilder) networkConfig(raw string) *HostBuilder {
728922
return hb
729923
}
730924

925+
func (hb *HostBuilder) rawNetworkConfig(raw string) *HostBuilder {
926+
hb.Host.NetworkConfig = aiv1beta1.NetConfig{
927+
Raw: []byte(raw),
928+
}
929+
return hb
930+
}
931+
731932
func (hb *HostBuilder) deviceHint() *HostBuilder {
732933
hb.Host.RootDeviceHints = baremetal.RootDeviceHints{
733934
DeviceName: "/dev/sda",
@@ -753,3 +954,25 @@ func iface(name string, mac string) *InterfacetBuilder {
753954
func (ib *InterfacetBuilder) build() *aiv1beta1.Interface {
754955
return &ib.Interface
755956
}
957+
958+
func getInstallConfigWithMismatchedNetworkConfig() *agentAsset.OptionalInstallConfig {
959+
a := getInstallConfigSingleHost()
960+
a.Config.Platform.BareMetal.Hosts[0].NetworkConfig = &apiextv1.JSON{
961+
Raw: []byte(installNetworkConfigOne),
962+
}
963+
return a
964+
}
965+
966+
func getAgentConfigEmptyInterfaceName() *AgentConfig {
967+
a := getAgentConfigSingleHost()
968+
a.Config.Hosts[0].Interfaces[0].Name = ""
969+
a.Config.Hosts[0].NetworkConfig.Raw = []byte(agentNetworkConfigOne)
970+
return a
971+
}
972+
973+
func getAgentConfigMalformedNetworkConfig() *AgentConfig {
974+
a := getAgentConfigSingleHost()
975+
a.Config.Hosts[0].Interfaces[0].Name = "eth0"
976+
a.Config.Hosts[0].NetworkConfig.Raw = []byte("not: valid: yaml: [[")
977+
return a
978+
}

0 commit comments

Comments
 (0)