MGMT-19194: Save Agent inventory and status in annotation by CrystalChun · Pull Request #6957 · openshift/assisted-service

CrystalChun · 2024-10-29T16:20:46Z

For Hypershift's backup and restore of hosted cluster from one hub to a new hub, the Agent requires these status fields to be restored in order for the NodePool to successfully re-adopt the Nodes for the hosted cluster.

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): testing done with govtech environment, see slack thread
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?

/cc @danielerez @jhernand @carbonin

openshift-ci-robot · 2024-10-29T16:20:50Z

openshift-ci · 2024-10-29T16:21:21Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: CrystalChun

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~~ [CrystalChun]

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

codecov · 2024-10-29T17:03:09Z

Codecov Report

Attention: Patch coverage is 93.33333% with 1 line in your changes missing coverage. Please review.

Project coverage is 68.82%. Comparing base (f3d95e1) to head (06f4d75).
Report is 2 commits behind head on master.

Files with missing lines	Patch %	Lines
...nternal/controller/controllers/agent_controller.go	93.33%	1 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #6957   +/-   ##
=======================================
  Coverage   68.81%   68.82%           
=======================================
  Files         251      251           
  Lines       37628    37630    +2     
=======================================
+ Hits        25895    25900    +5     
+ Misses       9416     9414    -2     
+ Partials     2317     2316    -1

Files with missing lines	Coverage Δ
...nternal/controller/controllers/agent_controller.go	`76.45% <93.33%> (+0.20%)`	⬆️

... and 1 file with indirect coverage changes

CrystalChun · 2024-10-29T20:57:32Z

/retest-required

CrystalChun · 2024-10-30T13:18:14Z

/retest-required

jhernand · 2024-10-30T13:53:41Z

/lgtm

openshift-ci-robot · 2024-10-30T16:20:01Z

/retest-required

Remaining retests: 0 against base HEAD 42c57d5 and 2 for PR HEAD fdcb76bfe76e2700394571360f22eafb2c0d50b9 in total

openshift-ci-robot · 2024-10-30T20:17:47Z

/retest-required

Remaining retests: 0 against base HEAD 42c57d5 and 2 for PR HEAD fdcb76bfe76e2700394571360f22eafb2c0d50b9 in total

jhernand · 2024-10-31T07:59:23Z

/retest-required

openshift-ci-robot · 2024-10-31T11:10:04Z

/retest-required

Remaining retests: 0 against base HEAD a06654b and 2 for PR HEAD fdcb76bfe76e2700394571360f22eafb2c0d50b9 in total

openshift-ci-robot · 2024-10-31T18:10:05Z

/retest-required

Remaining retests: 0 against base HEAD f3d95e1 and 1 for PR HEAD fdcb76bfe76e2700394571360f22eafb2c0d50b9 in total

jhernand · 2024-10-31T18:36:19Z

/lgtm

openshift-ci-robot · 2024-10-31T19:40:50Z

/retest-required

Remaining retests: 0 against base HEAD f3d95e1 and 2 for PR HEAD 990ec8c06a392ec8dc1375e8ea64eef7097456ca in total

For Hypershift's backup and restore of hosted cluster from one hub to a new hub, the Agent requires these status fields to be restored in order for the NodePool to successfully re-adopt the Nodes for the hosted cluster.

CrystalChun · 2024-11-01T20:02:58Z

/retest-required

openshift-ci · 2024-11-01T22:02:50Z

@CrystalChun: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/edge-e2e-ai-operator-disconnected-capi	`06f4d75`	link	false	`/test edge-e2e-ai-operator-disconnected-capi`

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.

jhernand · 2024-11-04T08:20:03Z

/lgtm

danielerez · 2024-11-04T09:53:23Z

+	updated := false
+	updated = setAgentAnnotation(log, agent, AgentStateAnnotation, swag.StringValue(h.Status))
+	updated = updated || setAgentAnnotation(log, agent, AgentRoleAnnotation, string(h.Role))
+	if h.Inventory != "" {


Just a minor nit for later, is it crucial to check if inventory is empty?
If not, we can make this func nicer to read I think.
I.e.

updated := false updated = setAgentAnnotation(log, agent, AgentStateAnnotation, swag.StringValue(h.Status) updated = setAgentAnnotation(log, agent, AgentRoleAnnotation, string(h.Role)) || updated updated = setAgentAnnotation(log, agent, AgentInventoryAnnotation, h.Inventory) || updated

Great suggestion! Will work that into a new PR. Thanks Daniel!!

Small PR with changes: #6967

openshift-bot · 2024-11-04T14:16:08Z

[ART PR BUILD NOTIFIER]

Distgit: ose-agent-installer-api-server
This PR has been included in build ose-agent-installer-api-server-container-v4.18.0-202411041337.p0.ge971390.assembly.stream.el9.
All builds following this will include this PR.

Minor code cleanup as suggested in openshift#6957 (comment)

CrystalChun · 2024-11-04T20:36:16Z

/cherry-pick release-ocm-2.12

openshift-cherrypick-robot · 2024-11-04T20:37:06Z

@CrystalChun: new pull request created: #6968

Details

In response to this:

/cherry-pick release-ocm-2.12

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.

Minor code cleanup as suggested in #6957 (comment)

…6957) For Hypershift's backup and restore of hosted cluster from one hub to a new hub, the Agent requires these status fields to be restored in order for the NodePool to successfully re-adopt the Nodes for the hosted cluster.

* MGMT-18635: Restore missing Host by Agent CR (#6730) When applying an Agent CR with a missing associated host, the Agnet CR is currently being deleted by the reconciler. This scenario would happen on restore to a new hub flows since the host is a DB only entity. Thus, this change introduces handling of a missing host by restoring the object into DB according to properties from the Agent CR. For restoring the host's status, added 'state' annotation to the agent which is set on reconcile. See full flow and more details in the enhancement: https://github.com/openshift/assisted-service/blob/master/docs/enhancements/backup-restore-support.md#solution-for-deleted-agent-crs * MGMT-19194: Save Agent inventory and status in annotation (#6957) For Hypershift's backup and restore of hosted cluster from one hub to a new hub, the Agent requires these status fields to be restored in order for the NodePool to successfully re-adopt the Nodes for the hosted cluster. --------- Co-authored-by: Daniel Erez <danielerez@gmail.com>

openshift-ci Bot requested review from carbonin, danielerez and jhernand October 29, 2024 16:20

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

openshift-ci Bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Oct 29, 2024

openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 29, 2024

openshift-ci Bot assigned jhernand Oct 30, 2024

openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Oct 30, 2024

CrystalChun force-pushed the testing-govtech-2 branch from fdcb76b to 990ec8c Compare October 31, 2024 18:23

openshift-ci Bot removed the lgtm Indicates that a PR is ready to be merged. label Oct 31, 2024

openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Oct 31, 2024

CrystalChun force-pushed the testing-govtech-2 branch from 990ec8c to 88385c7 Compare October 31, 2024 21:32

openshift-ci Bot removed the lgtm Indicates that a PR is ready to be merged. label Oct 31, 2024

MGMT-19194: Save Agent inventory and status in annotation

06f4d75

For Hypershift's backup and restore of hosted cluster from one hub to a new hub, the Agent requires these status fields to be restored in order for the NodePool to successfully re-adopt the Nodes for the hosted cluster.

CrystalChun force-pushed the testing-govtech-2 branch from 88385c7 to 06f4d75 Compare November 1, 2024 16:07

openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Nov 4, 2024

danielerez reviewed Nov 4, 2024

View reviewed changes

openshift-merge-bot Bot merged commit e971390 into openshift:master Nov 4, 2024

CrystalChun mentioned this pull request Nov 4, 2024

MGMT-19194: Cleanup adding inventory to Agent annotation #6967

Merged

6 of 20 tasks

CrystalChun added a commit to CrystalChun/assisted-service that referenced this pull request Nov 4, 2024

MGMT-19194: Cleanup adding inventory to Agent annotation

46bef9f

Minor code cleanup as suggested in openshift#6957 (comment)

openshift-cherrypick-robot mentioned this pull request Nov 4, 2024

[release-ocm-2.12] MGMT-19194: Save Agent inventory and status in annotation #6968

Merged

CrystalChun mentioned this pull request Nov 5, 2024

[release-ocm-2.11] MGMT-19194: Restore hosts using Agent CRs #6973

Merged

7 of 20 tasks

openshift-merge-bot Bot pushed a commit that referenced this pull request Nov 5, 2024

MGMT-19194: Cleanup adding inventory to Agent annotation (#6967)

cfe8887

Minor code cleanup as suggested in #6957 (comment)

CrystalChun deleted the testing-govtech-2 branch July 25, 2025 16:07

Conversation

CrystalChun commented Oct 29, 2024

List all the issues related to this PR

What environments does this code impact?

How was this code tested?

Checklist

Reviewers Checklist

Uh oh!

openshift-ci-robot commented Oct 29, 2024 • edited by openshift-ci Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

List all the issues related to this PR

What environments does this code impact?

How was this code tested?

Checklist

Reviewers Checklist

Uh oh!

openshift-ci Bot commented Oct 29, 2024

Uh oh!

codecov Bot commented Oct 29, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

CrystalChun commented Oct 29, 2024

Uh oh!

CrystalChun commented Oct 30, 2024

Uh oh!

jhernand commented Oct 30, 2024

Uh oh!

openshift-ci-robot commented Oct 30, 2024

Uh oh!

openshift-ci-robot commented Oct 30, 2024

Uh oh!

jhernand commented Oct 31, 2024

Uh oh!

openshift-ci-robot commented Oct 31, 2024

Uh oh!

openshift-ci-robot commented Oct 31, 2024

Uh oh!

jhernand commented Oct 31, 2024

Uh oh!

openshift-ci-robot commented Oct 31, 2024

Uh oh!

CrystalChun commented Nov 1, 2024

Uh oh!

openshift-ci Bot commented Nov 1, 2024

Uh oh!

jhernand commented Nov 4, 2024

Uh oh!

danielerez Nov 4, 2024

Choose a reason for hiding this comment

Uh oh!

CrystalChun Nov 4, 2024

Choose a reason for hiding this comment

Uh oh!

CrystalChun Nov 4, 2024

Choose a reason for hiding this comment

Uh oh!

openshift-bot commented Nov 4, 2024

Uh oh!

CrystalChun commented Nov 4, 2024

Uh oh!

openshift-cherrypick-robot commented Nov 4, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

openshift-ci-robot commented Oct 29, 2024 •

edited by openshift-ci Bot

Loading

codecov Bot commented Oct 29, 2024 •

edited

Loading