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.

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

Add checkin service files for Azure and Packet #201

Merged
merged 2 commits into from Apr 11, 2019

Conversation

Projects
None yet
6 participants
@ashcrow
Copy link
Contributor

ashcrow commented Apr 9, 2019

Adds the service unit for on boot check in.

/cc @imcleod @crawford @jlebon @lucab

Show resolved Hide resolved systemd/afterburn-checkin.service Outdated
Show resolved Hide resolved systemd/afterburn-checkin.service Outdated
Show resolved Hide resolved systemd/afterburn-checkin.service

@ashcrow ashcrow force-pushed the ashcrow:afterburn-checkin-service branch 2 times, most recently from 71cb2c6 to ef528ac Apr 9, 2019

Show resolved Hide resolved systemd/afterburn-checkin.service Outdated
Show resolved Hide resolved systemd/afterburn-checkin.service

@ashcrow ashcrow force-pushed the ashcrow:afterburn-checkin-service branch 2 times, most recently from 96cb7cb to beb1bc7 Apr 9, 2019

@crawford
Copy link
Member

crawford left a comment

Do we need an [Install] section and ConditionFirstBoot?

@bgilbert

This comment has been minimized.

Copy link
Member

bgilbert commented Apr 10, 2019

Whoops, we do need an [Install] section.

Azure needs to run on every boot (well, on some boots). I'm still partial to a separate ConditionFirstBoot unit for Packet, but @ashcrow, it looks like you decided to go the other way?

@ashcrow

This comment has been minimized.

Copy link
Contributor Author

ashcrow commented Apr 10, 2019

I'm still partial to a separate ConditionFirstBoot unit for Packet, but @ashcrow, it looks like you decided to go the other way?

I'm ok with creating a secondary unit for packet. Will update with [Install] and splitting the unit.

@ashcrow ashcrow force-pushed the ashcrow:afterburn-checkin-service branch from beb1bc7 to a51f2e7 Apr 10, 2019

@ashcrow

This comment has been minimized.

Copy link
Contributor Author

ashcrow commented Apr 10, 2019

Updated based on latest reviews.

@ashcrow ashcrow changed the title systemd: add checkin service file add checkin service files for Azure and Packet Apr 10, 2019

@ashcrow ashcrow changed the title add checkin service files for Azure and Packet Add checkin service files for Azure and Packet Apr 10, 2019

@jlebon

This comment has been minimized.

Copy link
Member

jlebon commented Apr 10, 2019

Maybe let's name the services afterburn-firstboot-checkin.service and afterburn-checkin.service? One could imagine growing the list in either of those as we teach Afterburn about more platforms that need check-ins.

ashcrow added some commits Apr 9, 2019

systemd: add checkin service file
Signed-off-by: Steve Milner <smilner@redhat.com>
systemd: add firstboot checkin service file
Signed-off-by: Steve Milner <smilner@redhat.com>

@ashcrow ashcrow force-pushed the ashcrow:afterburn-checkin-service branch from a51f2e7 to 63a42fc Apr 10, 2019

@ashcrow

This comment has been minimized.

Copy link
Contributor Author

ashcrow commented Apr 10, 2019

Updated.

@bgilbert
Copy link
Member

bgilbert left a comment

💯

@jlebon

jlebon approved these changes Apr 10, 2019

Copy link
Member

jlebon left a comment

LGTM!

@crawford
Copy link
Member

crawford left a comment

Does Azure need to check in on every boot? It probably doesn't hurt to do it that way, but I was under the impression that it only needed to report success one time.

@arithx

This comment has been minimized.

Copy link

arithx commented Apr 10, 2019

@crawford Azure needs to check in every boot that is on a different host iirc. But there is no harm in checking in every boot so it's easier to check-in every boot than to detect that.

@crawford

This comment has been minimized.

Copy link
Member

crawford commented Apr 10, 2019

@arithx Ahhh, I forgot about that restriction. LGTM

@ashcrow

This comment has been minimized.

Copy link
Contributor Author

ashcrow commented Apr 11, 2019

This is ready for merge.

@bgilbert bgilbert merged commit 8ecdfef into coreos:master Apr 11, 2019

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.