Skip to content

Conversation

@lmiccini
Copy link
Contributor

This commit adds an helper label to each pod with the value of its predictableip.
This is useful if we want to then use this value to afterwards advertise the IP address using frr, instead of having to introspect the designate configmaps.

Jira: https://issues.redhat.com/browse/OSPRH-20083

This commit adds an helper label to each pod with the value of its
predictableip.
This is useful if we want to then use this value to afterwards
advertise the IP address using frr, instead of having to introspect the
designate configmaps.

Jira: https://issues.redhat.com/browse/OSPRH-20083
@openshift-ci openshift-ci bot requested review from dprince and karelyatin October 15, 2025 07:00
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Oct 15, 2025

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: lmiccini
Once this PR has been reviewed and has the lgtm label, please assign karelyatin for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@beagles
Copy link
Collaborator

beagles commented Oct 20, 2025

/lgtm cool!

@beagles beagles requested a review from omersch381 October 27, 2025 18:15
IPKeyPrefix: "bind_address_",
ServiceName: "designate-backendbind9",
}
err = designate.HandlePodLabeling(ctx, helper, instance.Name, instance.Namespace, config)
Copy link
Contributor

Choose a reason for hiding this comment

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

We call this function every reconcile loop, and if I understand correctly, we list all of the pods of the deployment and then filter for the designate ones. We might want to implement a mechanism to call it only when the configmap changes, e.g. comparing hashes as we do with the pools.yaml generation. But we can implement that in a future patch if we see that it is necessary.

@omersch381
Copy link
Contributor

I left a comment but I am ok with merging it as is. Thank you Luca for working on this PR!

@omersch381
Copy link
Contributor

/lgtm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants