Skip to content

Commit 0b98cae

Browse files
robertvolkmannrobertvolkmann
authored
Merge pull request #32 from metal-stack/fix-deletion-of-privded-resources
fix: do not delete provided resources
2 parents 5574304 + ff689bd commit 0b98cae

File tree

2 files changed

+100
-79
lines changed

2 files changed

+100
-79
lines changed

‎internal/controller/metalstackcluster_controller.go‎

Lines changed: 98 additions & 78 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,11 @@ import (
5656
metalgo "github.com/metal-stack/metal-go"
5757
)
5858

59+
var (
60+
errProviderIPNotFound = errors.New("provider ip not found")
61+
errProviderIPTooManyFound = errors.New("multiple provider ips found")
62+
)
63+
5964
// MetalStackClusterReconciler reconciles a MetalStackCluster object
6065
type MetalStackClusterReconciler struct {
6166
MetalClient metalgo.Client
@@ -235,29 +240,21 @@ func (r *clusterReconciler) delete() error {
235240
return fmt.Errorf("unable to delete firewall deployment: %w", err)
236241
}
237242

238-
r.log.Info("deleted firewall deployment")
239-
240243
err = r.deleteControlPlaneIP()
241244
if err != nil {
242245
return fmt.Errorf("unable to delete control plane ip: %w", err)
243246
}
244247

245-
r.log.Info("deleted control plane ip")
246-
247248
err = r.deleteNodeNetwork()
248249
if err != nil {
249250
return fmt.Errorf("unable to delete node network: %w", err)
250251
}
251252

252-
r.log.Info("deleted node network")
253-
254253
err = r.deleteSshKeyPair(r.ctx)
255254
if err != nil {
256255
return fmt.Errorf("unable to delete ssh key pair: %w", err)
257256
}
258257

259-
r.log.Info("deleted ssh key pair")
260-
261258
return err
262259
}
263260

@@ -325,6 +322,8 @@ func (r *clusterReconciler) deleteSshKeyPair(ctx context.Context) error {
325322
return err
326323
}
327324

325+
r.log.Info("deleted ssh key pair")
326+
328327
return nil
329328
}
330329

@@ -362,6 +361,11 @@ func (r *clusterReconciler) ensureNodeNetwork() (string, error) {
362361
}
363362

364363
func (r *clusterReconciler) deleteNodeNetwork() error {
364+
if r.infraCluster.Spec.NodeNetworkID != nil {
365+
r.log.Info("skip deletion of node network")
366+
return nil
367+
}
368+
365369
nws, err := r.findNodeNetwork()
366370
if err != nil {
367371
return err
@@ -381,6 +385,7 @@ func (r *clusterReconciler) deleteNodeNetwork() error {
381385
if err != nil {
382386
return err
383387
}
388+
r.log.Info("deleted node network")
384389

385390
return nil
386391
default:
@@ -411,84 +416,90 @@ func (r *clusterReconciler) findNodeNetwork() ([]*models.V1NetworkResponse, erro
411416
}
412417

413418
func (r *clusterReconciler) ensureControlPlaneIP() (*models.V1IPResponse, error) {
414-
ips, err := r.findControlPlaneIP()
415-
if err != nil {
419+
ip, err := r.findControlPlaneIP()
420+
if ip != nil {
421+
return ip, nil
422+
}
423+
if errors.Is(err, errProviderIPTooManyFound) {
424+
return nil, fmt.Errorf("more than a single control plane ip exists for this cluster, operator investigation is required")
425+
}
426+
if err != nil && !errors.Is(err, errProviderIPNotFound) {
416427
return nil, err
417428
}
418429

419-
switch len(ips) {
420-
case 0:
421-
nwResp, err := r.metalClient.Network().FindNetworks(network.NewFindNetworksParams().WithBody(&models.V1NetworkFindRequest{
422-
Labels: map[string]string{
423-
tag.NetworkDefault: "",
424-
},
425-
}).WithContext(r.ctx), nil)
426-
if err != nil {
427-
return nil, fmt.Errorf("error finding default network: %w", err)
428-
}
429-
430-
if len(nwResp.Payload) != 1 {
431-
return nil, fmt.Errorf("no distinct default network configured in the metal-api")
432-
}
430+
nwResp, err := r.metalClient.Network().FindNetworks(network.NewFindNetworksParams().WithBody(&models.V1NetworkFindRequest{
431+
Labels: map[string]string{
432+
tag.NetworkDefault: "",
433+
},
434+
}).WithContext(r.ctx), nil)
435+
if err != nil {
436+
return nil, fmt.Errorf("error finding default network: %w", err)
437+
}
433438

434-
resp, err := r.metalClient.IP().AllocateIP(ipmodels.NewAllocateIPParams().WithBody(&models.V1IPAllocateRequest{
435-
Description: fmt.Sprintf("%s/%s control plane ip", r.infraCluster.GetNamespace(), r.infraCluster.GetName()),
436-
Name: r.infraCluster.GetName() + "-control-plane",
437-
Networkid: nwResp.Payload[0].ID,
438-
Projectid: &r.infraCluster.Spec.ProjectID,
439-
Tags: []string{
440-
tag.New(tag.ClusterID, string(r.infraCluster.GetUID())),
441-
v1alpha1.TagControlPlanePurpose,
442-
},
443-
Type: ptr.To(models.V1IPBaseTypeStatic),
444-
}).WithContext(r.ctx), nil)
445-
if err != nil {
446-
return nil, fmt.Errorf("error creating ip: %w", err)
447-
}
439+
if len(nwResp.Payload) != 1 {
440+
return nil, fmt.Errorf("no distinct default network configured in the metal-api")
441+
}
448442

449-
return resp.Payload, nil
450-
case 1:
451-
return ips[0], nil
452-
default:
453-
return nil, fmt.Errorf("more than a single control plane ip exists for this cluster, operator investigation is required")
443+
resp, err := r.metalClient.IP().AllocateIP(ipmodels.NewAllocateIPParams().WithBody(&models.V1IPAllocateRequest{
444+
Description: fmt.Sprintf("%s/%s control plane ip", r.infraCluster.GetNamespace(), r.infraCluster.GetName()),
445+
Name: r.infraCluster.GetName() + "-control-plane",
446+
Networkid: nwResp.Payload[0].ID,
447+
Projectid: &r.infraCluster.Spec.ProjectID,
448+
Tags: []string{
449+
tag.New(tag.ClusterID, string(r.infraCluster.GetUID())),
450+
v1alpha1.TagControlPlanePurpose,
451+
},
452+
Type: ptr.To(models.V1IPBaseTypeEphemeral),
453+
}).WithContext(r.ctx), nil)
454+
if err != nil {
455+
return nil, fmt.Errorf("error creating ip: %w", err)
454456
}
457+
458+
return resp.Payload, nil
455459
}
456460

457461
func (r *clusterReconciler) deleteControlPlaneIP() error {
458-
ips, err := r.findControlPlaneIP()
462+
if r.infraCluster.Spec.ControlPlaneIP != nil {
463+
r.log.Info("skip deletion of provided control plane ip")
464+
return nil
465+
}
466+
ip, err := r.findControlPlaneIP()
467+
if err != nil && errors.Is(err, errProviderIPNotFound) {
468+
return nil
469+
}
459470
if err != nil {
460-
return err
471+
return fmt.Errorf("unable to delete control plane ip: %w", err)
461472
}
462-
463-
switch len(ips) {
464-
case 0:
473+
if ip.Type != nil && *ip.Type == models.V1IPBaseTypeStatic {
474+
r.log.Info("skip deletion of static control plane ip")
465475
return nil
466-
case 1:
467-
ip := ips[0]
468-
469-
if ip.Ipaddress == nil {
470-
return fmt.Errorf("control plane ip address not set")
471-
}
476+
}
472477

473-
_, err := r.metalClient.IP().FreeIP(ipmodels.NewFreeIPParams().WithID(*ip.Ipaddress).WithContext(r.ctx), nil)
474-
if err != nil {
475-
return err
476-
}
478+
if ip.Ipaddress == nil {
479+
return fmt.Errorf("control plane ip address not set")
480+
}
477481

482+
if ip.Type != nil && *ip.Type == models.V1IPAllocateRequestTypeStatic {
483+
r.log.Info("skipping deletion of static control plane ip", "ip", *ip.Ipaddress)
478484
return nil
479-
default:
480-
return fmt.Errorf("more than a single control plane ip exists for this cluster, operator investigation is required")
481485
}
486+
_, err = r.metalClient.IP().FreeIP(ipmodels.NewFreeIPParams().WithID(*ip.Ipaddress).WithContext(r.ctx), nil)
487+
if err != nil {
488+
return err
489+
}
490+
r.log.Info("deleted control plane ip", "address", *ip.Ipaddress)
491+
492+
return nil
482493
}
483494

484-
func (r *clusterReconciler) findControlPlaneIP() ([]*models.V1IPResponse, error) {
495+
func (r *clusterReconciler) findControlPlaneIP() (*models.V1IPResponse, error) {
485496
if r.infraCluster.Spec.ControlPlaneIP != nil {
486497
resp, err := r.metalClient.IP().FindIP(ipmodels.NewFindIPParams().WithID(*r.infraCluster.Spec.ControlPlaneIP).WithContext(r.ctx), nil)
487498
if err != nil {
488499
return nil, err
489500
}
490501

491-
return []*models.V1IPResponse{resp.Payload}, nil
502+
return resp.Payload, nil
492503
}
493504

494505
resp, err := r.metalClient.IP().FindIPs(ipmodels.NewFindIPsParams().WithBody(&models.V1IPFindRequest{
@@ -502,7 +513,14 @@ func (r *clusterReconciler) findControlPlaneIP() ([]*models.V1IPResponse, error)
502513
return nil, err
503514
}
504515

505-
return resp.Payload, nil
516+
switch len(resp.Payload) {
517+
case 0:
518+
return nil, errProviderIPNotFound
519+
case 1:
520+
return resp.Payload[0], nil
521+
default:
522+
return nil, errProviderIPTooManyFound
523+
}
506524
}
507525

508526
func (r *clusterReconciler) ensureFirewallDeployment(nodeNetworkID, sshPubKey string) (*fcmv2.FirewallDeployment, error) {
@@ -649,6 +667,8 @@ func (r *clusterReconciler) deleteFirewallDeployment() error {
649667
return fmt.Errorf("error deleting firewall deployment: %w", err)
650668
}
651669

670+
r.log.Info("deleting firewall deployment")
671+
652672
return errors.New("firewall deployment was deleted, process is still ongoing")
653673
}
654674

@@ -702,30 +722,30 @@ func (r *clusterReconciler) status() error {
702722
})
703723

704724
g.Go(func() error {
705-
ips, err := r.findControlPlaneIP()
725+
ip, err := r.findControlPlaneIP()
706726

707727
conditionUpdates <- func() {
708-
if err != nil {
709-
conditions.MarkFalse(r.infraCluster, v1alpha1.ClusterControlPlaneEndpointEnsured, "InternalError", clusterv1.ConditionSeverityError, "%s", err.Error())
710-
return
711-
}
712-
713-
switch len(ips) {
714-
case 0:
728+
if errors.Is(err, errProviderIPNotFound) {
715729
if r.infraCluster.Spec.ControlPlaneEndpoint.Host != "" {
716730
conditions.MarkTrue(r.infraCluster, v1alpha1.ClusterControlPlaneEndpointEnsured)
717731
} else {
718732
conditions.MarkFalse(r.infraCluster, v1alpha1.ClusterControlPlaneEndpointEnsured, "NotCreated", clusterv1.ConditionSeverityError, "control plane ip was not yet created")
719733
}
720-
case 1:
721-
if r.infraCluster.Spec.ControlPlaneEndpoint.Host == *ips[0].Ipaddress {
722-
conditions.MarkTrue(r.infraCluster, v1alpha1.ClusterControlPlaneEndpointEnsured)
723-
} else {
724-
conditions.MarkFalse(r.infraCluster, v1alpha1.ClusterControlPlaneEndpointEnsured, "NotSet", clusterv1.ConditionSeverityWarning, "control plane ip was not yet patched into the cluster's spec")
725-
}
726-
default:
734+
735+
}
736+
if errors.Is(err, errProviderIPTooManyFound) {
727737
conditions.MarkFalse(r.infraCluster, v1alpha1.ClusterControlPlaneEndpointEnsured, "InternalError", clusterv1.ConditionSeverityError, "more than a single control plane ip exists for this cluster, operator investigation is required")
728738
}
739+
if err != nil {
740+
conditions.MarkFalse(r.infraCluster, v1alpha1.ClusterControlPlaneEndpointEnsured, "InternalError", clusterv1.ConditionSeverityError, "%s", err.Error())
741+
return
742+
}
743+
744+
if r.infraCluster.Spec.ControlPlaneEndpoint.Host == *ip.Ipaddress {
745+
conditions.MarkTrue(r.infraCluster, v1alpha1.ClusterControlPlaneEndpointEnsured)
746+
} else {
747+
conditions.MarkFalse(r.infraCluster, v1alpha1.ClusterControlPlaneEndpointEnsured, "NotSet", clusterv1.ConditionSeverityWarning, "control plane ip was not yet patched into the cluster's spec")
748+
}
729749
}
730750

731751
return err

‎internal/controller/metalstackcluster_controller_test.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,7 @@ var _ = Describe("MetalStackCluster Controller", func() {
168168
Description: resource.Namespace + "/" + resource.Name + " control plane ip",
169169
Networkid: ptr.To("internet"),
170170
Projectid: ptr.To("test-project"),
171-
Type: ptr.To("static"),
171+
Type: ptr.To("ephemeral"),
172172
})), nil).Run(func(args mock.Arguments) {
173173
findIPResponse.Payload = []*models.V1IPResponse{
174174
{
@@ -343,6 +343,7 @@ var _ = Describe("MetalStackCluster Controller", func() {
343343
"cluster.metal-stack.io/id=" + string(resource.UID),
344344
"metal-stack.infrastructure.cluster.x-k8s.io/purpose=control-plane",
345345
},
346+
Type: ptr.To(models.V1IPBaseTypeStatic),
346347
},
347348
}, nil)
348349
},

0 commit comments

Comments
 (0)