From bdda19656c37d61d0cf6bb4817bbd568ae8cf185 Mon Sep 17 00:00:00 2001 From: Martin Schuppert Date: Mon, 28 Oct 2024 15:47:07 +0100 Subject: [PATCH] Make IPSet OSclient owner check more robust If the owner of an OSIPset is an OpenStackClient resource, the rolen name of an role reservation gets set to . Right now this check is using an owner label, and queries if an OpenStackClient object exist with this name. In some circumstance this has seen to fail, most likely on the initial create and as a result it ended in two role reservations in the OSNet. ~~~ roleReservations: OpenstackClientopenstackclient: <<<<<<<<<<<< good reservation addToPredictableIPs: false reservations: - deleted: false hostname: openstackclient-0 ip: x.x.x.x serviceVIP: false vip: false ... openstackclient: <<<<<<<<<<<< wrong reservation addToPredictableIPs: false reservations: - deleted: true hostname: openstackclient-0 ip: x.x.x.x serviceVIP: false vip: false status: reservations: ... openstackclient-0: deleted: true <<<<<<<<<<<<<<<< wrong marked as deleted openstackclient above ip: x.x.x.x ~~~ Because later reconciliation have the correct role, the later one is marged as deleted and as a result the reservation in the status get set to delete: true, too. This results in the corresponding IPSet to never reach the success state. This makes the check if the owner is an OpenStackClient more robust, by checking the owner ref lists controller entry and compares if it is `OpenStackClient`. With this there is no api call required, which could fail for different issues. Closes: https://bugzilla.redhat.com/show_bug.cgi?id=2321103 Signed-off-by: Martin Schuppert --- controllers/openstacknetconfig_controller.go | 21 +++++--------------- 1 file changed, 5 insertions(+), 16 deletions(-) diff --git a/controllers/openstacknetconfig_controller.go b/controllers/openstacknetconfig_controller.go index 1c500288..cdd40d67 100644 --- a/controllers/openstacknetconfig_controller.go +++ b/controllers/openstacknetconfig_controller.go @@ -1228,22 +1228,11 @@ func (r *OpenStackNetConfigReconciler) ensureIPReservation( // // For backward compatability check if owning object is osClient and set Role to // - osClient := &ospdirectorv1beta1.OpenStackClient{} - err := r.Get(ctx, types.NamespacedName{ - Name: osIPset.Labels[common.OwnerNameLabelSelector], - Namespace: osIPset.Namespace}, - osClient) - if err != nil { - if !k8s_errors.IsNotFound(err) { - cond.Message = fmt.Sprintf("Failed to get %s %s ", osClient.Kind, osIPset.Labels[common.OwnerNameLabelSelector]) - cond.Reason = shared.OsClientCondReasonError - cond.Type = shared.CommonCondTypeError - err = common.WrapErrorForObject(cond.Message, instance, err) - - return nil, err + for _, ref := range osIPset.GetOwnerReferences() { + if ref.Controller != nil && + *ref.Controller && ref.Kind == "OpenStackClient" { + roleName = fmt.Sprintf("%s%s", openstackclient.Role, osIPset.Spec.RoleName) } - } else { - roleName = fmt.Sprintf("%s%s", openstackclient.Role, osIPset.Spec.RoleName) } allRoles[roleName] = true @@ -1251,7 +1240,7 @@ func (r *OpenStackNetConfigReconciler) ensureIPReservation( // // are there new networks added to the CR? // - err = r.ensureIPs( + err := r.ensureIPs( instance, cond, osNet,