Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not delete bastion floating ip if set in spec #2257

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
+16 −0
Open

Do not delete bastion floating ip if set in spec #2257

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f712e66
Do not delete bastion floating ip if set in spec
anders-elastisys Aug 16, 2024
074ff36
Update comments
anders-elastisys Dec 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions controllers/openstackcluster_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,18 @@ func (r *OpenStackClusterReconciler) deleteBastion(ctx context.Context, scope *s
return err
}

var statusFloatingIP *string
var specFloatingIP *string
if openStackCluster.Status.Bastion != nil && openStackCluster.Status.Bastion.FloatingIP != "" {
statusFloatingIP = &openStackCluster.Status.Bastion.FloatingIP
}
if openStackCluster.Spec.Bastion.FloatingIP != nil {
Copy link
Contributor

@alexandrevilain alexandrevilain Mar 11, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if openStackCluster.Spec.Bastion.FloatingIP != nil {
if openStackCluster.Spec.Bastion != nil && openStackCluster.Spec.Bastion.FloatingIP != nil {

Looks like this is the reason of failing e2e.

specFloatingIP = openStackCluster.Spec.Bastion.FloatingIP
}

// We only remove the bastion's floating IP if it exists and if it's not the same value defined both in the spec and in status.
// This decision was made so if a user specifies a pre-created floating IP that is intended to only be used for the bastion, the floating IP won't get removed once the bastion is destroyed.
if statusFloatingIP != nil && (specFloatingIP == nil || *statusFloatingIP != *specFloatingIP) {
if err = networkingService.DeleteFloatingIP(openStackCluster, openStackCluster.Status.Bastion.FloatingIP); err != nil {
handleUpdateOSCError(openStackCluster, fmt.Errorf("failed to delete floating IP: %w", err), false)
return fmt.Errorf("failed to delete floating IP: %w", err)
Expand All @@ -275,6 +286,11 @@ func (r *OpenStackClusterReconciler) deleteBastion(ctx context.Context, scope *s

for _, address := range addresses {
if address.Type == corev1.NodeExternalIP {
// If a floating IP retrieved is the same as what is set in the bastion spec, skip deleting it.
// This decision was made so if a user specifies a pre-created floating IP that is intended to only be used for the bastion, the floating IP won't get removed once the bastion is destroyed.
if specFloatingIP != nil && address.Address == *specFloatingIP {
continue
}
// Floating IP may not have properly saved in bastion status (thus not deleted above), delete any remaining floating IP
if err = networkingService.DeleteFloatingIP(openStackCluster, address.Address); err != nil {
handleUpdateOSCError(openStackCluster, fmt.Errorf("failed to delete floating IP: %w", err), false)
Expand Down