From 96dd095872866e6a39a38a17deebd413e87df08d Mon Sep 17 00:00:00 2001 From: Zane Bitter Date: Thu, 11 Apr 2024 23:09:19 +1200 Subject: [PATCH 1/3] OCPBUGS-32105: Narrow race between bootstrap and controller If a host is in the Installed state already (which can occur when the assisted-installer-controller sets the progress to Done), don't try to set the progress to Joined as it will not only never succeed, but also take 30+ minutes of unlogged retries inside the client before an error is returned. This narrows the window in which this can occur, but if the bootstrap assisted-installer reads the Host before the assisted-installer-controller updates the status, this could still occur. Ensure any failed requests are retried by not adding the Node to the readyMasters list until the Progress has been set to either Joined or Done (the latter triggers a change of Status to Installed). Improve debugging by not logging different request_ids for messages corresponding to a single request. --- src/installer/installer.go | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/installer/installer.go b/src/installer/installer.go index 3a49c3d77..27e94e29b 100644 --- a/src/installer/installer.go +++ b/src/installer/installer.go @@ -681,16 +681,18 @@ func (i *installer) updateReadyMasters(nodes *v1.NodeList, readyMasters *[]strin ctx := utils.GenerateRequestContext() log := utils.RequestIDLogger(ctx, i.log) log.Infof("Found a new ready master node %s with id %s", node.Name, node.Status.NodeInfo.SystemUUID) - *readyMasters = append(*readyMasters, node.Name) host, ok := common.HostMatchByNameOrIPAddress(node, inventoryHostsMap, knownIpAddresses) + if ok && (host.Host.Status == nil || *host.Host.Status != models.HostStatusInstalled) { + if err := i.inventoryClient.UpdateHostInstallProgress(ctx, host.Host.InfraEnvID.String(), host.Host.ID.String(), models.HostStageJoined, ""); err != nil { + log.Errorf("Failed to update node installation status, %s", err) + continue + } + } + *readyMasters = append(*readyMasters, node.Name) if !ok { return fmt.Errorf("Node %s is not in inventory hosts", node.Name) } - ctx = utils.GenerateRequestContext() - if err := i.inventoryClient.UpdateHostInstallProgress(ctx, host.Host.InfraEnvID.String(), host.Host.ID.String(), models.HostStageJoined, ""); err != nil { - utils.RequestIDLogger(ctx, i.log).Errorf("Failed to update node installation status, %s", err) - } } } From 47f8f0b4ced9534959c09376743f83ad336a5923 Mon Sep 17 00:00:00 2001 From: Zane Bitter Date: Mon, 15 Apr 2024 21:57:42 +1200 Subject: [PATCH 2/3] OCPBUGS-32105: Reduce retries of 4xx error codes Since 4xx error codes indicate a problem on the client side, most of them cannot be usefully retried at the HTTP transport level. e.g. if a 409 Conflict is returned in response to a PUT request, then we need to fetch the resource again with a GET before creating a new PUT request. Blocking for 30+ minutes in the original PUT call (without logging) is not helpful; we want the transport to return immediately so we can try again. Retry on only those 4xx error codes where it is conceivable that trying the same request again might work. --- src/inventory_client/inventory_client.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/inventory_client/inventory_client.go b/src/inventory_client/inventory_client.go index 4042464af..f34fde176 100644 --- a/src/inventory_client/inventory_client.go +++ b/src/inventory_client/inventory_client.go @@ -117,11 +117,15 @@ func CreateInventoryClientWithDelay(clusterId string, inventoryURL string, pullS rehttp.RetryAny( rehttp.RetryAll( rehttp.RetryMaxRetries(minRetries), - rehttp.RetryStatusInterval(400, 404), + rehttp.RetryStatuses(404, 423, 425), ), rehttp.RetryAll( rehttp.RetryMaxRetries(maxRetries), - rehttp.RetryStatusInterval(405, 600), + rehttp.RetryStatuses(408, 429), + ), + rehttp.RetryAll( + rehttp.RetryMaxRetries(maxRetries), + rehttp.RetryStatusInterval(500, 600), ), rehttp.RetryAll( rehttp.RetryMaxRetries(maxRetries), From 69c4e33b0e2031cbd8ae2c01611e46ef0ac8d00c Mon Sep 17 00:00:00 2001 From: Zane Bitter Date: Wed, 10 Jul 2024 12:14:50 +1200 Subject: [PATCH 3/3] OCPBUGS-36779: Reload host inventory on conflict In a race between the assisted-installer and assisted-installer-controller to mark a control plane node as ready, the former will get a 409 Conflict response from assisted-service if the latter wins. Since f548d3284a438c8e2020253d2250620b095c70a1 we retry that check promptly. However, prior to this change we never read fresh inventory data (which would show the host has already reached the "installed" state and thus prevent us from attempting another (unnecessary) write that is doomed to fail due to a conflict. Reload the host inventory in the event of a failure to update the host status, so that we can take into account that the change may already have been performed by the assisted-installer-controller. --- src/installer/installer.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/installer/installer.go b/src/installer/installer.go index 27e94e29b..a248dc3c6 100644 --- a/src/installer/installer.go +++ b/src/installer/installer.go @@ -626,6 +626,8 @@ func (i *installer) waitForMasterNodes(ctx context.Context, minMasterNodes int, } if err = i.updateReadyMasters(nodes, &readyMasters, inventoryHostsMap); err != nil { i.log.WithError(err).Warnf("Failed to update ready with masters") + // Re-fetch inventory on next invocation + inventoryHostsMap = nil return false } i.log.Infof("Found %d ready master nodes", len(readyMasters)) @@ -686,7 +688,7 @@ func (i *installer) updateReadyMasters(nodes *v1.NodeList, readyMasters *[]strin if ok && (host.Host.Status == nil || *host.Host.Status != models.HostStatusInstalled) { if err := i.inventoryClient.UpdateHostInstallProgress(ctx, host.Host.InfraEnvID.String(), host.Host.ID.String(), models.HostStageJoined, ""); err != nil { log.Errorf("Failed to update node installation status, %s", err) - continue + return err } } *readyMasters = append(*readyMasters, node.Name)