Skip to content

Commit b052158

Browse files
openshift-ci[bot]openshift-ci[bot]
authored
Merge pull request openshift#7443 from andfasano/fix-rendezvousip-not-worker-2
OCPBUGS-5471: Try to select rendezvousIP among non-worker hosts
2 parents 2689db6 + 56b4fb2 commit b052158

File tree

5 files changed

+282
-24
lines changed

5 files changed

+282
-24
lines changed

‎cmd/openshift-install/testdata/agent/image/validations/no-rendezvousip.txt‎

Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,115 @@
1+
# Verify that when the rendezvousIP is not defined in the agent-config.yaml, it is then
2+
# selected amongst the non-worker nodes (given that interfaces are properly defined)
3+
4+
exec openshift-install agent create image --dir $WORK
5+
6+
stderr 'The rendezvous host IP \(node0 IP\) is 10.19.17.126'
7+
8+
exists $WORK/agent.x86_64.iso
9+
exists $WORK/auth/kubeconfig
10+
exists $WORK/auth/kubeadmin-password
11+
12+
-- install-config.yaml --
13+
apiVersion: v1
14+
baseDomain: test.metalkube.org
15+
controlPlane:
16+
name: master
17+
replicas: 3
18+
compute:
19+
- name: worker
20+
replicas: 1
21+
metadata:
22+
namespace: cluster0
23+
name: ostest
24+
networking:
25+
clusterNetwork:
26+
- cidr: 10.128.0.0/14
27+
hostPrefix: 23
28+
networkType: OVNKubernetes
29+
machineNetwork:
30+
- cidr: 192.168.111.0/24
31+
serviceNetwork:
32+
- 172.30.0.0/16
33+
platform:
34+
baremetal:
35+
apiVips:
36+
- 192.168.111.5
37+
ingressVips:
38+
- 192.168.111.4
39+
sshKey: ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABgQDK6UTEydcEKzuNdPaofn8Z2DwgHqdcionLZBiPf/zIRNco++etLsat7Avv7yt04DINQd5zjxIFgG8jblaUB5E5C9ClUcMwb52GO0ay2Y9v1uBv1a4WhI3peKktAzYNk0EBMQlJtXPjRMrC9ylBPh+DsBHMu+KmDnfk7PIwyN4efC8k5kSRuPWoNdme1rz2+umU8FSmaWTHIajrbspf4GQbsntA5kuKEtDbfoNCU97o2KrRnUbeg3a8hwSjfh3u6MhlnGcg5K2Ij+zivEsWGCLKYUtE1ErqwfIzwWmJ6jnV66XCQGHf4Q1iIxqF7s2a1q24cgG2Z/iDXfqXrCIfy4P7b/Ztak3bdT9jfAdVZtdO5/r7I+O5hYhF86ayFlDWzZWP/ByiSb+q4CQbfVgK3BMmiAv2MqLHdhesmD/SmIcoOWUF6rFmRKZVFFpKpt5ATNTgUJ3JRowoXrrDruVXClUGRiCS6Zabd1rZ3VmTchaPJwtzQMdfIWISXj+Ig+C4UK0=
40+
pullSecret: '{"auths": {"quay.io": {"auth": "c3VwZXItc2VjcmV0Cg=="}}}'
41+
42+
-- agent-config.yaml --
43+
apiVersion: v1alpha1
44+
metadata:
45+
name: ostest
46+
namespace: cluster0
47+
hosts:
48+
- hostname: worker-0
49+
role: worker
50+
interfaces:
51+
- name: enp1s0
52+
macAddress: aa:bb:dc:dd:a8:04
53+
networkConfig:
54+
interfaces:
55+
- name: enp1s0
56+
type: ethernet
57+
state: up
58+
mac-address: aa:bb:dc:dd:a8:04
59+
ipv4:
60+
enabled: true
61+
address:
62+
- ip: 10.19.17.129
63+
prefix-length: 23
64+
dhcp: false
65+
- hostname: master-0
66+
role: master
67+
interfaces:
68+
- name: enp1s0
69+
macAddress: aa:bb:dc:dd:a8:01
70+
networkConfig:
71+
interfaces:
72+
- name: enp1s0
73+
type: ethernet
74+
state: up
75+
mac-address: aa:bb:dc:dd:a8:01
76+
ipv4:
77+
enabled: true
78+
address:
79+
- ip: 10.19.17.126
80+
prefix-length: 23
81+
dhcp: false
82+
- hostname: master-1
83+
role: master
84+
interfaces:
85+
- name: enp1s0
86+
macAddress: aa:bb:dc:dd:a8:02
87+
networkConfig:
88+
interfaces:
89+
- name: enp1s0
90+
type: ethernet
91+
state: up
92+
mac-address: aa:bb:dc:dd:a8:02
93+
ipv4:
94+
enabled: true
95+
address:
96+
- ip: 10.19.17.127
97+
prefix-length: 23
98+
dhcp: false
99+
- hostname: master-2
100+
role: master
101+
interfaces:
102+
- name: enp1s0
103+
macAddress: aa:bb:dc:dd:a8:03
104+
networkConfig:
105+
interfaces:
106+
- name: enp1s0
107+
type: ethernet
108+
state: up
109+
mac-address: aa:bb:dc:dd:a8:03
110+
ipv4:
111+
enabled: true
112+
address:
113+
- ip: 10.19.17.128
114+
prefix-length: 23
115+
dhcp: false

‎pkg/asset/agent/image/ignition.go‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -518,12 +518,12 @@ func RetrieveRendezvousIP(agentConfig *agent.Config, nmStateConfigs []*v1beta1.N
518518
rendezvousIP = agentConfig.RendezvousIP
519519
logrus.Debug("RendezvousIP from the AgentConfig ", rendezvousIP)
520520

521-
} else if len(nmStateConfigs) > 0 {
522-
rendezvousIP, err = manifests.GetNodeZeroIP(nmStateConfigs)
523-
logrus.Debug("RendezvousIP from the NMStateConfig ", rendezvousIP)
524521
} else {
525-
err = errors.New("missing rendezvousIP in agent-config or at least one NMStateConfig manifest")
526-
return "", err
522+
rendezvousIP, err = manifests.GetNodeZeroIP(agentConfig, nmStateConfigs)
523+
if err != nil {
524+
return "", errors.Wrap(err, "missing rendezvousIP in agent-config, at least one host networkConfig in agent-config, or at least one NMStateConfig manifest")
525+
}
526+
logrus.Debug("RendezvousIP from the NMStateConfig ", rendezvousIP)
527527
}
528528

529529
// Convert IPv6 address to canonical to match host format for comparisons

‎pkg/asset/agent/image/ignition_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -228,7 +228,7 @@ func TestRetrieveRendezvousIP(t *testing.T) {
228228
},
229229
},
230230
},
231-
expectedError: "missing rendezvousIP in agent-config or at least one NMStateConfig manifest",
231+
expectedError: "missing rendezvousIP in agent-config, at least one host networkConfig in agent-config, or at least one NMStateConfig manifest",
232232
},
233233
{
234234
Name: "non-canonical-ipv6-address",

‎pkg/asset/agent/manifests/nmstateconfig.go‎

Lines changed: 54 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import (
2323
"github.com/openshift/installer/pkg/asset/agent/agentconfig"
2424
"github.com/openshift/installer/pkg/asset/agent/manifests/staticnetworkconfig"
2525
"github.com/openshift/installer/pkg/types"
26+
agenttypes "github.com/openshift/installer/pkg/types/agent"
2627
)
2728

2829
var (
@@ -242,41 +243,77 @@ func (n *NMStateConfig) validateNMStateLabels() field.ErrorList {
242243
return allErrs
243244
}
244245

245-
func getFirstIP(nmStateConfig *nmStateConfig) string {
246+
func getFirstIP(nmstateRaw []byte) (string, error) {
247+
var nmStateConfig nmStateConfig
248+
err := yaml.Unmarshal(nmstateRaw, &nmStateConfig)
249+
if err != nil {
250+
return "", fmt.Errorf("error unmarshalling NMStateConfig: %w", err)
251+
}
252+
246253
for _, intf := range nmStateConfig.Interfaces {
247254
for _, addr4 := range intf.IPV4.Address {
248255
if addr4.IP != "" {
249-
return addr4.IP
256+
return addr4.IP, nil
250257
}
251258
}
252259
for _, addr6 := range intf.IPV6.Address {
253260
if addr6.IP != "" {
254-
return addr6.IP
261+
return addr6.IP, nil
255262
}
256263
}
257264
}
258-
return ""
265+
266+
return "", nil
259267
}
260268

261-
// GetNodeZeroIP retrieves the first IP from the user provided NMStateConfigs to set as the node0 IP
262-
func GetNodeZeroIP(nmStateConfigs []*aiv1beta1.NMStateConfig) (string, error) {
263-
for i := range nmStateConfigs {
264-
var nmStateConfig nmStateConfig
265-
err := yaml.Unmarshal(nmStateConfigs[i].Spec.NetConfig.Raw, &nmStateConfig)
266-
if err != nil {
267-
return "", fmt.Errorf("error unmarshalling NMStateConfig: %v", err)
268-
}
269-
if nodeZeroIP := getFirstIP(&nmStateConfig); nodeZeroIP != "" {
270-
if net.ParseIP(nodeZeroIP) == nil {
271-
return "", fmt.Errorf("could not parse static IP: %s", nodeZeroIP)
269+
// GetNodeZeroIP retrieves the first IP to be set as the node0 IP.
270+
// The method prioritizes the search by trying to scan first the NMState configs defined
271+
// in the agent-config hosts - so that it would be possible to skip the worker nodes - and then
272+
// the NMStateConfig.
273+
func GetNodeZeroIP(agentConfig *agenttypes.Config, nmStateConfigs []*aiv1beta1.NMStateConfig) (string, error) {
274+
rawConfigs := []aiv1beta1.RawNetConfig{}
275+
276+
// Select first the configs from the hosts, if defined
277+
if agentConfig != nil {
278+
// Skip worker hosts (or without an explicit role assigned)
279+
for _, host := range agentConfig.Hosts {
280+
if host.Role != "master" {
281+
continue
272282
}
283+
rawConfigs = append(rawConfigs, host.NetworkConfig.Raw)
284+
}
273285

274-
return nodeZeroIP, nil
286+
// Add other hosts without explicit role with a lower
287+
// priority as potential candidates
288+
for _, host := range agentConfig.Hosts {
289+
if host.Role != "" {
290+
continue
291+
}
292+
rawConfigs = append(rawConfigs, host.NetworkConfig.Raw)
275293
}
294+
}
295+
296+
// Fallback on nmstate configs (in case hosts weren't found or didn't have static configuration)
297+
for _, nmStateConfig := range nmStateConfigs {
298+
rawConfigs = append(rawConfigs, nmStateConfig.Spec.NetConfig.Raw)
299+
}
276300

301+
// Try to look for an eligible IP
302+
for _, raw := range rawConfigs {
303+
nodeZeroIP, err := getFirstIP(raw)
304+
if err != nil {
305+
return "", fmt.Errorf("error unmarshalling NMStateConfig: %w", err)
306+
}
307+
if nodeZeroIP == "" {
308+
continue
309+
}
310+
if net.ParseIP(nodeZeroIP) == nil {
311+
return "", fmt.Errorf("could not parse static IP: %s", nodeZeroIP)
312+
}
313+
return nodeZeroIP, nil
277314
}
278315

279-
return "", fmt.Errorf("invalid NMStateConfig yaml, no interface IPs set")
316+
return "", fmt.Errorf("invalid NMState configurations provided, no interface IPs set")
280317
}
281318

282319
// GetNMIgnitionFiles returns the list of NetworkManager configuration files

‎pkg/asset/agent/manifests/nmstateconfig_test.go‎

Lines changed: 107 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import (
1515
"github.com/openshift/assisted-service/models"
1616
"github.com/openshift/installer/pkg/asset"
1717
"github.com/openshift/installer/pkg/asset/mock"
18+
"github.com/openshift/installer/pkg/types/agent"
1819
)
1920

2021
func TestNMStateConfig_Generate(t *testing.T) {
@@ -473,6 +474,7 @@ func TestGetNodeZeroIP(t *testing.T) {
473474
expectedIP string
474475
expectedError string
475476
configs []string
477+
hosts []agent.Host
476478
}{
477479
{
478480
name: "no interfaces",
@@ -608,6 +610,110 @@ interfaces:
608610
`,
609611
},
610612
},
613+
{
614+
name: "skip workers/nodes without role",
615+
expectedIP: "192.168.122.22",
616+
hosts: []agent.Host{
617+
{
618+
Role: "worker",
619+
NetworkConfig: aiv1beta1.NetConfig{Raw: []byte(`
620+
interfaces:
621+
- name: eth0
622+
type: ethernet
623+
ipv4:
624+
address:
625+
- ip: 192.168.122.31`)},
626+
},
627+
{
628+
Role: "",
629+
NetworkConfig: aiv1beta1.NetConfig{Raw: []byte(`
630+
interfaces:
631+
- name: eth0
632+
type: ethernet
633+
ipv4:
634+
address:
635+
- ip: 192.168.122.32`)},
636+
},
637+
{
638+
Role: "master",
639+
NetworkConfig: aiv1beta1.NetConfig{Raw: []byte(`
640+
interfaces:
641+
- name: eth0
642+
type: ethernet
643+
ipv4:
644+
address:
645+
- ip: 192.168.122.22`)},
646+
},
647+
},
648+
},
649+
{
650+
name: "fail if only workers",
651+
expectedError: "invalid NMState configurations provided, no interface IPs set",
652+
hosts: []agent.Host{
653+
{
654+
Role: "worker",
655+
NetworkConfig: aiv1beta1.NetConfig{Raw: []byte(`
656+
interfaces:
657+
- name: eth0
658+
type: ethernet
659+
ipv4:
660+
address:
661+
- ip: 192.168.122.31`)},
662+
},
663+
},
664+
},
665+
{
666+
name: "fail if only master without static configuration",
667+
expectedError: "invalid NMState configurations provided, no interface IPs set",
668+
hosts: []agent.Host{
669+
{
670+
Role: "master",
671+
},
672+
},
673+
},
674+
{
675+
name: "fallback on configs if missing host definition",
676+
expectedIP: "192.168.122.22",
677+
hosts: []agent.Host{
678+
{
679+
Role: "master",
680+
},
681+
},
682+
configs: []string{`
683+
interfaces:
684+
- name: eth0
685+
type: ethernet
686+
ipv4:
687+
address:
688+
- ip: 192.168.122.22`,
689+
},
690+
},
691+
{
692+
name: "implicit masters",
693+
expectedIP: "192.168.122.32",
694+
hosts: []agent.Host{
695+
{
696+
Role: "worker",
697+
NetworkConfig: aiv1beta1.NetConfig{Raw: []byte(`
698+
interfaces:
699+
- name: eth0
700+
type: ethernet
701+
ipv4:
702+
address:
703+
- ip: 192.168.122.31`)},
704+
},
705+
{
706+
Role: "",
707+
NetworkConfig: aiv1beta1.NetConfig{Raw: []byte(`
708+
interfaces:
709+
- name: eth0
710+
type: ethernet
711+
ipv4:
712+
address:
713+
- ip: 192.168.122.32`)},
714+
},
715+
},
716+
},
611717
}
612718
for _, tc := range cases {
613719
t.Run(tc.name, func(t *testing.T) {
@@ -622,7 +728,7 @@ interfaces:
622728
})
623729
}
624730

625-
ip, err := GetNodeZeroIP(configs)
731+
ip, err := GetNodeZeroIP(&agent.Config{Hosts: tc.hosts}, configs)
626732
if tc.expectedError == "" {
627733
assert.NoError(t, err)
628734
assert.Equal(t, tc.expectedIP, ip)

0 commit comments

Comments
 (0)