metal: add new virtio-channel for switching boot order

[Prashanth684]

Following up on comment in #1700 to split the functionality to trigger the boot order
switch to metal.go making it generic.

[cgwalters]

From what I can tell we're sending the signal and switching the boot order unconditionally on the firstboot. If that's all we need I think we already can do that because e.g. the virtio journal connection also shows we booted successfully.

Hmm right but the virtio journal connection lives in testiso.go too. Maybe it makes sense to drain that whole thing into metal.go?

If you've tested this I'm ok to merge as is as well, we can always improve more on followups. And thanks for taking this on!

[darkmuggle]

Interesting....I'm with @cgwalters on having this all in one place. But I would rather have the test in the first place.

LGTM

[Prashanth684]

From what I can tell we're sending the signal and switching the boot order unconditionally on the firstboot. If that's all we need I think we already can do that because e.g. the virtio journal connection also shows we booted successfully.

Hmm right but the virtio journal connection lives in testiso.go too. Maybe it makes sense to drain that whole thing into metal.go?

If you've tested this I'm ok to merge as is as well, we can always improve more on followups. And thanks for taking this on!

that can be done - but i'll have to pull in adding the live signal and testiso completion units to metal too along with the journal connection..is that desirable ?

[Prashanth684]

From what I can tell we're sending the signal and switching the boot order unconditionally on the firstboot. If that's all we need I think we already can do that because e.g. the virtio journal connection also shows we booted successfully.
Hmm right but the virtio journal connection lives in testiso.go too. Maybe it makes sense to drain that whole thing into metal.go?
If you've tested this I'm ok to merge as is as well, we can always improve more on followups. And thanks for taking this on!

that can be done - but i'll have to pull in adding the live signal and testiso completion units to metal too along with the journal connection..is that desirable ?

ok..never mind the above comment..i got it working by just checking for the virtio journal as suggested, but had to change the systemd unit to be similar to the live signal unit by adding dependency on coreos-installer target for the pxe case https://github.com/coreos/coreos-assembler/blob/master/mantle/cmd/kola/testiso.go#L102

I'm not sure we want to do that? is there any other way to make it run on first boot for the pxe case?

[Prashanth684]

From what I can tell we're sending the signal and switching the boot order unconditionally on the firstboot. If that's all we need I think we already can do that because e.g. the virtio journal connection also shows we booted successfully.
Hmm right but the virtio journal connection lives in testiso.go too. Maybe it makes sense to drain that whole thing into metal.go?
If you've tested this I'm ok to merge as is as well, we can always improve more on followups. And thanks for taking this on!

that can be done - but i'll have to pull in adding the live signal and testiso completion units to metal too along with the journal connection..is that desirable ?

ok..never mind the above comment..i got it working by just checking for the virtio journal as suggested, but had to change the systemd unit to be similar to the live signal unit by adding dependency on coreos-installer target for the pxe case https://github.com/coreos/coreos-assembler/blob/master/mantle/cmd/kola/testiso.go#L102

I'm not sure we want to do that? is there any other way to make it run on first boot for the pxe case?

@cgwalters ^^ thoughts?

[cgwalters]

/approve
@Prashanth684 I think I lean towards getting in your change as is and doing more work as a followup.
Still good to /lgtm here?

[Prashanth684]

/approve
@Prashanth684 I think I lean towards getting in your change as is and doing more work as a followup.
Still good to /lgtm here?

thanks ! yeah this works. good to lgtm

[jlebon]

/lgtm

Restarted CI on this to be safe.

[openshift-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cgwalters, jlebon, Prashanth684

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [cgwalters,jlebon]

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