Skip to content

Commit f46ba0f

Browse files
openshift-merge-bot[bot]openshift-merge-bot[bot]
authored
Merge pull request #1802 from rabi/ctlplane_fixed_ip
Validate ansibleHost and ctlplane fixedIP for pre-provisioned nodes
2 parents 777d3bb + 3f6c604 commit f46ba0f

File tree

14 files changed

+158
-27
lines changed

14 files changed

+158
-27
lines changed

‎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)