Skip to content

Commit

Permalink
Make IPSet OSclient owner check more robust
Browse files Browse the repository at this point in the history
If the owner of an OSIPset is an OpenStackClient resource, the
rolen name of an role reservation gets set to
<openstackclient.Role><instance.Name>. 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 <[email protected]>
  • Loading branch information
stuggi authored and openshift-cherrypick-robot committed Oct 29, 2024
1 parent 7b930b5 commit bdda196
Showing 1 changed file with 5 additions and 16 deletions.
21 changes: 5 additions & 16 deletions controllers/openstacknetconfig_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -1228,30 +1228,19 @@ func (r *OpenStackNetConfigReconciler) ensureIPReservation(
//
// For backward compatability check if owning object is osClient and set Role to <openstackclient.Role><instance.Name>
//
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

//
// are there new networks added to the CR?
//
err = r.ensureIPs(
err := r.ensureIPs(
instance,
cond,
osNet,
Expand Down

0 comments on commit bdda196

Please sign in to comment.