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

Jump to bottom

Fix heartbeat for LDAP hosts #8847

Merged
merged 1 commit into from
Nov 5, 2021
Merged

Fix heartbeat for LDAP hosts #8847

merged 1 commit into from
Nov 5, 2021

Conversation

zmb3
Copy link
Collaborator

@zmb3 zmb3 commented Nov 3, 2021 •
edited
Loading

Fixes #8846

@zmb3 zmb3 mentioned this pull request Nov 3, 2021
Closed
@zmb3 zmb3 force-pushed the zmb3/heartbeat-all-ldap-hosts branch from adc761b to dfea3a4 Compare November 4, 2021 20:46
russjones
russjones approved these changes Nov 4, 2021
View reviewed changes
Copy link
Contributor

@russjones russjones left a comment

Choose a reason for hiding this comment

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

Bot.

@zmb3 zmb3 force-pushed the zmb3/heartbeat-all-ldap-hosts branch from dfea3a4 to 4e2d574 Compare November 4, 2021 21:55
@zmb3 zmb3 enabled auto-merge (rebase) November 4, 2021 21:55
@zmb3 zmb3 disabled auto-merge November 4, 2021 22:00
@zmb3 zmb3 force-pushed the zmb3/heartbeat-all-ldap-hosts branch 2 times, most recently from c7bead8 to 3641bf6 Compare November 4, 2021 23:11
@zmb3
Copy link
Collaborator Author

zmb3 commented Nov 4, 2021

@nklaassen spotted an issue with this, so I reworked it. Please take another look.

Note that making a copy may be a little extra paranoid, the entry := entry trick would also work here, but we've had a few issues recently where we captured a pointer that was changing out from under us.

@zmb3 zmb3 closed this Nov 4, 2021
@zmb3 zmb3 requested a review from nklaassen November 4, 2021 23:12
@zmb3 zmb3 reopened this Nov 4, 2021
@zmb3 zmb3 requested a review from rosstimothy November 4, 2021 23:13
@zmb3
Fix heartbeat for LDAP hosts
50331f0 
The LDAP search returns a `[]*ldap.Entry`, so make a copy of the entry
that we can close over when setting up heartbeats.

Fixes #8846
@zmb3 zmb3 force-pushed the zmb3/heartbeat-all-ldap-hosts branch from 3641bf6 to 50331f0 Compare November 5, 2021 14:45
@zmb3 zmb3 merged commit 6fa8834 into master Nov 5, 2021
@zmb3 zmb3 deleted the zmb3/heartbeat-all-ldap-hosts branch November 5, 2021 15:10
@zmb3 zmb3 mentioned this pull request Nov 5, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rosstimothy rosstimothy rosstimothy approved these changes

@xacrimon xacrimon xacrimon approved these changes

@nklaassen nklaassen nklaassen approved these changes

@russjones russjones russjones approved these changes

@r0mant r0mant r0mant approved these changes

@benarent benarent Awaiting requested review from benarent

@ibeckermayer ibeckermayer Awaiting requested review from ibeckermayer

@klizhentas klizhentas Awaiting requested review from klizhentas

Assignees
No one assigned
Labels
backport-required desktop-access
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Host Discovery will only show last item in list.
6 participants
@zmb3 @r0mant @russjones @nklaassen @xacrimon @rosstimothy