Skip to content

Commit 6ba04de

Browse files
authored
Merge pull request #1191 from shiftstack/events
✨Refactor CreateInstance and CreateBastion
2 parents d9be9d0 + 9b96c8f commit 6ba04de

File tree

8 files changed

+475
-420
lines changed

8 files changed

+475
-420
lines changed

controllers/openstackcluster_controller.go

Lines changed: 29 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -222,8 +222,8 @@ func deleteBastion(scope *scope.Scope, cluster *clusterv1.Cluster, openStackClus
222222
}
223223
}
224224

225-
machineSpec := &openStackCluster.Spec.Bastion.Instance
226-
if err = computeService.DeleteInstance(openStackCluster, machineSpec, instanceName, instanceStatus); err != nil {
225+
instanceSpec := bastionToInstanceSpec(openStackCluster, cluster.Name)
226+
if err = computeService.DeleteInstance(openStackCluster, instanceSpec, instanceStatus); err != nil {
227227
handleUpdateOSCError(openStackCluster, errors.Errorf("failed to delete bastion: %v", err))
228228
return errors.Errorf("failed to delete bastion: %v", err)
229229
}
@@ -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: 85 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -248,7 +248,14 @@ func (r *OpenStackMachineReconciler) reconcileDelete(ctx context.Context, scope
248248
}
249249
}
250250

251-
if err := computeService.DeleteInstance(openStackMachine, &openStackMachine.Spec, openStackMachine.Name, instanceStatus); err != nil {
251+
instanceSpec, err := machineToInstanceSpec(openStackCluster, machine, openStackMachine, "")
252+
if err != nil {
253+
err = errors.Errorf("machine spec is invalid: %v", err)
254+
handleUpdateMachineError(scope.Logger, openStackMachine, err)
255+
return ctrl.Result{}, err
256+
}
257+
258+
if err := computeService.DeleteInstance(openStackMachine, instanceSpec, instanceStatus); err != nil {
252259
handleUpdateMachineError(scope.Logger, openStackMachine, errors.Errorf("error deleting OpenStack instance %s with ID %s: %v", instanceStatus.Name(), instanceStatus.ID(), err))
253260
return ctrl.Result{}, nil
254261
}
@@ -392,7 +399,14 @@ func (r *OpenStackMachineReconciler) getOrCreate(logger logr.Logger, cluster *cl
392399

393400
if instanceStatus == nil {
394401
logger.Info("Machine not exist, Creating Machine", "Machine", openStackMachine.Name)
395-
instanceStatus, err = computeService.CreateInstance(openStackCluster, machine, openStackMachine, cluster.Name, userData)
402+
instanceSpec, err := machineToInstanceSpec(openStackCluster, machine, openStackMachine, userData)
403+
if err != nil {
404+
err = errors.Errorf("machine spec is invalid: %v", err)
405+
handleUpdateMachineError(logger, openStackMachine, err)
406+
return nil, err
407+
}
408+
409+
instanceStatus, err = computeService.CreateInstance(openStackMachine, openStackCluster, instanceSpec, cluster.Name)
396410
if err != nil {
397411
return nil, errors.Errorf("error creating Openstack instance: %v", err)
398412
}
@@ -401,6 +415,75 @@ func (r *OpenStackMachineReconciler) getOrCreate(logger logr.Logger, cluster *cl
401415
return instanceStatus, nil
402416
}
403417

418+
func machineToInstanceSpec(openStackCluster *infrav1.OpenStackCluster, machine *clusterv1.Machine, openStackMachine *infrav1.OpenStackMachine, userData string) (*compute.InstanceSpec, error) {
419+
if openStackMachine == nil {
420+
return nil, fmt.Errorf("create Options need be specified to create instace")
421+
}
422+
423+
if machine.Spec.FailureDomain == nil {
424+
return nil, fmt.Errorf("failure domain not set")
425+
}
426+
427+
instanceSpec := compute.InstanceSpec{
428+
Name: openStackMachine.Name,
429+
Image: openStackMachine.Spec.Image,
430+
ImageUUID: openStackMachine.Spec.ImageUUID,
431+
Flavor: openStackMachine.Spec.Flavor,
432+
SSHKeyName: openStackMachine.Spec.SSHKeyName,
433+
UserData: userData,
434+
Metadata: openStackMachine.Spec.ServerMetadata,
435+
ConfigDrive: openStackMachine.Spec.ConfigDrive != nil && *openStackMachine.Spec.ConfigDrive,
436+
FailureDomain: *machine.Spec.FailureDomain,
437+
RootVolume: openStackMachine.Spec.RootVolume,
438+
Subnet: openStackMachine.Spec.Subnet,
439+
ServerGroupID: openStackMachine.Spec.ServerGroupID,
440+
Trunk: openStackMachine.Spec.Trunk,
441+
}
442+
443+
machineTags := []string{}
444+
445+
// Append machine specific tags
446+
machineTags = append(machineTags, openStackMachine.Spec.Tags...)
447+
448+
// Append cluster scope tags
449+
machineTags = append(machineTags, openStackCluster.Spec.Tags...)
450+
451+
// tags need to be unique or the "apply tags" call will fail.
452+
deduplicate := func(tags []string) []string {
453+
seen := make(map[string]struct{}, len(machineTags))
454+
unique := make([]string, 0, len(machineTags))
455+
for _, tag := range tags {
456+
if _, ok := seen[tag]; !ok {
457+
seen[tag] = struct{}{}
458+
unique = append(unique, tag)
459+
}
460+
}
461+
return unique
462+
}
463+
machineTags = deduplicate(machineTags)
464+
465+
instanceSpec.Tags = machineTags
466+
467+
instanceSpec.SecurityGroups = openStackMachine.Spec.SecurityGroups
468+
if openStackCluster.Spec.ManagedSecurityGroups {
469+
var managedSecurityGroup string
470+
if util.IsControlPlaneMachine(machine) {
471+
managedSecurityGroup = openStackCluster.Status.ControlPlaneSecurityGroup.ID
472+
} else {
473+
managedSecurityGroup = openStackCluster.Status.WorkerSecurityGroup.ID
474+
}
475+
476+
instanceSpec.SecurityGroups = append(instanceSpec.SecurityGroups, infrav1.SecurityGroupParam{
477+
UUID: managedSecurityGroup,
478+
})
479+
}
480+
481+
instanceSpec.Networks = openStackMachine.Spec.Networks
482+
instanceSpec.Ports = openStackMachine.Spec.Ports
483+
484+
return &instanceSpec, nil
485+
}
486+
404487
func handleUpdateMachineError(logger logr.Logger, openstackMachine *infrav1.OpenStackMachine, message error) {
405488
err := capierrors.UpdateMachineError
406489
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)