Skip to content

USHIFT-6469: Propagate service failures for correct exit code#6150

Merged
openshift-merge-bot[bot] merged 1 commit intoopenshift:mainfrom
pmtk:avoid-protocol-error
Jan 29, 2026
Merged

USHIFT-6469: Propagate service failures for correct exit code#6150
openshift-merge-bot[bot] merged 1 commit intoopenshift:mainfrom
pmtk:avoid-protocol-error

Conversation

@pmtk
Copy link
Member

@pmtk pmtk commented Jan 26, 2026

Changes the way of restarting MicroShift in case of service failure. Previously ServiceManager would send a SIGTERM to MicroShift process to trigger shutdown, however that carries no context whatsoever.

When service was failing to start and MicroShift quit before sending READY message, systemd would report MicroShift as failing with (readiness notification) protocol error which might be confusing.

To avoid protocol error in case of quiting before readiness, MicroShift needs to exit with non-0 code which will happen if the RunMicroShift() function returns an error.

@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Jan 26, 2026
@openshift-ci-robot
Copy link

openshift-ci-robot commented Jan 26, 2026

@pmtk: This pull request references USHIFT-6469 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the bug to target the "4.22.0" version, but no target version was set.

Details

In response to this:

Changes the way of restarting MicroShift in case of service failure. Previously ServiceManager would send a SIGTERM to MicroShift process to trigger shutdown, however that carries no context whatsoever.

When service was failing to start and MicroShift quit before sending READY message, systemd would report MicroShift as failing with (readiness notification) protocol error which might be confusing.

To avoid protocol error in case of quiting before readiness, MicroShift needs to exit with non-0 code which will happen if the RunMicroShift() function returns an error.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

@openshift-ci openshift-ci bot requested review from jogeo and kasturinarra January 26, 2026 12:00
@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 26, 2026
@pmtk pmtk force-pushed the avoid-protocol-error branch 2 times, most recently from a94e967 to 4bc8afb Compare January 26, 2026 14:41
services: []Service{},
serviceMap: make(map[string]Service),
startRec: startRec,
ErrChan: make(chan error, 1),
Copy link
Contributor

Choose a reason for hiding this comment

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

Can multiple services fail at the same time? This would only pick up one.

Copy link
Member Author

Choose a reason for hiding this comment

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

But how do you wait for many on the other side if one is enough to kill the process?
And that could pollute an image - when first svc fail, the rest is canceled so they could fail with something unrelated

@pmtk pmtk force-pushed the avoid-protocol-error branch from 4bc8afb to b31b6cc Compare January 27, 2026 12:18
Changes the way of restarting MicroShift in case of service failure.
Previously ServiceManager would send a SIGTERM to MicroShift process to
trigger shutdown, however that carries no context whatsoever.

When service was failing to start and MicroShift quit before sending
READY message, systemd would report MicroShift as failing with
(readiness notification) protocol error which might be confusing.

To avoid protocol error in case of quiting before readiness,
MicroShft needs to exit with non-0 code which will happen if the
RunMicroShift() function returns an error.
@pmtk pmtk force-pushed the avoid-protocol-error branch from b31b6cc to 99f5c97 Compare January 27, 2026 16:24
Copy link
Contributor

@pacevedom pacevedom left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jan 29, 2026
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jan 29, 2026

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: pacevedom, pmtk

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:

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

@pmtk
Copy link
Member Author

pmtk commented Jan 29, 2026

/verified by @pmtk

@openshift-ci-robot openshift-ci-robot added the verified Signifies that the PR passed pre-merge verification criteria label Jan 29, 2026
@openshift-ci-robot
Copy link

@pmtk: This PR has been marked as verified by @pmtk.

Details

In response to this:

/verified by @pmtk

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jan 29, 2026

@pmtk: all tests passed!

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

@openshift-merge-bot openshift-merge-bot bot merged commit 8f6f4d1 into openshift:main Jan 29, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. lgtm Indicates that a PR is ready to be merged. verified Signifies that the PR passed pre-merge verification criteria

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants