MGMT-13445: Update Assisted Installer with the new LVMS requirements

With openshift/lvm-operator#303, the resource requirements for LVMS has been reduced. Based on this reduction, this PR adjusts the LVMS requirement on Assisted Installer solving https://issues.redhat.com/browse/MGMT-13445. Signed-off-by: Suleyman Akbas <sakbas@redhat.com>
suleymanakbas91 · Feb 28, 2023 · 1d51b07 · 1d51b07
1 parent e563fc6
commit 1d51b07
Show file tree

Hide file tree

Showing 4 changed files with 100 additions and 13 deletions.
diff --git a/internal/operators/lvm/config.go b/internal/operators/lvm/config.go
@@ -7,16 +7,20 @@ import (
 const (
 	// LvmMinOpenshiftVersion is the minimum OCP version in which lvmo is supported
 	// Any changes here should be updated at line 16 too.
-	LvmoMinOpenshiftVersion string = "4.11.0"
-	LvmsMinOpenshiftVersion string = "4.12.0"
+	LvmoMinOpenshiftVersion                           string = "4.11.0"
+	LvmsMinOpenshiftVersion                           string = "4.12.0"
+	LvmsMinOpenshiftVersionForNewResourceRequirements string = "4.13.0"
 
 	LvmoSubscriptionName string = "odf-lvm-operator"
 	LvmsSubscriptionName string = "lvms-operator"
+
+	LvmsMemoryRequirement           int64 = 400
+	LvmsMemoryRequirementBefore4_13 int64 = 1200
 )
 
 type Config struct {
 	LvmCPUPerHost          int64  `envconfig:"LVM_CPU_PER_HOST" default:"1"`
-	LvmMemoryPerHostMiB    int64  `envconfig:"LVM_MEMORY_PER_HOST_MIB" default:"1200"`
+	LvmMemoryPerHostMiB    int64  `envconfig:"LVM_MEMORY_PER_HOST_MIB" default:"400"`
 	LvmMinOpenshiftVersion string `envconfig:"LVM_MIN_OPENSHIFT_VERSION" default:"4.11.0"`
 }
 

diff --git a/internal/operators/lvm/lvm_operator.go b/internal/operators/lvm/lvm_operator.go
@@ -163,7 +163,7 @@ func (o *operator) GetPreflightRequirements(context context.Context, cluster *co
 			Master: &models.HostTypeHardwareRequirements{
 				Quantitative: &models.ClusterHostRequirementsDetails{
 					CPUCores: o.config.LvmCPUPerHost,
-					RAMMib:   o.config.LvmMemoryPerHostMiB,
+					RAMMib:   o.getLvmMemoryPerHostMib(context, cluster),
 				},
 				Qualitative: []string{
 					"At least 1 non-installation disk with no partitions or filesystems",
@@ -179,3 +179,17 @@ func (o *operator) GetPreflightRequirements(context context.Context, cluster *co
 func (o *operator) GetSupportedArchitectures() []string {
 	return []string{common.X86CPUArchitecture, common.ARM64CPUArchitecture}
 }
+
+func (o *operator) getLvmMemoryPerHostMib(ctx context.Context, cluster *common.Cluster) int64 {
+	log := logutil.FromContext(ctx, o.log)
+	greaterOrEqual, err := common.BaseVersionGreaterOrEqual(cluster.OpenshiftVersion, LvmsMinOpenshiftVersionForNewResourceRequirements)
+	if err != nil {
+		log.Warnf("Error parsing cluster.OpenshiftVersion: %s, setting lvms memory requirement to %d", err.Error(), o.config.LvmMemoryPerHostMiB)
+		return o.config.LvmMemoryPerHostMiB
+	}
+	if !greaterOrEqual {
+		return LvmsMemoryRequirementBefore4_13
+	}
+
+	return o.config.LvmMemoryPerHostMiB
+}
diff --git a/internal/operators/lvm/lvm_operator_test.go b/internal/operators/lvm/lvm_operator_test.go
@@ -60,10 +60,15 @@ var _ = Describe("Lvm Operator", func() {
 			res, _ := operator.GetHostRequirements(ctx, cluster, host)
 			Expect(res).Should(Equal(expectedResult))
 		},
-			table.Entry("host",
-				&common.Cluster{Cluster: models.Cluster{Hosts: []*models.Host{hostWithSufficientResources}}},
+			table.Entry("version is 4.13",
+				&common.Cluster{Cluster: models.Cluster{OpenshiftVersion: "4.13.0", Hosts: []*models.Host{hostWithSufficientResources}}},
+				hostWithSufficientResources,
+				&models.ClusterHostRequirementsDetails{CPUCores: operator.config.LvmCPUPerHost, RAMMib: LvmsMemoryRequirement},
+			),
+			table.Entry("version is 4.12",
+				&common.Cluster{Cluster: models.Cluster{OpenshiftVersion: "4.12.0", Hosts: []*models.Host{hostWithSufficientResources}}},
 				hostWithSufficientResources,
-				&models.ClusterHostRequirementsDetails{CPUCores: operator.config.LvmCPUPerHost, RAMMib: operator.config.LvmMemoryPerHostMiB},
+				&models.ClusterHostRequirementsDetails{CPUCores: operator.config.LvmCPUPerHost, RAMMib: LvmsMemoryRequirementBefore4_13},
 			),
 		)
 	})

diff --git a/subsystem/cluster_test.go b/subsystem/cluster_test.go
@@ -3670,10 +3670,6 @@ var _ = Describe("Preflight Cluster Requirements", func() {
 			CPUCores: 6,
 			RAMMib:   conversions.GibToMib(19),
 		}
-		masterLVMRequirements = models.ClusterHostRequirementsDetails{
-			CPUCores: 1,
-			RAMMib:   1200,
-		}
 	)
 
 	BeforeEach(func() {
@@ -3714,15 +3710,83 @@ var _ = Describe("Preflight Cluster Requirements", func() {
 				Expect(*op.Requirements.Master.Quantitative).To(BeEquivalentTo(masterCNVRequirements))
 				Expect(*op.Requirements.Worker.Quantitative).To(BeEquivalentTo(workerCNVRequirements))
 			case lvm.Operator.Name:
-				Expect(*op.Requirements.Master.Quantitative).To(BeEquivalentTo(masterLVMRequirements))
-				Expect(*op.Requirements.Worker.Quantitative).To(BeEquivalentTo(models.ClusterHostRequirementsDetails{}))
+				continue // lvm operator is tested separately
 			default:
 				Fail("Unexpected operator")
 			}
 		}
 	})
 })
 
+var _ = Describe("Preflight Cluster Requirements for lvms", func() {
+	var (
+		ctx                             = context.Background()
+		masterLVMRequirementsBefore4_13 = models.ClusterHostRequirementsDetails{
+			CPUCores: 1,
+			RAMMib:   1200,
+		}
+		masterLVMRequirements = models.ClusterHostRequirementsDetails{
+			CPUCores: 1,
+			RAMMib:   400,
+		}
+	)
+	It("should be reported for 4.12 cluster", func() {
+		var cluster, err = userBMClient.Installer.V2RegisterCluster(ctx, &installer.V2RegisterClusterParams{
+			NewClusterParams: &models.ClusterCreateParams{
+				Name:              swag.String("test-cluster"),
+				OpenshiftVersion:  swag.String("4.12.0"),
+				PullSecret:        swag.String(pullSecret),
+				BaseDNSDomain:     "example.com",
+				VipDhcpAllocation: swag.Bool(true),
+			},
+		})
+		Expect(err).ToNot(HaveOccurred())
+		clusterID := *cluster.GetPayload().ID
+		params := installer.V2GetPreflightRequirementsParams{ClusterID: clusterID}
+
+		response, err := userBMClient.Installer.V2GetPreflightRequirements(ctx, &params)
+		Expect(err).ToNot(HaveOccurred())
+		requirements := response.GetPayload()
+		for _, op := range requirements.Operators {
+			switch op.OperatorName {
+			case lvm.Operator.Name:
+				Expect(*op.Requirements.Master.Quantitative).To(BeEquivalentTo(masterLVMRequirementsBefore4_13))
+				Expect(*op.Requirements.Worker.Quantitative).To(BeEquivalentTo(models.ClusterHostRequirementsDetails{}))
+			}
+		}
+		_, err = userBMClient.Installer.V2DeregisterCluster(ctx, &installer.V2DeregisterClusterParams{ClusterID: clusterID})
+		Expect(err).NotTo(HaveOccurred())
+	})
+
+	It("should be reported for 4.13 cluster", func() {
+		var cluster, err = userBMClient.Installer.V2RegisterCluster(ctx, &installer.V2RegisterClusterParams{
+			NewClusterParams: &models.ClusterCreateParams{
+				Name:              swag.String("test-cluster"),
+				OpenshiftVersion:  swag.String("4.13.0"),
+				PullSecret:        swag.String(pullSecret),
+				BaseDNSDomain:     "example.com",
+				VipDhcpAllocation: swag.Bool(true),
+			},
+		})
+		Expect(err).ToNot(HaveOccurred())
+		clusterID := *cluster.GetPayload().ID
+		params := installer.V2GetPreflightRequirementsParams{ClusterID: clusterID}
+
+		response, err := userBMClient.Installer.V2GetPreflightRequirements(ctx, &params)
+		Expect(err).ToNot(HaveOccurred())
+		requirements := response.GetPayload()
+		for _, op := range requirements.Operators {
+			switch op.OperatorName {
+			case lvm.Operator.Name:
+				Expect(*op.Requirements.Master.Quantitative).To(BeEquivalentTo(masterLVMRequirements))
+				Expect(*op.Requirements.Worker.Quantitative).To(BeEquivalentTo(models.ClusterHostRequirementsDetails{}))
+			}
+		}
+		_, err = userBMClient.Installer.V2DeregisterCluster(ctx, &installer.V2DeregisterClusterParams{ClusterID: clusterID})
+		Expect(err).NotTo(HaveOccurred())
+	})
+})
+
 var _ = Describe("Multiple-VIPs Support", func() {
 	var (
 		ctx          = context.Background()