Skip to content

Commit 0614560

Browse files
committed
Make InstanceSpec the canonical struct for instance creation
Refactor instance creation in machine controller and cluster controller (for the bastion) to call compute.CreateInstance() with an InstanceSpec.
1 parent 4e66c12 commit 0614560

File tree

6 files changed

+397
-283
lines changed

6 files changed

+397
-283
lines changed

controllers/openstackcluster_controller.go

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -320,7 +320,8 @@ func reconcileBastion(scope *scope.Scope, cluster *clusterv1.Cluster, openStackC
320320
return nil
321321
}
322322

323-
instanceStatus, err = computeService.CreateBastion(openStackCluster, cluster.Name)
323+
instanceSpec := bastionToInstanceSpec(openStackCluster, cluster.Name)
324+
instanceStatus, err = computeService.CreateInstance(openStackCluster, openStackCluster, instanceSpec, cluster.Name)
324325
if err != nil {
325326
return errors.Errorf("failed to reconcile bastion: %v", err)
326327
}
@@ -356,6 +357,31 @@ func reconcileBastion(scope *scope.Scope, cluster *clusterv1.Cluster, openStackC
356357
return nil
357358
}
358359

360+
func bastionToInstanceSpec(openStackCluster *infrav1.OpenStackCluster, clusterName string) *compute.InstanceSpec {
361+
name := fmt.Sprintf("%s-bastion", clusterName)
362+
instanceSpec := &compute.InstanceSpec{
363+
Name: name,
364+
Flavor: openStackCluster.Spec.Bastion.Instance.Flavor,
365+
SSHKeyName: openStackCluster.Spec.Bastion.Instance.SSHKeyName,
366+
Image: openStackCluster.Spec.Bastion.Instance.Image,
367+
ImageUUID: openStackCluster.Spec.Bastion.Instance.ImageUUID,
368+
FailureDomain: openStackCluster.Spec.Bastion.AvailabilityZone,
369+
RootVolume: openStackCluster.Spec.Bastion.Instance.RootVolume,
370+
}
371+
372+
instanceSpec.SecurityGroups = openStackCluster.Spec.Bastion.Instance.SecurityGroups
373+
if openStackCluster.Spec.ManagedSecurityGroups {
374+
instanceSpec.SecurityGroups = append(instanceSpec.SecurityGroups, infrav1.SecurityGroupParam{
375+
UUID: openStackCluster.Status.BastionSecurityGroup.ID,
376+
})
377+
}
378+
379+
instanceSpec.Networks = openStackCluster.Spec.Bastion.Instance.Networks
380+
instanceSpec.Ports = openStackCluster.Spec.Bastion.Instance.Ports
381+
382+
return instanceSpec
383+
}
384+
359385
func reconcileNetworkComponents(scope *scope.Scope, cluster *clusterv1.Cluster, openStackCluster *infrav1.OpenStackCluster) error {
360386
clusterName := fmt.Sprintf("%s-%s", cluster.Namespace, cluster.Name)
361387

controllers/openstackmachine_controller.go

Lines changed: 77 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -392,7 +392,14 @@ func (r *OpenStackMachineReconciler) getOrCreate(logger logr.Logger, cluster *cl
392392

393393
if instanceStatus == nil {
394394
logger.Info("Machine not exist, Creating Machine", "Machine", openStackMachine.Name)
395-
instanceStatus, err = computeService.CreateInstance(openStackCluster, machine, openStackMachine, cluster.Name, userData)
395+
instanceSpec, err := machineToInstanceSpec(openStackCluster, machine, openStackMachine, userData)
396+
if err != nil {
397+
err = errors.Errorf("machine spec is invalid: %v", err)
398+
handleUpdateMachineError(logger, openStackMachine, err)
399+
return nil, err
400+
}
401+
402+
instanceStatus, err = computeService.CreateInstance(openStackMachine, openStackCluster, instanceSpec, cluster.Name)
396403
if err != nil {
397404
return nil, errors.Errorf("error creating Openstack instance: %v", err)
398405
}
@@ -401,6 +408,75 @@ func (r *OpenStackMachineReconciler) getOrCreate(logger logr.Logger, cluster *cl
401408
return instanceStatus, nil
402409
}
403410

411+
func machineToInstanceSpec(openStackCluster *infrav1.OpenStackCluster, machine *clusterv1.Machine, openStackMachine *infrav1.OpenStackMachine, userData string) (*compute.InstanceSpec, error) {
412+
if openStackMachine == nil {
413+
return nil, fmt.Errorf("create Options need be specified to create instace")
414+
}
415+
416+
if machine.Spec.FailureDomain == nil {
417+
return nil, fmt.Errorf("failure domain not set")
418+
}
419+
420+
instanceSpec := compute.InstanceSpec{
421+
Name: openStackMachine.Name,
422+
Image: openStackMachine.Spec.Image,
423+
ImageUUID: openStackMachine.Spec.ImageUUID,
424+
Flavor: openStackMachine.Spec.Flavor,
425+
SSHKeyName: openStackMachine.Spec.SSHKeyName,
426+
UserData: userData,
427+
Metadata: openStackMachine.Spec.ServerMetadata,
428+
ConfigDrive: openStackMachine.Spec.ConfigDrive != nil && *openStackMachine.Spec.ConfigDrive,
429+
FailureDomain: *machine.Spec.FailureDomain,
430+
RootVolume: openStackMachine.Spec.RootVolume,
431+
Subnet: openStackMachine.Spec.Subnet,
432+
ServerGroupID: openStackMachine.Spec.ServerGroupID,
433+
Trunk: openStackMachine.Spec.Trunk,
434+
}
435+
436+
machineTags := []string{}
437+
438+
// Append machine specific tags
439+
machineTags = append(machineTags, openStackMachine.Spec.Tags...)
440+
441+
// Append cluster scope tags
442+
machineTags = append(machineTags, openStackCluster.Spec.Tags...)
443+
444+
// tags need to be unique or the "apply tags" call will fail.
445+
deduplicate := func(tags []string) []string {
446+
seen := make(map[string]struct{}, len(machineTags))
447+
unique := make([]string, 0, len(machineTags))
448+
for _, tag := range tags {
449+
if _, ok := seen[tag]; !ok {
450+
seen[tag] = struct{}{}
451+
unique = append(unique, tag)
452+
}
453+
}
454+
return unique
455+
}
456+
machineTags = deduplicate(machineTags)
457+
458+
instanceSpec.Tags = machineTags
459+
460+
instanceSpec.SecurityGroups = openStackMachine.Spec.SecurityGroups
461+
if openStackCluster.Spec.ManagedSecurityGroups {
462+
var managedSecurityGroup string
463+
if util.IsControlPlaneMachine(machine) {
464+
managedSecurityGroup = openStackCluster.Status.ControlPlaneSecurityGroup.ID
465+
} else {
466+
managedSecurityGroup = openStackCluster.Status.WorkerSecurityGroup.ID
467+
}
468+
469+
instanceSpec.SecurityGroups = append(instanceSpec.SecurityGroups, infrav1.SecurityGroupParam{
470+
UUID: managedSecurityGroup,
471+
})
472+
}
473+
474+
instanceSpec.Networks = openStackMachine.Spec.Networks
475+
instanceSpec.Ports = openStackMachine.Spec.Ports
476+
477+
return &instanceSpec, nil
478+
}
479+
404480
func handleUpdateMachineError(logger logr.Logger, openstackMachine *infrav1.OpenStackMachine, message error) {
405481
err := capierrors.UpdateMachineError
406482
openstackMachine.Status.FailureReason = &err
Lines changed: 226 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,226 @@
1+
/*
2+
Copyright 2020 The Kubernetes Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package controllers
18+
19+
import (
20+
"testing"
21+
22+
. "github.com/onsi/gomega"
23+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
24+
"k8s.io/utils/pointer"
25+
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
26+
27+
infrav1 "sigs.k8s.io/cluster-api-provider-openstack/api/v1beta1"
28+
"sigs.k8s.io/cluster-api-provider-openstack/pkg/cloud/services/compute"
29+
)
30+
31+
const (
32+
networkUUID = "d412171b-9fd7-41c1-95a6-c24e5953974d"
33+
subnetUUID = "d2d8d98d-b234-477e-a547-868b7cb5d6a5"
34+
extraSecurityGroupUUID = "514bb2d8-3390-4a3b-86a7-7864ba57b329"
35+
controlPlaneSecurityGroupUUID = "c9817a91-4821-42db-8367-2301002ab659"
36+
workerSecurityGroupUUID = "9c6c0d28-03c9-436c-815d-58440ac2c1c8"
37+
serverGroupUUID = "7b940d62-68ef-4e42-a76a-1a62e290509c"
38+
39+
openStackMachineName = "test-openstack-machine"
40+
namespace = "test-namespace"
41+
imageName = "test-image"
42+
flavorName = "test-flavor"
43+
sshKeyName = "test-ssh-key"
44+
failureDomain = "test-failure-domain"
45+
)
46+
47+
func getDefaultOpenStackCluster() *infrav1.OpenStackCluster {
48+
return &infrav1.OpenStackCluster{
49+
Spec: infrav1.OpenStackClusterSpec{},
50+
Status: infrav1.OpenStackClusterStatus{
51+
Network: &infrav1.Network{
52+
ID: networkUUID,
53+
Subnet: &infrav1.Subnet{
54+
ID: subnetUUID,
55+
},
56+
},
57+
ControlPlaneSecurityGroup: &infrav1.SecurityGroup{ID: controlPlaneSecurityGroupUUID},
58+
WorkerSecurityGroup: &infrav1.SecurityGroup{ID: workerSecurityGroupUUID},
59+
},
60+
}
61+
}
62+
63+
func getDefaultMachine() *clusterv1.Machine {
64+
return &clusterv1.Machine{
65+
Spec: clusterv1.MachineSpec{
66+
FailureDomain: pointer.StringPtr(failureDomain),
67+
},
68+
}
69+
}
70+
71+
func getDefaultOpenStackMachine() *infrav1.OpenStackMachine {
72+
return &infrav1.OpenStackMachine{
73+
ObjectMeta: metav1.ObjectMeta{
74+
Name: openStackMachineName,
75+
Namespace: namespace,
76+
},
77+
Spec: infrav1.OpenStackMachineSpec{
78+
// ProviderID is set by the controller
79+
// InstanceID is set by the controller
80+
// FloatingIP is only used by the cluster controller for the Bastion
81+
// TODO: Test Networks, Ports, Subnet, and Trunk separately
82+
CloudName: "test-cloud",
83+
Flavor: flavorName,
84+
Image: imageName,
85+
SSHKeyName: sshKeyName,
86+
Tags: []string{"test-tag"},
87+
ServerMetadata: map[string]string{
88+
"test-metadata": "test-value",
89+
},
90+
ConfigDrive: pointer.BoolPtr(true),
91+
ServerGroupID: serverGroupUUID,
92+
},
93+
}
94+
}
95+
96+
func getDefaultInstanceSpec() *compute.InstanceSpec {
97+
return &compute.InstanceSpec{
98+
Name: openStackMachineName,
99+
Image: imageName,
100+
Flavor: flavorName,
101+
SSHKeyName: sshKeyName,
102+
UserData: "user-data",
103+
Metadata: map[string]string{
104+
"test-metadata": "test-value",
105+
},
106+
ConfigDrive: *pointer.BoolPtr(true),
107+
FailureDomain: *pointer.StringPtr(failureDomain),
108+
ServerGroupID: serverGroupUUID,
109+
Tags: []string{"test-tag"},
110+
}
111+
}
112+
113+
func Test_machineToInstanceSpec(t *testing.T) {
114+
RegisterTestingT(t)
115+
116+
tests := []struct {
117+
name string
118+
openStackCluster func() *infrav1.OpenStackCluster
119+
machine func() *clusterv1.Machine
120+
openStackMachine func() *infrav1.OpenStackMachine
121+
wantInstanceSpec func() *compute.InstanceSpec
122+
wantErr bool
123+
}{
124+
{
125+
name: "Defaults",
126+
openStackCluster: getDefaultOpenStackCluster,
127+
machine: getDefaultMachine,
128+
openStackMachine: getDefaultOpenStackMachine,
129+
wantInstanceSpec: getDefaultInstanceSpec,
130+
wantErr: false,
131+
},
132+
{
133+
name: "Control plane security group",
134+
openStackCluster: func() *infrav1.OpenStackCluster {
135+
c := getDefaultOpenStackCluster()
136+
c.Spec.ManagedSecurityGroups = true
137+
return c
138+
},
139+
machine: func() *clusterv1.Machine {
140+
m := getDefaultMachine()
141+
m.Labels = map[string]string{
142+
clusterv1.MachineControlPlaneLabelName: "true",
143+
}
144+
return m
145+
},
146+
openStackMachine: getDefaultOpenStackMachine,
147+
wantInstanceSpec: func() *compute.InstanceSpec {
148+
i := getDefaultInstanceSpec()
149+
i.SecurityGroups = []infrav1.SecurityGroupParam{{UUID: controlPlaneSecurityGroupUUID}}
150+
return i
151+
},
152+
wantErr: false,
153+
},
154+
{
155+
name: "Worker security group",
156+
openStackCluster: func() *infrav1.OpenStackCluster {
157+
c := getDefaultOpenStackCluster()
158+
c.Spec.ManagedSecurityGroups = true
159+
return c
160+
},
161+
machine: getDefaultMachine,
162+
openStackMachine: getDefaultOpenStackMachine,
163+
wantInstanceSpec: func() *compute.InstanceSpec {
164+
i := getDefaultInstanceSpec()
165+
i.SecurityGroups = []infrav1.SecurityGroupParam{{UUID: workerSecurityGroupUUID}}
166+
return i
167+
},
168+
wantErr: false,
169+
},
170+
{
171+
name: "Extra security group",
172+
openStackCluster: func() *infrav1.OpenStackCluster {
173+
c := getDefaultOpenStackCluster()
174+
c.Spec.ManagedSecurityGroups = true
175+
return c
176+
},
177+
machine: getDefaultMachine,
178+
openStackMachine: func() *infrav1.OpenStackMachine {
179+
m := getDefaultOpenStackMachine()
180+
m.Spec.SecurityGroups = []infrav1.SecurityGroupParam{{UUID: extraSecurityGroupUUID}}
181+
return m
182+
},
183+
wantInstanceSpec: func() *compute.InstanceSpec {
184+
i := getDefaultInstanceSpec()
185+
i.SecurityGroups = []infrav1.SecurityGroupParam{
186+
{UUID: extraSecurityGroupUUID},
187+
{UUID: workerSecurityGroupUUID},
188+
}
189+
return i
190+
},
191+
wantErr: false,
192+
},
193+
{
194+
name: "Tags",
195+
openStackCluster: func() *infrav1.OpenStackCluster {
196+
c := getDefaultOpenStackCluster()
197+
c.Spec.Tags = []string{"cluster-tag", "duplicate-tag"}
198+
return c
199+
},
200+
machine: getDefaultMachine,
201+
openStackMachine: func() *infrav1.OpenStackMachine {
202+
m := getDefaultOpenStackMachine()
203+
m.Spec.Tags = []string{"machine-tag", "duplicate-tag"}
204+
return m
205+
},
206+
wantInstanceSpec: func() *compute.InstanceSpec {
207+
i := getDefaultInstanceSpec()
208+
i.Tags = []string{"machine-tag", "duplicate-tag", "cluster-tag"}
209+
return i
210+
},
211+
wantErr: false,
212+
},
213+
}
214+
for _, tt := range tests {
215+
t.Run(tt.name, func(t *testing.T) {
216+
got, err := machineToInstanceSpec(tt.openStackCluster(), tt.machine(), tt.openStackMachine(), "user-data")
217+
if tt.wantErr {
218+
Expect(err).To(HaveOccurred())
219+
} else {
220+
Expect(err).NotTo(HaveOccurred())
221+
}
222+
223+
Expect(got).To(Equal(tt.wantInstanceSpec()))
224+
})
225+
}
226+
}

0 commit comments

Comments
 (0)