MGMT-15699: Service changes for avoiding MCO reboot #5453

ori-amizur · 2023-09-03T10:28:47Z

Send the feature flag through the install command to indicate if the skip mco reboot feature is enabled.
The relevant logic is logic is done by the assisted installer.

List all the issues related to this PR

What environments does this code impact?

Automation (CI, tools, etc)
Cloud
Operator Managed Deployments
None

How was this code tested?

assisted-test-infra environment
dev-scripts environment
Reviewer's test appreciated
Waiting for CI to do a full test run
Manual (Elaborate on how it was tested)
No tests needed

Checklist

Title and description added to both, commit and PR.
Relevant issues have been associated (see CONTRIBUTING guide)
This change does not require a documentation update (docstring, docs, README, etc)
Does this change include unit-tests (note that code changes require unit-tests)

Reviewers Checklist

Are the title and description (in both PR and commit) meaningful and clear?
Is there a bug required (and linked) for this change?
Should this PR be backported?

openshift-ci-robot · 2023-09-03T10:28:50Z

codecov · 2023-09-03T10:46:11Z

Codecov Report

Merging #5453 (23f46f2) into master (6420e49) will decrease coverage by 0.08%.
Report is 34 commits behind head on master.
The diff coverage is 100.00%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #5453      +/-   ##
==========================================
- Coverage   67.74%   67.66%   -0.08%     
==========================================
  Files         229      232       +3     
  Lines       33571    34091     +520     
==========================================
+ Hits        22743    23069     +326     
- Misses       8789     8965     +176     
- Partials     2039     2057      +18

Files	Coverage Δ
internal/host/hostcommands/install_cmd.go	`82.79% <100.00%> (+0.16%)`	⬆️
internal/host/hostcommands/instruction_manager.go	`96.10% <100.00%> (ø)`

... and 11 files with indirect coverage changes

ori-amizur · 2023-09-03T13:01:08Z

/uncc @eliorerz
/uncc @adriengentil

ori-amizur · 2023-09-03T13:53:36Z

/retest

ori-amizur · 2023-09-03T14:19:46Z

/retest

ori-amizur · 2023-09-03T14:42:14Z

/retest

ori-amizur · 2023-09-03T15:09:05Z

/retest

ori-amizur · 2023-09-03T15:30:29Z

/retest

ori-amizur · 2023-09-03T20:24:13Z

/retest

ori-amizur · 2023-09-04T08:03:04Z

/uncc @gamli75
/uncc @paul-maidment

osherdp · 2023-09-04T10:15:42Z

/uncc
as I have have less feedback to give about it

ori-amizur · 2023-09-04T15:33:00Z

/retest

ori-amizur · 2023-09-04T18:26:17Z

/test edge-e2e-ai-operator-ztp
/test e2e-agent-compact-ipv4

ori-amizur · 2023-09-05T05:19:54Z

/retest

filanov · 2023-09-05T07:16:10Z

/hold
until 2.9 will be in feature freeze (7.9)

carbonin · 2023-09-05T13:35:47Z

In order to avoid MCO reboot the release image and the pull secret are needed by assisted installer. Therefore, they were added to the install command.
In addition feature flag was added. If this flag is set, the pull secret is sent in the install command.
Only if the pull secret is ent, the MCO reboot will be avoided.

I only see the feature flag being sent here. Is there more work to be done on the service side or is the pull secret stuff a detail on the installer side (i.e. the installer sets the pull secret somewhere when this flag is set)?

Also can you link to the installer PR when it is up?

ori-amizur · 2023-09-06T09:01:53Z

I only see the feature flag being sent here. Is there more work to be done on the service side?

No. AFAIK this is the only work needed on the service side.

Also can you link to the installer PR when it is up?

openshift/assisted-installer#713

openshift-ci-robot · 2023-09-06T09:04:40Z

filanov

Please add a tests that validate the argument that you pass

Send the feature flag through the install command to indicate if the skip mco reboot feature is enabled. The relevant logic is logic is done by the assisted installer.

ori-amizur · 2023-09-26T19:13:55Z

/retest

ori-amizur · 2023-09-27T04:20:37Z

/retest

ori-amizur · 2023-09-27T19:43:26Z

/retest

openshift-ci · 2023-09-28T07:46:09Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: filanov, ori-amizur

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:

~~OWNERS~~ [filanov,ori-amizur]

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

ori-amizur · 2023-09-28T07:52:14Z

/unhold

openshift-ci-robot · 2023-09-28T10:43:38Z

/retest-required

Remaining retests: 0 against base HEAD 41a00a0 and 2 for PR HEAD 23f46f2 in total

openshift-ci · 2023-09-28T12:38:38Z

@ori-amizur: all tests passed!

Full PR test history. Your PR dashboard.

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/test-infra repository. I understand the commands that are listed here.

Send the feature flag through the install command to indicate if the skip mco reboot feature is enabled. The relevant logic is logic is done by the assisted installer.

openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Sep 3, 2023

openshift-ci bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Sep 3, 2023

openshift-ci bot requested review from adriengentil and eliorerz September 3, 2023 10:29

openshift-ci bot added api-review Categorizes an issue or PR as actively needing an API review. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Sep 3, 2023

ori-amizur marked this pull request as draft September 3, 2023 13:00

openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 3, 2023

openshift-ci bot removed request for adriengentil and eliorerz September 3, 2023 13:01

ori-amizur force-pushed the MGMT-15699 branch from 567b1c6 to 7174044 Compare September 4, 2023 07:58

openshift-ci bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Sep 4, 2023

ori-amizur marked this pull request as ready for review September 4, 2023 08:00

openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 4, 2023

openshift-ci bot requested review from gamli75 and paul-maidment September 4, 2023 08:01

openshift-ci bot removed request for gamli75 and paul-maidment September 4, 2023 08:03

ori-amizur marked this pull request as draft September 4, 2023 08:03

openshift-ci bot requested review from carbonin and osherdp September 4, 2023 08:25

openshift-ci bot removed the request for review from osherdp September 4, 2023 10:15

openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 5, 2023

ori-amizur force-pushed the MGMT-15699 branch from 7174044 to bd161fb Compare September 5, 2023 13:23

ori-amizur force-pushed the MGMT-15699 branch from bd161fb to a9cfea1 Compare September 6, 2023 09:06

filanov reviewed Sep 26, 2023

View reviewed changes

MGMT-15699: Service changes for avoiding MCO reboot

23f46f2

Send the feature flag through the install command to indicate if the skip mco reboot feature is enabled. The relevant logic is logic is done by the assisted installer.

ori-amizur force-pushed the MGMT-15699 branch from a9cfea1 to 23f46f2 Compare September 26, 2023 15:32

filanov approved these changes Sep 28, 2023

View reviewed changes

openshift-ci bot assigned filanov Sep 28, 2023

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Sep 28, 2023

openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 28, 2023

openshift-merge-robot merged commit efc20a5 into openshift:master Sep 28, 2023
16 checks passed

MGMT-15699: Service changes for avoiding MCO reboot #5453

MGMT-15699: Service changes for avoiding MCO reboot #5453

Conversation

ori-amizur commented Sep 3, 2023 • edited

List all the issues related to this PR

What environments does this code impact?

How was this code tested?

Checklist

Reviewers Checklist

openshift-ci-robot commented Sep 3, 2023 • edited by openshift-ci bot

List all the issues related to this PR

What environments does this code impact?

How was this code tested?

Checklist

Reviewers Checklist

codecov bot commented Sep 3, 2023 • edited

Codecov Report

ori-amizur commented Sep 3, 2023

ori-amizur commented Sep 3, 2023

ori-amizur commented Sep 3, 2023

ori-amizur commented Sep 3, 2023

ori-amizur commented Sep 3, 2023

ori-amizur commented Sep 3, 2023

ori-amizur commented Sep 3, 2023

ori-amizur commented Sep 4, 2023

osherdp commented Sep 4, 2023 • edited

ori-amizur commented Sep 4, 2023

ori-amizur commented Sep 4, 2023

ori-amizur commented Sep 5, 2023

filanov commented Sep 5, 2023

carbonin commented Sep 5, 2023

ori-amizur commented Sep 6, 2023 • edited

openshift-ci-robot commented Sep 6, 2023 • edited by openshift-ci bot

List all the issues related to this PR

What environments does this code impact?

How was this code tested?

Checklist

Reviewers Checklist

filanov left a comment

Choose a reason for hiding this comment

ori-amizur commented Sep 26, 2023

ori-amizur commented Sep 27, 2023

ori-amizur commented Sep 27, 2023

openshift-ci bot commented Sep 28, 2023

ori-amizur commented Sep 28, 2023

openshift-ci-robot commented Sep 28, 2023

openshift-ci bot commented Sep 28, 2023

ori-amizur commented Sep 3, 2023 •

edited

openshift-ci-robot commented Sep 3, 2023 •

edited by openshift-ci bot

codecov bot commented Sep 3, 2023 •

edited

osherdp commented Sep 4, 2023 •

edited

ori-amizur commented Sep 6, 2023 •

edited

openshift-ci-robot commented Sep 6, 2023 •

edited by openshift-ci bot