Repository: openshift/assisted-service (language: python)

Issue:
MGMT-19194: Save Agent inventory and status in annotation
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.

<!--
Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.

You can refer to [Kubernetes community documentation] on writing good commit messages, which provides good tips and ideas.

Some PRs address specific issues. Please, refer to the [CONTRIBUTING] documentation for more
information on how to link a PR to an existing issue.

It's recommended to take a few extra minutes to provide more information about
how this code was tested. Here are some questions that may be worth answering:

- Should this PR be tested by the reviewer?
- Is this PR relying on CI for an e2e test run?
- Should this PR be tested in a specific environment?
- Any logs, screenshots, etc that can help with the review process?

-->

## List all the issues related to this PR

- [ ] New Feature <!-- new functionality -->
- [x] Enhancement <!-- refactor, code changes, improvement, that won't add new features -->
- [ ] Bug fix
- [ ] Tests
- [ ] Documentation
- [ ] CI/CD <!-- Notice that changes for Dockerfiles/Jenkinsfiles aren't tested in CI due to a known bug. -->

## What environments does this code impact?

- [ ] Automation (CI, tools, etc)
- [ ] Cloud
- [x] Operator Managed Deployments
- [ ] None

## How was this code tested?

<!-- Please, select one or more if needed: -->

- [ ] assisted-test-infra environment
- [ ] dev-scripts environment
- [ ] Reviewer's test appreciated
- [ ] Waiting for CI to do a full test run
- [x] Manual (Elaborate on how it was tested): testing done with govtech environment, see [slack thread](https://redhat-internal.slack.com/archives/C07S20C4SHX/p1729272921555049)
- [ ] No tests needed

## Checklist

- [x] Title and description added to both, commit and PR.
- [x] Relevant issues have been associated (see [CONTRIBUTING] guide)
- [x] 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?

[Kubernetes community documentation]: https://github.com/kubernetes/community/blob/master/contributors/guide/pull-requests.md#commit-message-guidelines
[CONTRIBUTING]: https://github.com/openshift/assisted-service/blob/master/CONTRIBUTING.md

/cc @danielerez @jhernand @carbonin 


Discussion / hints:

Fix patch:
diff --git a/internal/controller/controllers/agent_controller.go b/internal/controller/controllers/agent_controller.go
index 8987c3620f4..3f5216654c8 100644
--- a/internal/controller/controllers/agent_controller.go
+++ b/internal/controller/controllers/agent_controller.go
@@ -72,6 +72,8 @@ const (
 	AgentLabelHasNonrotationalDisk       = InventoryLabelPrefix + "storage-hasnonrotationaldisk"
 	AgentLabelCpuArchitecture            = InventoryLabelPrefix + "cpu-architecture"
 	AgentLabelCpuVirtEnabled             = InventoryLabelPrefix + "cpu-virtenabled"
+	AgentInventoryAnnotation             = "agent." + aiv1beta1.Group + "/inventory"
+	AgentRoleAnnotation                  = "agent." + aiv1beta1.Group + "/role"
 	AgentLabelHostManufacturer           = InventoryLabelPrefix + "host-manufacturer"
 	AgentLabelHostProductName            = InventoryLabelPrefix + "host-productname"
 	AgentLabelHostIsVirtual              = InventoryLabelPrefix + "host-isvirtual"
@@ -173,10 +175,10 @@ func (r *AgentReconciler) Reconcile(origCtx context.Context, req ctrl.Request) (
 		log.WithError(err).Warnf("failed to set infraEnv name label on agent %s/%s", agent.Namespace, agent.Name)
 	}
 
-	// Add/Update 'state' annotation (with host's status)
-	if setAgentAnnotation(log, agent, AgentStateAnnotation, swag.StringValue(h.Status)) {
+	// Add/Update Agent annotations
+	if updateAnnotations(log, agent, &h.Host) {
 		if err = r.updateAndReplaceAgent(ctx, agent); err != nil {
-			log.WithError(err).Warnf("failed to set state annotation on agent %s/%s", agent.Namespace, agent.Name)
+			log.WithError(err).Warnf("failed to set annotations on agent %s/%s", agent.Namespace, agent.Name)
 		}
 	}
 
@@ -250,6 +252,16 @@ func (r *AgentReconciler) Reconcile(origCtx context.Context, req ctrl.Request) (
 	return r.updateStatus(ctx, log, agent, origAgent, &h.Host, h.ClusterID, nil, false)
 }
 
+func updateAnnotations(log logrus.FieldLogger, agent *v1beta1.Agent, h *models.Host) bool {
+	updated := false
+	updated = setAgentAnnotation(log, agent, AgentStateAnnotation, swag.StringValue(h.Status))
+	updated = updated || setAgentAnnotation(log, agent, AgentRoleAnnotation, string(h.Role))
+	if h.Inventory != "" {
+		updated = updated || setAgentAnnotation(log, agent, AgentInventoryAnnotation, h.Inventory)
+	}
+	return updated
+}
+
 // shouldCleanUnboundSpokeNode returns true if the agent has deprovision info set and the host is not in the process of unbinding
 func shouldCleanUnboundSpokeNode(agent *aiv1beta1.Agent, host *common.Host) bool {
 	if host.Status == nil {
@@ -1812,17 +1824,10 @@ func (r *AgentReconciler) restoreHostByAgent(ctx context.Context, agent *aiv1bet
 }
 
 func createNewHost(agent *v1beta1.Agent, clusterID *strfmt.UUID, infraEnvID strfmt.UUID) (*models.Host, error) {
-	// Create Intentory
-	hostInventory := models.Inventory{}
-	manufacturer, exist := agent.Labels[AgentLabelHostManufacturer]
-	if exist {
-		hostInventory.SystemVendor = &models.SystemVendor{}
-		hostInventory.SystemVendor.Manufacturer = manufacturer
-	}
-	inventory, err := json.Marshal(hostInventory)
-	if err != nil {
-		return nil, errors.New("Failed to marshal agent's inventory")
-	}
+	// Retrieve host inventory and role
+	hostrole := agent.Annotations[AgentRoleAnnotation]
+	role := models.HostRole(hostrole)
+	inventory := agent.Annotations[AgentInventoryAnnotation]
 
 	// Fetch State
 	var hostStatus string
@@ -1843,12 +1848,12 @@ func createNewHost(agent *v1beta1.Agent, clusterID *strfmt.UUID, infraEnvID strf
 		Kind:          swag.String(models.HostKindHost),
 		RegisteredAt:  strfmt.DateTime(agent.CreationTimestamp.Time),
 		CheckedInAt:   strfmt.DateTime(agent.CreationTimestamp.Time),
-		Role:          agent.Spec.Role,
-		SuggestedRole: agent.Spec.Role,
+		Role:          role,
+		SuggestedRole: role,
 		ClusterID:     clusterID,
 		InfraEnvID:    infraEnvID,
 		Status:        &hostStatus,
-		Inventory:     string(inventory),
+		Inventory:     inventory,
 	}
 
 	return host, nil


Test patch:
diff --git a/internal/controller/controllers/agent_controller_test.go b/internal/controller/controllers/agent_controller_test.go
index 8f9ffcde204..a4f7e7d6ad8 100644
--- a/internal/controller/controllers/agent_controller_test.go
+++ b/internal/controller/controllers/agent_controller_test.go
@@ -391,13 +391,13 @@ var _ = Describe("agent reconcile", func() {
 			func(ctx context.Context, agent *v1beta1.Agent, opts ...client.UpdateOption) error {
 				return c.Update(ctx, agent)
 			},
-		).Times(3)
+		).Times(5)
 		mockClient.EXPECT().Status().Return(mockSubResourceWriter).AnyTimes()
 		mockSubResourceWriter.EXPECT().Update(gomock.Any(), gomock.AssignableToTypeOf(&v1beta1.Agent{})).DoAndReturn(
 			func(ctx context.Context, agent *v1beta1.Agent, opts ...client.UpdateOption) error {
 				return c.Status().Update(ctx, agent)
 			},
-		).Times(2)
+		).Times(4)
 		allowGetInfraEnvInternal(mockInstallerInternal, infraEnvId, "infraEnvName")
 
 		// We test 4 times to verify that agent is only updated twice
@@ -4536,7 +4536,8 @@ var _ = Describe("createNewHost", func() {
 
 	It("all properties exist", func() {
 		agent.ObjectMeta.Annotations = map[string]string{
-			AgentStateAnnotation: models.HostStatusInstalled,
+			AgentStateAnnotation:     models.HostStatusInstalled,
+			AgentInventoryAnnotation: "{\"disks\":null,\"gpus\":null,\"interfaces\":null,\"routes\":null,\"system_vendor\":{\"manufacturer\":\"RedHat\"}}",
 		}
 		agent.ObjectMeta.Labels = map[string]string{
 			AgentLabelHostManufacturer: "RedHat",
@@ -4561,14 +4562,12 @@ var _ = Describe("createNewHost", func() {
 		Expect(host.Inventory).To(Equal(string(inventory)))
 	})
 
-	It("missing host-manufacturer label", func() {
+	It("missing inventory annotation", func() {
 		host, err := createNewHost(agent, &clusterID, infraEnvID)
 		Expect(err).To(BeNil())
 
-		hostInventory := models.Inventory{}
-		inventory, err := json.Marshal(hostInventory)
 		Expect(err).To(BeNil())
-		Expect(host.Inventory).To(Equal(string(inventory)))
+		Expect(host.Inventory).To(BeEmpty())
 	})
 
 	It("missing state annotation - bound", func() {