OCPBUGS-56913: Retry registration steps on service restart

Fix the bug where installConfig overrides and extra manifests are not
applied when the service restarts after finding an existing cluster.

Previously, the registerCluster() function would immediately return if
a cluster already existed, skipping the steps to apply installConfig
overrides and register extra manifests. This meant that if the service
crashed or was restarted after cluster registration but before these
steps completed, the configuration would be incomplete.

Now, registerCluster() unconditionally calls both
ApplyInstallConfigOverrides() and RegisterExtraManifests() after
obtaining the cluster (whether newly created or existing). Since both
functions are idempotent, this is safe to retry and ensures all
configuration steps complete successfully.

Add subsystem tests to verify:
- Retry of installConfig overrides on restart (idempotent)
- Application of missing overrides to existing cluster
- Retry of extra manifest registration (idempotent)

Assisted-by: Claude Code

Author: zaneb

cmd/agentbasedinstaller/client/main.go

@@ -35,6 +35,7 @@ import (
 
 	"github.com/kelseyhightower/envconfig"
 	"github.com/openshift/assisted-service/client"
+	"github.com/openshift/assisted-service/models"
 	"github.com/openshift/assisted-service/pkg/auth"
 	log "github.com/sirupsen/logrus"
 )
@@ -121,23 +122,37 @@ func registerCluster(ctx context.Context, log *log.Logger, bmInventory *client.A
 	if err != nil {
 		log.Fatal(err.Error())
 	}
+
+	var modelsCluster *models.Cluster
 	if existingCluster != nil {
-		log.Infof("Skipping cluster registration. Found existing cluster with id: %s", existingCluster.ID.String())
-		return existingCluster.ID.String()
-	}
+		log.Infof("Found existing cluster with id: %s", existingCluster.ID.String())
+		modelsCluster = existingCluster
+	} else {
+		pullSecret, err := agentbasedinstaller.GetPullSecret(RegisterOptions.PullSecretFile)
+		if err != nil {
+			log.Fatal("Failed to get pull secret: ", err.Error())
+		}
 
-	pullSecret, err := agentbasedinstaller.GetPullSecret(RegisterOptions.PullSecretFile)
-	if err != nil {
-		log.Fatal("Failed to get pull secret: ", err.Error())
+		modelsCluster, err = agentbasedinstaller.RegisterCluster(ctx, log, bmInventory, pullSecret,
+			RegisterOptions.ClusterDeploymentFile, RegisterOptions.AgentClusterInstallFile, RegisterOptions.ClusterImageSetFile,
+			RegisterOptions.ReleaseImageMirror, RegisterOptions.OperatorInstallFile, false)
+		if err != nil {
+			log.Fatal("Failed to register cluster with assisted-service: ", err)
+		}
 	}
 
-	modelsCluster, err := agentbasedinstaller.RegisterCluster(ctx, log, bmInventory, pullSecret,
-		RegisterOptions.ClusterDeploymentFile, RegisterOptions.AgentClusterInstallFile, RegisterOptions.ClusterImageSetFile,
-		RegisterOptions.ReleaseImageMirror, RegisterOptions.OperatorInstallFile, false)
+	// Apply installConfig overrides if present (idempotent)
+	log.Info("Applying installConfig overrides...")
+	updatedCluster, err := agentbasedinstaller.ApplyInstallConfigOverrides(ctx, log, bmInventory, modelsCluster, RegisterOptions.AgentClusterInstallFile)
 	if err != nil {
-		log.Fatal("Failed to register cluster with assisted-service: ", err)
+		log.Fatal("Failed to apply installConfig overrides: ", err)
+	}
+	if updatedCluster != nil {
+		modelsCluster = updatedCluster
 	}
 
+	// Register extra manifests (idempotent)
+	log.Info("Registering extra manifests...")
 	err = agentbasedinstaller.RegisterExtraManifests(os.DirFS(RegisterOptions.ExtraManifests), ctx, log, bmInventory.Manifests, modelsCluster)
 	if err != nil {
 		log.Fatal("Failed to register extra manifests with assisted-service: ", err)

subsystem/agent_based_installer_client_test.go

@@ -2,6 +2,7 @@ package subsystem
 
 import (
 	"context"
+	"os"
 
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
@@ -72,4 +73,61 @@ var _ = Describe("RegisterClusterAndInfraEnv", func() {
 		Expect(registerClusterErr).To(HaveOccurred())
 		Expect(modelCluster).To(BeNil())
 	})
+
+	It("retry installConfig overrides on restart scenario", func() {
+		// First registration with overrides
+		modelCluster, registerClusterErr := agentbasedinstaller.RegisterCluster(ctx, log, utils_test.TestContext.UserBMClient, pullSecret,
+			"../docs/hive-integration/crds/clusterDeployment.yaml",
+			"../docs/hive-integration/crds/agentClusterInstall-with-installconfig-overrides.yaml",
+			"../docs/hive-integration/crds/clusterImageSet.yaml", "", "", false)
+		Expect(registerClusterErr).NotTo(HaveOccurred())
+		Expect(modelCluster.InstallConfigOverrides).To(Equal(`{"fips": true}`))
+
+		// Simulate restart: Apply overrides again to existing cluster
+		// This should be idempotent and not fail
+		updatedCluster, err := agentbasedinstaller.ApplyInstallConfigOverrides(ctx, log, utils_test.TestContext.UserBMClient,
+			modelCluster, "../docs/hive-integration/crds/agentClusterInstall-with-installconfig-overrides.yaml")
+
+		Expect(err).NotTo(HaveOccurred())
+		// Should return nil because overrides are already correctly applied
+		Expect(updatedCluster).To(BeNil())
+	})
+
+	It("apply overrides when missing on existing cluster", func() {
+		// Register cluster without overrides first
+		modelCluster, registerClusterErr := agentbasedinstaller.RegisterCluster(ctx, log, utils_test.TestContext.UserBMClient, pullSecret,
+			"../docs/hive-integration/crds/clusterDeployment.yaml",
+			"../docs/hive-integration/crds/agentClusterInstall.yaml",
+			"../docs/hive-integration/crds/clusterImageSet.yaml", "", "", false)
+		Expect(registerClusterErr).NotTo(HaveOccurred())
+		Expect(modelCluster.InstallConfigOverrides).To(BeEmpty())
+
+		// Now apply overrides (simulating a restart where overrides should be applied)
+		updatedCluster, err := agentbasedinstaller.ApplyInstallConfigOverrides(ctx, log, utils_test.TestContext.UserBMClient,
+			modelCluster, "../docs/hive-integration/crds/agentClusterInstall-with-installconfig-overrides.yaml")
+
+		Expect(err).NotTo(HaveOccurred())
+		Expect(updatedCluster).NotTo(BeNil())
+		Expect(updatedCluster.InstallConfigOverrides).To(Equal(`{"fips": true}`))
+	})
+
+	It("retry extra manifests registration on restart", func() {
+		// Register cluster first
+		modelCluster, registerClusterErr := agentbasedinstaller.RegisterCluster(ctx, log, utils_test.TestContext.UserBMClient, pullSecret,
+			"../docs/hive-integration/crds/clusterDeployment.yaml",
+			"../docs/hive-integration/crds/agentClusterInstall.yaml",
+			"../docs/hive-integration/crds/clusterImageSet.yaml", "", "", false)
+		Expect(registerClusterErr).NotTo(HaveOccurred())
+
+		// Register extra manifests - this should be idempotent
+		// First registration
+		err := agentbasedinstaller.RegisterExtraManifests(os.DirFS("../docs/hive-integration/crds/extra-manifests"),
+			ctx, log, utils_test.TestContext.UserBMClient.Manifests, modelCluster)
+		Expect(err).NotTo(HaveOccurred())
+
+		// Retry registration (simulating restart) - should not fail
+		err = agentbasedinstaller.RegisterExtraManifests(os.DirFS("../docs/hive-integration/crds/extra-manifests"),
+			ctx, log, utils_test.TestContext.UserBMClient.Manifests, modelCluster)
+		Expect(err).NotTo(HaveOccurred())
+	})
 })