Skip to content

Commit 43c0ed5

Browse files
author
shiftstack-merge-bot
committed
2 parents be72b75 + a0432a8 commit 43c0ed5

File tree

8 files changed

+120
-28
lines changed

8 files changed

+120
-28
lines changed

‎OWNERS_ALIASES

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,4 +24,3 @@ aliases:
2424
- mdbooth
2525
cluster-api-openstack-reviewers:
2626
- emilienm
27-
- dulek

‎controllers/openstackmachine_controller.go

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -776,17 +776,22 @@ func (r *OpenStackMachineReconciler) getOrCreateInstance(logger logr.Logger, ope
776776
}
777777
if instanceStatus == nil {
778778
// Check if there is an existing instance with machine name, in case where instance ID would not have been stored in machine status
779-
if instanceStatus, err = computeService.GetInstanceStatusByName(openStackMachine, openStackMachine.Name); err == nil {
780-
if instanceStatus != nil {
781-
return instanceStatus, nil
782-
}
783-
if openStackMachine.Status.InstanceID != nil {
784-
logger.Info("Not reconciling machine in failed state. The previously existing OpenStack instance is no longer available")
785-
conditions.MarkFalse(openStackMachine, infrav1.InstanceReadyCondition, infrav1.InstanceNotFoundReason, clusterv1.ConditionSeverityError, "virtual machine no longer exists")
786-
openStackMachine.SetFailure(capierrors.UpdateMachineError, errors.New("virtual machine no longer exists"))
787-
return nil, nil
788-
}
779+
instanceStatus, err = computeService.GetInstanceStatusByName(openStackMachine, openStackMachine.Name)
780+
if err != nil {
781+
logger.Info("Unable to get OpenStack instance by name", "name", openStackMachine.Name)
782+
conditions.MarkFalse(openStackMachine, infrav1.InstanceReadyCondition, infrav1.InstanceCreateFailedReason, clusterv1.ConditionSeverityError, err.Error())
783+
return nil, err
784+
}
785+
if instanceStatus != nil {
786+
return instanceStatus, nil
789787
}
788+
if openStackMachine.Status.InstanceID != nil {
789+
logger.Info("Not reconciling machine in failed state. The previously existing OpenStack instance is no longer available")
790+
conditions.MarkFalse(openStackMachine, infrav1.InstanceReadyCondition, infrav1.InstanceNotFoundReason, clusterv1.ConditionSeverityError, "virtual machine no longer exists")
791+
openStackMachine.SetFailure(capierrors.UpdateMachineError, errors.New("virtual machine no longer exists"))
792+
return nil, nil
793+
}
794+
790795
instanceSpec, err := machineToInstanceSpec(openStackCluster, machine, openStackMachine, userData)
791796
if err != nil {
792797
return nil, err

‎docs/book/src/clusteropenstack/configuration.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -565,6 +565,7 @@ permitted from anywhere, as with the default rules).
565565
We can add security group rules that authorize traffic from all nodes via `allNodesSecurityGroupRules`.
566566
It takes a list of security groups rules that should be applied to selected nodes.
567567
The following rule fields are mutually exclusive: `remoteManagedGroups`, `remoteGroupID` and `remoteIPPrefix`.
568+
If none of these fields are set, the rule will have a remote IP prefix of `0.0.0.0/0` per Neutron default.
568569

569570
Valid values for `remoteManagedGroups` are `controlplane`, `worker` and `bastion`.
570571

‎docs/book/src/topics/crd-changes/v1alpha7-to-v1beta1.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -396,6 +396,9 @@ The field `managedSecurityGroups` is now a pointer to a `ManagedSecurityGroups`
396396
Also, we can now add security group rules that authorize traffic from all nodes via `allNodesSecurityGroupRules`.
397397
It takes a list of security groups rules that should be applied to selected nodes.
398398
The following rule fields are mutually exclusive: `remoteManagedGroups`, `remoteGroupID` and `remoteIPPrefix`.
399+
If none of these fields are set, the rule will have a remote IP prefix of `0.0.0.0/0` per Neutron default.
400+
401+
```yaml
399402
Valid values for `remoteManagedGroups` are `controlplane`, `worker` and `bastion`.
400403

401404
Also, `OpenStackCluster.Spec.AllowAllInClusterTraffic` moved under `ManagedSecurityGroups`.

‎pkg/cloud/services/loadbalancer/loadbalancer.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -92,9 +92,10 @@ func (s *Service) ReconcileLoadBalancer(openStackCluster *infrav1.OpenStackClust
9292

9393
if lb.ProvisioningStatus != loadBalancerProvisioningStatusActive {
9494
var err error
95-
lb, err = s.waitForLoadBalancerActive(lb.ID)
95+
lbID := lb.ID
96+
lb, err = s.waitForLoadBalancerActive(lbID)
9697
if err != nil {
97-
return false, fmt.Errorf("load balancer %q with id %s is not active after timeout: %v", loadBalancerName, lb.ID, err)
98+
return false, fmt.Errorf("load balancer %q with id %s is not active after timeout: %v", loadBalancerName, lbID, err)
9899
}
99100
}
100101

‎pkg/cloud/services/loadbalancer/loadbalancer_test.go

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,13 @@ func Test_ReconcileLoadBalancer(t *testing.T) {
4545
mockCtrl := gomock.NewController(t)
4646
defer mockCtrl.Finish()
4747

48+
// Shortcut wait timeout
49+
backoffDurationPrev := backoff.Duration
50+
backoff.Duration = 0
51+
defer func() {
52+
backoff.Duration = backoffDurationPrev
53+
}()
54+
4855
// Stub the call to net.LookupHost
4956
lookupHost = func(host string) (addrs *string, err error) {
5057
if net.ParseIP(host) != nil {
@@ -139,6 +146,27 @@ func Test_ReconcileLoadBalancer(t *testing.T) {
139146
},
140147
wantError: nil,
141148
},
149+
{
150+
name: "reconcile loadbalancer in non active state should timeout",
151+
expectNetwork: func(*mock.MockNetworkClientMockRecorder) {
152+
// add network api call results here
153+
},
154+
expectLoadBalancer: func(m *mock.MockLbClientMockRecorder) {
155+
pendingLB := loadbalancers.LoadBalancer{
156+
ID: "aaaaaaaa-bbbb-cccc-dddd-333333333333",
157+
Name: "k8s-clusterapi-cluster-AAAAA-kubeapi",
158+
ProvisioningStatus: "PENDING_CREATE",
159+
}
160+
161+
// return existing loadbalancer in non-active state
162+
lbList := []loadbalancers.LoadBalancer{pendingLB}
163+
m.ListLoadBalancers(loadbalancers.ListOpts{Name: pendingLB.Name}).Return(lbList, nil)
164+
165+
// wait for loadbalancer until it times out
166+
m.GetLoadBalancer("aaaaaaaa-bbbb-cccc-dddd-333333333333").Return(&pendingLB, nil).Return(&pendingLB, nil).AnyTimes()
167+
},
168+
wantError: fmt.Errorf("load balancer \"k8s-clusterapi-cluster-AAAAA-kubeapi\" with id aaaaaaaa-bbbb-cccc-dddd-333333333333 is not active after timeout: timed out waiting for the condition"),
169+
},
142170
}
143171
for _, tt := range lbtests {
144172
t.Run(tt.name, func(t *testing.T) {

‎pkg/cloud/services/networking/securitygroups.go

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -297,10 +297,6 @@ func getAllNodesRules(remoteManagedGroups map[string]string, allNodesSecurityGro
297297

298298
// validateRemoteManagedGroups validates that the remoteManagedGroups target existing managed security groups.
299299
func validateRemoteManagedGroups(remoteManagedGroups map[string]string, ruleRemoteManagedGroups []infrav1.ManagedSecurityGroupName) error {
300-
if len(ruleRemoteManagedGroups) == 0 {
301-
return fmt.Errorf("remoteManagedGroups is required")
302-
}
303-
304300
for _, group := range ruleRemoteManagedGroups {
305301
if _, ok := remoteManagedGroups[group.String()]; !ok {
306302
return fmt.Errorf("remoteManagedGroups: %s is not a valid remote managed security group", group)

‎pkg/cloud/services/networking/securitygroups_test.go

Lines changed: 70 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -49,19 +49,14 @@ func TestValidateRemoteManagedGroups(t *testing.T) {
4949
wantErr: true,
5050
},
5151
{
52-
name: "Valid rule with missing remoteManagedGroups",
52+
name: "Valid rule with no remoteManagedGroups",
5353
rule: infrav1.SecurityGroupRuleSpec{
54-
PortRangeMin: ptr.To(22),
55-
PortRangeMax: ptr.To(22),
56-
Protocol: ptr.To("tcp"),
54+
PortRangeMin: ptr.To(22),
55+
PortRangeMax: ptr.To(22),
56+
Protocol: ptr.To("tcp"),
57+
RemoteIPPrefix: ptr.To("0.0.0.0/0"),
5758
},
58-
remoteManagedGroups: map[string]string{
59-
"self": "self",
60-
"controlplane": "1",
61-
"worker": "2",
62-
"bastion": "3",
63-
},
64-
wantErr: true,
59+
wantErr: false,
6560
},
6661
{
6762
name: "Valid rule with remoteManagedGroups",
@@ -171,6 +166,70 @@ func TestGetAllNodesRules(t *testing.T) {
171166
},
172167
},
173168
},
169+
{
170+
name: "Valid remoteIPPrefix in a rule",
171+
remoteManagedGroups: map[string]string{
172+
"controlplane": "1",
173+
"worker": "2",
174+
},
175+
allNodesSecurityGroupRules: []infrav1.SecurityGroupRuleSpec{
176+
{
177+
Protocol: ptr.To("tcp"),
178+
PortRangeMin: ptr.To(22),
179+
PortRangeMax: ptr.To(22),
180+
RemoteIPPrefix: ptr.To("0.0.0.0/0"),
181+
},
182+
},
183+
wantRules: []resolvedSecurityGroupRuleSpec{
184+
{
185+
Protocol: "tcp",
186+
PortRangeMin: 22,
187+
PortRangeMax: 22,
188+
RemoteIPPrefix: "0.0.0.0/0",
189+
},
190+
},
191+
},
192+
{
193+
name: "Valid allNodesSecurityGroupRules with no remote parameter",
194+
remoteManagedGroups: map[string]string{
195+
"controlplane": "1",
196+
"worker": "2",
197+
},
198+
allNodesSecurityGroupRules: []infrav1.SecurityGroupRuleSpec{
199+
{
200+
Protocol: ptr.To("tcp"),
201+
PortRangeMin: ptr.To(22),
202+
PortRangeMax: ptr.To(22),
203+
},
204+
},
205+
wantRules: []resolvedSecurityGroupRuleSpec{
206+
{
207+
Protocol: "tcp",
208+
PortRangeMin: 22,
209+
PortRangeMax: 22,
210+
},
211+
},
212+
wantErr: false,
213+
},
214+
{
215+
name: "Invalid allNodesSecurityGroupRules with bastion while remoteManagedGroups does not have bastion",
216+
remoteManagedGroups: map[string]string{
217+
"controlplane": "1",
218+
"worker": "2",
219+
},
220+
allNodesSecurityGroupRules: []infrav1.SecurityGroupRuleSpec{
221+
{
222+
Protocol: ptr.To("tcp"),
223+
PortRangeMin: ptr.To(22),
224+
PortRangeMax: ptr.To(22),
225+
RemoteManagedGroups: []infrav1.ManagedSecurityGroupName{
226+
"bastion",
227+
},
228+
},
229+
},
230+
wantRules: nil,
231+
wantErr: true,
232+
},
174233
{
175234
name: "Invalid allNodesSecurityGroupRules with wrong remoteManagedGroups",
176235
remoteManagedGroups: map[string]string{

0 commit comments

Comments
 (0)