-
Notifications
You must be signed in to change notification settings - Fork 392
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.
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
pkg/daemon: check channel closing #373
pkg/daemon: check channel closing #373
Conversation
5e5156a
to
9ca6f9d
Compare
9ca6f9d
to
4053ed5
Compare
/test unit |
4053ed5
to
2d3dac0
Compare
/test e2e-aws |
/kind bug |
/approve |
/lgtm |
On reboot, the SessionNew subscribed channel gets closed causing the node ssh/accessed annotation to be added. This patch fixes that by checking if the channel is indeed closing before going ahead and add the annotation. Signed-off-by: Antonio Murdaca <runcom@linux.com>
2d3dac0
to
6e9f76d
Compare
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ashcrow, cgwalters, jlebon, runcom, yuqi-zhang The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/retest |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest |
Fix #372, after this patch, no ssh/accessed annotation is applied on a machine config addition.
On reboot, the SessionNew subscribed channel gets closed
causing the node ssh/accessed annotation to be added.
This patch fixes that by checking if the channel is indeed
closing before going ahead and add the annotation.
Haven't added an e2e cause watching for something not to happen is gonna be the best flake ever, I'll defer to add proper testing around ssh/accessed being added on real login in a follow up.
Signed-off-by: Antonio Murdaca runcom@linux.com