Skip to content

Commit 3f6c604

Browse files
rabirabi
committed
Validate ansibleHost and default ctlplane fixedIP for pre-provisioned nodes
For pre-provisioned dataplane nodes, ansibleHost must be a valid IP address. This is required because the controller defaults the ctlplane network fixedIP from ansibleHost during IPAM reservation, ensuring the reserved IP matches the already-configured node interface. Without a valid IP, IPAM could reserve a different address and break connectivity during deployment. The validating webhook enforces that for pre-provisioned nodes: - ansibleHost is not empty - ansibleHost is a valid IP address The controller (ipam.go) defaults the ctlplane fixedIP from ansibleHost when not already set. The ctlplane network is identified using the netServiceNetMap from NetConfig, so it works regardless of the network name. Signed-off-by: rabi <ramishra@redhat.com>
1 parent 15fdbf6 commit 3f6c604

14 files changed

Lines changed: 158 additions & 27 deletions

File tree

‎api/dataplane/v1beta1/openstackdataplanenodeset_webhook.go‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ package v1beta1
1919
import (
2020
"context"
2121
"fmt"
22+
"net"
2223
"reflect"
2324
"strings"
2425

@@ -133,6 +134,10 @@ func (r *OpenStackDataPlaneNodeSet) validateNodes(ctx context.Context, c client.
133134
errors = append(errors, r.Spec.duplicateNodeCheck(nodeSetList, r.ObjectMeta.Name)...)
134135
}
135136

137+
if r.Spec.PreProvisioned {
138+
errors = append(errors, r.Spec.validatePreProvisionedNodes()...)
139+
}
140+
136141
return errors, nil
137142

138143
}
@@ -227,3 +232,32 @@ func (spec *OpenStackDataPlaneNodeSetSpec) ValidateDelete() field.ErrorList {
227232
return field.ErrorList{}
228233

229234
}
235+
236+
// validatePreProvisionedNodes validates that ansibleHost is a valid IP
237+
// for pre-provisioned nodes. An IP is required so that the controller
238+
// can default the ctlplane fixedIP from it, ensuring IPAM reserves the
239+
// correct address that matches the already-configured node interface.
240+
func (spec *OpenStackDataPlaneNodeSetSpec) validatePreProvisionedNodes() field.ErrorList {
241+
var errors field.ErrorList
242+
243+
for nodeName, node := range spec.Nodes {
244+
nodePath := field.NewPath("spec").Child("nodes").Key(nodeName)
245+
246+
if node.Ansible.AnsibleHost == "" {
247+
errors = append(errors, field.Required(
248+
nodePath.Child("ansible", "ansibleHost"),
249+
"ansibleHost must be a valid IP address for "+
250+
"pre-provisioned nodes"))
251+
continue
252+
}
253+
254+
if net.ParseIP(node.Ansible.AnsibleHost) == nil {
255+
errors = append(errors, field.Invalid(
256+
nodePath.Child("ansible", "ansibleHost"),
257+
node.Ansible.AnsibleHost,
258+
"ansibleHost must be a valid IP address for "+
259+
"pre-provisioned nodes"))
260+
}
261+
}
262+
return errors
263+
}

‎internal/dataplane/ipam.go‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import (
2020
"context"
2121
"errors"
2222
"fmt"
23+
"net"
2324
"sort"
2425
"strings"
2526

@@ -359,10 +360,17 @@ func reserveIPs(ctx context.Context, helper *helper.Helper,
359360
}
360361

361362
if len(node.Networks) > 0 {
362-
for _, net := range node.Networks {
363-
if strings.EqualFold(string(net.Name), dataplanev1.CtlPlaneNetwork) ||
364-
netServiceNetMap[strings.ToLower(string(net.Name))] == dataplanev1.CtlPlaneNetwork {
363+
for i, nnet := range node.Networks {
364+
if strings.EqualFold(string(nnet.Name), dataplanev1.CtlPlaneNetwork) ||
365+
netServiceNetMap[strings.ToLower(string(nnet.Name))] == dataplanev1.CtlPlaneNetwork {
365366
foundCtlPlane = true
367+
// Default ctlplane fixedIP from ansibleHost for pre-provisioned nodes
368+
if instance.Spec.PreProvisioned &&
369+
node.Ansible.AnsibleHost != "" &&
370+
(nnet.FixedIP == nil || *nnet.FixedIP == "") &&
371+
net.ParseIP(node.Ansible.AnsibleHost) != nil {
372+
node.Networks[i].FixedIP = &node.Ansible.AnsibleHost
373+
}
366374
break
367375
}
368376
}

‎test/functional/ctlplane/base_test.go‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -420,7 +420,11 @@ func DefaultDataPlaneNoNodeSetSpec(tlsEnabled bool) map[string]interface{} {
420420
if tlsEnabled {
421421
spec["tlsEnabled"] = true
422422
}
423-
spec["nodes"] = map[string]dataplanev1.NodeSection{"edpm-compute-node-1": {}}
423+
spec["nodes"] = map[string]dataplanev1.NodeSection{"edpm-compute-node-1": {
424+
Ansible: dataplanev1.AnsibleOpts{
425+
AnsibleHost: "192.168.122.100",
426+
},
427+
}}
424428
return spec
425429
}
426430

‎test/functional/dataplane/base_test.go‎

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,11 @@ func CustomServiceImageSpec() map[string]interface{} {
141141
"ansibleVars": ansibleServiceVars,
142142
},
143143
},
144-
"nodes": map[string]dataplanev1.NodeSection{"edpm-compute-node-1": {}},
144+
"nodes": map[string]dataplanev1.NodeSection{"edpm-compute-node-1": {
145+
Ansible: dataplanev1.AnsibleOpts{
146+
AnsibleHost: "192.168.122.100",
147+
},
148+
}},
145149
}
146150
}
147151

@@ -235,6 +239,9 @@ func DefaultDataPlaneNodeSetSpec(nodeSetName string) map[string]interface{} {
235239
{Name: "networkinternal", SubnetName: "subnet1"},
236240
{Name: "ctlplane", SubnetName: "subnet1"},
237241
},
242+
"ansible": map[string]interface{}{
243+
"ansibleHost": "192.168.122.100",
244+
},
238245
},
239246
},
240247
"baremetalSetTemplate": map[string]interface{}{
@@ -268,6 +275,9 @@ func DuplicateServiceNodeSetSpec(nodeSetName string) map[string]interface{} {
268275
{Name: "networkinternal", SubnetName: "subnet1"},
269276
{Name: "ctlplane", SubnetName: "subnet1"},
270277
},
278+
"ansible": map[string]interface{}{
279+
"ansibleHost": "192.168.122.100",
280+
},
271281
},
272282
},
273283
"secretMaxSize": 1048576,
@@ -292,7 +302,11 @@ func DefaultDataPlaneNoNodeSetSpec(tlsEnabled bool) map[string]interface{} {
292302
if tlsEnabled {
293303
spec["tlsEnabled"] = true
294304
}
295-
spec["nodes"] = map[string]dataplanev1.NodeSection{"edpm-compute-node-1": {}}
305+
spec["nodes"] = map[string]dataplanev1.NodeSection{"edpm-compute-node-1": {
306+
Ansible: dataplanev1.AnsibleOpts{
307+
AnsibleHost: "192.168.122.100",
308+
},
309+
}}
296310
return spec
297311
}
298312

‎test/functional/dataplane/openstackdataplanenodeset_controller_test.go‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -320,6 +320,9 @@ var _ = Describe("Dataplane NodeSet Test", func() {
320320
nodes := map[string]dataplanev1.NodeSection{
321321
dataplaneNodeName.Name: {
322322
HostName: dataplaneNodeName.Name,
323+
Ansible: dataplanev1.AnsibleOpts{
324+
AnsibleHost: "192.168.122.100",
325+
},
323326
},
324327
}
325328
Expect(dataplaneNodeSetInstance.Spec.Nodes).Should(Equal(nodes))
@@ -422,6 +425,9 @@ var _ = Describe("Dataplane NodeSet Test", func() {
422425
nodes := map[string]dataplanev1.NodeSection{
423426
dataplaneNodeName.Name: {
424427
HostName: dataplaneNodeName.Name,
428+
Ansible: dataplanev1.AnsibleOpts{
429+
AnsibleHost: "192.168.122.100",
430+
},
425431
},
426432
}
427433
Expect(dataplaneNodeSetInstance.Spec.Nodes).Should(Equal(nodes))
@@ -726,6 +732,7 @@ var _ = Describe("Dataplane NodeSet Test", func() {
726732
},
727733
"ansible": map[string]interface{}{
728734
"ansibleUser": "test-user",
735+
"ansibleHost": "192.168.122.100",
729736
},
730737
}
731738

@@ -845,6 +852,9 @@ var _ = Describe("Dataplane NodeSet Test", func() {
845852
nodes := map[string]dataplanev1.NodeSection{
846853
dataplaneNodeName.Name: {
847854
HostName: dataplaneNodeName.Name,
855+
Ansible: dataplanev1.AnsibleOpts{
856+
AnsibleHost: "192.168.122.100",
857+
},
848858
},
849859
}
850860
Expect(dataplaneNodeSetInstance.Spec.Nodes).Should(Equal(nodes))
@@ -1144,6 +1154,7 @@ var _ = Describe("Dataplane NodeSet Test", func() {
11441154
},
11451155
"ansible": map[string]interface{}{
11461156
"ansibleUser": "test-user",
1157+
"ansibleHost": "192.168.122.100",
11471158
},
11481159
}
11491160

‎test/functional/dataplane/openstackdataplanenodeset_webhook_test.go‎

Lines changed: 53 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,11 @@ var _ = Describe("DataplaneNodeSet Webhook", func() {
130130
nodeSetSpec["preProvisioned"] = true
131131
nodeSetSpec["nodes"] = map[string]interface{}{
132132
"compute-0": map[string]interface{}{
133-
"hostName": "compute-0"},
133+
"hostName": "compute-0",
134+
"ansible": map[string]interface{}{
135+
"ansibleHost": "192.168.122.100",
136+
},
137+
},
134138
}
135139
DeferCleanup(th.DeleteInstance, CreateDataplaneNodeSet(dataplaneNodeSetName, nodeSetSpec))
136140
})
@@ -141,7 +145,11 @@ var _ = Describe("DataplaneNodeSet Webhook", func() {
141145
newNodeSetSpec["preProvisioned"] = true
142146
newNodeSetSpec["nodes"] = map[string]interface{}{
143147
"compute-0": map[string]interface{}{
144-
"hostName": "compute-0"},
148+
"hostName": "compute-0",
149+
"ansible": map[string]interface{}{
150+
"ansibleHost": "192.168.122.100",
151+
},
152+
},
145153
}
146154
newInstance := DefaultDataplaneNodeSetTemplate(types.NamespacedName{Name: "test-duplicate-node", Namespace: namespace}, newNodeSetSpec)
147155
unstructuredObj := &unstructured.Unstructured{Object: newInstance}
@@ -159,16 +167,24 @@ var _ = Describe("DataplaneNodeSet Webhook", func() {
159167
"compute-3": map[string]interface{}{
160168
"hostName": "compute-3",
161169
"ansible": map[string]interface{}{
162-
"ansibleHost": "compute-3",
170+
"ansibleHost": "192.168.122.103",
163171
},
164172
},
165173
"compute-2": map[string]interface{}{
166-
"hostName": "compute-2"},
174+
"hostName": "compute-2",
175+
"ansible": map[string]interface{}{
176+
"ansibleHost": "192.168.122.102",
177+
},
178+
},
167179
"compute-8": map[string]interface{}{
168-
"hostName": "compute-8"},
180+
"hostName": "compute-8",
181+
"ansible": map[string]interface{}{
182+
"ansibleHost": "192.168.122.108",
183+
},
184+
},
169185
"compute-0": map[string]interface{}{
170186
"ansible": map[string]interface{}{
171-
"ansibleHost": "compute-0",
187+
"ansibleHost": "192.168.122.100",
172188
},
173189
},
174190
}
@@ -180,13 +196,43 @@ var _ = Describe("DataplaneNodeSet Webhook", func() {
180196
}).Should(ContainSubstring("already exists in another cluster"))
181197
})
182198
})
199+
200+
When("A pre-provisioned NodeSet is created without a valid IP as ansibleHost", func() {
201+
It("Should block creation", func() {
202+
Eventually(func(_ Gomega) string {
203+
spec := DefaultDataPlaneNoNodeSetSpec(false)
204+
spec["nodes"] = map[string]interface{}{
205+
"compute-0": map[string]interface{}{
206+
"hostName": "compute-0",
207+
"ansible": map[string]interface{}{
208+
"ansibleHost": "compute-0.example.com",
209+
},
210+
},
211+
}
212+
name := types.NamespacedName{
213+
Name: "test-hostname-ansiblehost", Namespace: namespace}
214+
obj := DefaultDataplaneNodeSetTemplate(name, spec)
215+
unstructuredObj := &unstructured.Unstructured{Object: obj}
216+
_, err := controllerutil.CreateOrPatch(
217+
th.Ctx, th.K8sClient, unstructuredObj,
218+
func() error { return nil })
219+
return fmt.Sprintf("%s", err)
220+
}).Should(ContainSubstring(
221+
"ansibleHost must be a valid IP address"))
222+
})
223+
})
224+
183225
When("A NodeSet is updated with a OpenStackDataPlaneDeployment", func() {
184226
BeforeEach(func() {
185227
nodeSetSpec := DefaultDataPlaneNoNodeSetSpec(false)
186228
nodeSetSpec["preProvisioned"] = true
187229
nodeSetSpec["nodes"] = map[string]interface{}{
188230
"compute-0": map[string]interface{}{
189-
"hostName": "compute-0"},
231+
"hostName": "compute-0",
232+
"ansible": map[string]interface{}{
233+
"ansibleHost": "192.168.122.100",
234+
},
235+
},
190236
}
191237

192238
DeferCleanup(th.DeleteInstance, CreateDataplaneNodeSet(dataplaneNodeSetName, nodeSetSpec))

‎test/kuttl/tests/dataplane-create-test/00-assert.yaml‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,12 @@ spec:
1717
nodes:
1818
edpm-compute-0:
1919
hostName: edpm-compute-0
20+
ansible:
21+
ansibleHost: 192.168.122.100
2022
networks:
2123
- name: ctlplane
2224
subnetName: subnet1
2325
defaultRoute: true
24-
fixedIP: 192.168.122.100
2526
- name: internalapi
2627
subnetName: subnet1
2728
- name: storage

‎test/kuttl/tests/dataplane-create-test/00-dataplane-create.yaml‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,11 +61,12 @@ spec:
6161
nodes:
6262
edpm-compute-0:
6363
hostName: edpm-compute-0
64+
ansible:
65+
ansibleHost: 192.168.122.100
6466
networks:
6567
- name: ctlplane
6668
subnetName: subnet1
6769
defaultRoute: true
68-
fixedIP: 192.168.122.100
6970
- name: internalapi
7071
subnetName: subnet1
7172
- name: storage

‎test/kuttl/tests/dataplane-deploy-multiple-secrets/00-assert.yaml‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -51,11 +51,12 @@ spec:
5151
nodes:
5252
edpm-compute-0:
5353
hostName: edpm-compute-0
54+
ansible:
55+
ansibleHost: 192.168.122.100
5456
networks:
5557
- name: ctlplane
5658
subnetName: subnet1
5759
defaultRoute: true
58-
fixedIP: 192.168.122.100
5960
- name: internalapi
6061
subnetName: subnet1
6162
fixedIP: 172.17.0.100
@@ -67,11 +68,12 @@ spec:
6768
fixedIP: 172.19.0.100
6869
edpm-compute-1:
6970
hostName: edpm-compute-1
71+
ansible:
72+
ansibleHost: 192.168.122.101
7073
networks:
7174
- name: ctlplane
7275
subnetName: subnet1
7376
defaultRoute: true
74-
fixedIP: 192.168.122.101
7577
- name: internalapi
7678
subnetName: subnet1
7779
fixedIP: 172.17.0.101
@@ -83,11 +85,12 @@ spec:
8385
fixedIP: 172.19.0.101
8486
edpm-compute-2:
8587
hostName: edpm-compute-2
88+
ansible:
89+
ansibleHost: 192.168.122.102
8690
networks:
8791
- name: ctlplane
8892
subnetName: subnet1
8993
defaultRoute: true
90-
fixedIP: 192.168.122.102
9194
- name: internalapi
9295
subnetName: subnet1
9396
fixedIP: 172.17.0.102

‎test/kuttl/tests/dataplane-deploy-multiple-secrets/00-dataplane-create.yaml‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -49,11 +49,12 @@ spec:
4949
nodes:
5050
edpm-compute-0:
5151
hostName: edpm-compute-0
52+
ansible:
53+
ansibleHost: 192.168.122.100
5254
networks:
5355
- name: ctlplane
5456
subnetName: subnet1
5557
defaultRoute: true
56-
fixedIP: 192.168.122.100
5758
- name: internalapi
5859
subnetName: subnet1
5960
fixedIP: 172.17.0.100
@@ -65,11 +66,12 @@ spec:
6566
fixedIP: 172.19.0.100
6667
edpm-compute-1:
6768
hostName: edpm-compute-1
69+
ansible:
70+
ansibleHost: 192.168.122.101
6871
networks:
6972
- name: ctlplane
7073
subnetName: subnet1
7174
defaultRoute: true
72-
fixedIP: 192.168.122.101
7375
- name: internalapi
7476
subnetName: subnet1
7577
fixedIP: 172.17.0.101
@@ -81,11 +83,12 @@ spec:
8183
fixedIP: 172.19.0.101
8284
edpm-compute-2:
8385
hostName: edpm-compute-2
86+
ansible:
87+
ansibleHost: 192.168.122.102
8488
networks:
8589
- name: ctlplane
8690
subnetName: subnet1
8791
defaultRoute: true
88-
fixedIP: 192.168.122.102
8992
- name: internalapi
9093
subnetName: subnet1
9194
fixedIP: 172.17.0.102

0 commit comments

Comments
 (0)