From 1f406085b8e7074ec6f67a6c13644d7e64905c82 Mon Sep 17 00:00:00 2001 From: Riccardo Piccoli Date: Thu, 4 Apr 2024 14:04:34 +0200 Subject: [PATCH] MGMT-17438: bugfix, make sure ODF and LVM requirements do not change if MCE is not selected Signed-off-by: Riccardo Piccoli --- internal/operators/api/api.go | 3 +- internal/operators/api/mock_operator_api.go | 8 +- internal/operators/cnv/cnv_operator.go | 2 +- internal/operators/cnv/cnv_operator_test.go | 2 +- internal/operators/lso/ls_operator.go | 2 +- internal/operators/lvm/lvm_operator.go | 23 +- internal/operators/lvm/lvm_operator_test.go | 247 +++++++++----------- internal/operators/manager.go | 14 +- internal/operators/manager_test.go | 97 +++++++- internal/operators/mce/mce_operator.go | 2 +- internal/operators/mce/mce_operator_test.go | 2 +- internal/operators/odf/odf_operator.go | 31 ++- internal/operators/odf/odf_operator_test.go | 4 +- internal/operators/odf/validations.go | 2 +- 14 files changed, 245 insertions(+), 194 deletions(-) diff --git a/internal/operators/api/api.go b/internal/operators/api/api.go index 99dca41e6ddc..f93cc801f01b 100644 --- a/internal/operators/api/api.go +++ b/internal/operators/api/api.go @@ -38,7 +38,7 @@ type Operator interface { // ValidateCluster verifies whether this operator is valid for given cluster ValidateCluster(ctx context.Context, cluster *common.Cluster) (ValidationResult, error) // ValidateHost verifies whether this operator is valid for given host - ValidateHost(ctx context.Context, cluster *common.Cluster, hosts *models.Host) (ValidationResult, error) + ValidateHost(ctx context.Context, cluster *common.Cluster, hosts *models.Host, additionalOperatorRequirements *models.ClusterHostRequirementsDetails) (ValidationResult, error) // GenerateManifests generates manifests for the operator GenerateManifests(*common.Cluster) (map[string][]byte, []byte, error) // GetHostRequirements provides operator's requirements towards the host @@ -61,5 +61,4 @@ type Operator interface { type StorageOperator interface { Operator StorageClassName() string - SetAdditionalDiskRequirements(additionalSizeGB int64) } diff --git a/internal/operators/api/mock_operator_api.go b/internal/operators/api/mock_operator_api.go index fe51a522453e..691a9d9d564b 100644 --- a/internal/operators/api/mock_operator_api.go +++ b/internal/operators/api/mock_operator_api.go @@ -211,16 +211,16 @@ func (mr *MockOperatorMockRecorder) ValidateCluster(arg0, arg1 interface{}) *gom } // ValidateHost mocks base method. -func (m *MockOperator) ValidateHost(arg0 context.Context, arg1 *common.Cluster, arg2 *models.Host) (ValidationResult, error) { +func (m *MockOperator) ValidateHost(arg0 context.Context, arg1 *common.Cluster, arg2 *models.Host, arg3 *models.ClusterHostRequirementsDetails) (ValidationResult, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "ValidateHost", arg0, arg1, arg2) + ret := m.ctrl.Call(m, "ValidateHost", arg0, arg1, arg2, arg3) ret0, _ := ret[0].(ValidationResult) ret1, _ := ret[1].(error) return ret0, ret1 } // ValidateHost indicates an expected call of ValidateHost. -func (mr *MockOperatorMockRecorder) ValidateHost(arg0, arg1, arg2 interface{}) *gomock.Call { +func (mr *MockOperatorMockRecorder) ValidateHost(arg0, arg1, arg2, arg3 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ValidateHost", reflect.TypeOf((*MockOperator)(nil).ValidateHost), arg0, arg1, arg2) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ValidateHost", reflect.TypeOf((*MockOperator)(nil).ValidateHost), arg0, arg1, arg2, arg3) } diff --git a/internal/operators/cnv/cnv_operator.go b/internal/operators/cnv/cnv_operator.go index 2535af29ee25..7f93738e6d6f 100644 --- a/internal/operators/cnv/cnv_operator.go +++ b/internal/operators/cnv/cnv_operator.go @@ -99,7 +99,7 @@ func (o *operator) validateRequirements(cluster *models.Cluster) (api.Validation } // ValidateHost returns validationResult based on node type requirements such as memory and cpu -func (o *operator) ValidateHost(ctx context.Context, cluster *common.Cluster, host *models.Host) (api.ValidationResult, error) { +func (o *operator) ValidateHost(ctx context.Context, cluster *common.Cluster, host *models.Host, _ *models.ClusterHostRequirementsDetails) (api.ValidationResult, error) { if host.Inventory == "" { o.log.Info("Empty Inventory of host with hostID ", host.ID) return api.ValidationResult{Status: api.Pending, ValidationId: o.GetHostValidationID(), Reasons: []string{"Missing Inventory in some of the hosts"}}, nil diff --git a/internal/operators/cnv/cnv_operator_test.go b/internal/operators/cnv/cnv_operator_test.go index 2e27b98159a1..b8e7dbda020f 100644 --- a/internal/operators/cnv/cnv_operator_test.go +++ b/internal/operators/cnv/cnv_operator_test.go @@ -280,7 +280,7 @@ var _ = Describe("CNV operator", func() { {SizeBytes: 20 * conversions.GiB, DriveType: models.DriveTypeSSD, ID: "disk3"}, })} table.DescribeTable("validateHost when ", func(cluster *common.Cluster, host *models.Host, expectedResult api.ValidationResult) { - res, _ := cnvOperator.ValidateHost(context.TODO(), cluster, host) + res, _ := cnvOperator.ValidateHost(context.TODO(), cluster, host, nil) Expect(res).Should(Equal(expectedResult)) }, table.Entry("No virt capabilities", diff --git a/internal/operators/lso/ls_operator.go b/internal/operators/lso/ls_operator.go index 6b64e5300c8b..8248fac7c5b9 100644 --- a/internal/operators/lso/ls_operator.go +++ b/internal/operators/lso/ls_operator.go @@ -55,7 +55,7 @@ func (l *lsOperator) ValidateCluster(_ context.Context, _ *common.Cluster) (api. } // ValidateHost always return "valid" result -func (l *lsOperator) ValidateHost(_ context.Context, _ *common.Cluster, _ *models.Host) (api.ValidationResult, error) { +func (l *lsOperator) ValidateHost(_ context.Context, _ *common.Cluster, _ *models.Host, _ *models.ClusterHostRequirementsDetails) (api.ValidationResult, error) { return api.ValidationResult{Status: api.Success, ValidationId: l.GetHostValidationID(), Reasons: []string{}}, nil } diff --git a/internal/operators/lvm/lvm_operator.go b/internal/operators/lvm/lvm_operator.go index 0c63f92064d3..3422073e6f09 100644 --- a/internal/operators/lvm/lvm_operator.go +++ b/internal/operators/lvm/lvm_operator.go @@ -17,10 +17,9 @@ import ( // operator is an ODF LVM OLM operator plugin; it implements api.Operator type operator struct { - log logrus.FieldLogger - Config *Config - extracter oc.Extracter - additionalDiskRequirementGB int64 + log logrus.FieldLogger + Config *Config + extracter oc.Extracter } const defaultStorageClassName = "lvms-" + defaultDeviceName @@ -65,10 +64,6 @@ func (o *operator) StorageClassName() string { return defaultStorageClassName } -func (o *operator) SetAdditionalDiskRequirements(additionalSizeGB int64) { - o.additionalDiskRequirementGB = additionalSizeGB -} - // GetDependencies provides a list of dependencies of the Operator func (o *operator) GetDependencies(cluster *common.Cluster) ([]string, error) { return make([]string, 0), nil @@ -101,7 +96,7 @@ func (o *operator) ValidateCluster(_ context.Context, cluster *common.Cluster) ( } // ValidateHost always return "valid" result -func (o *operator) ValidateHost(ctx context.Context, cluster *common.Cluster, host *models.Host) (api.ValidationResult, error) { +func (o *operator) ValidateHost(ctx context.Context, cluster *common.Cluster, host *models.Host, additionalOperatorRequirements *models.ClusterHostRequirementsDetails) (api.ValidationResult, error) { if host.Inventory == "" { message := "Missing Inventory in the host" return api.ValidationResult{Status: api.Pending, ValidationId: o.GetHostValidationID(), Reasons: []string{message}}, nil @@ -113,13 +108,17 @@ func (o *operator) ValidateHost(ctx context.Context, cluster *common.Cluster, ho return api.ValidationResult{Status: api.Failure, ValidationId: o.GetHostValidationID(), Reasons: []string{message}}, err } - diskCount, _ := operatorscommon.NonInstallationDiskCount(inventory.Disks, host.InstallationDiskID, o.additionalDiskRequirementGB) + minDiskSizeGb := int64(0) + if additionalOperatorRequirements != nil { + minDiskSizeGb = additionalOperatorRequirements.DiskSizeGb + } + diskCount, _ := operatorscommon.NonInstallationDiskCount(inventory.Disks, host.InstallationDiskID, minDiskSizeGb) role := common.GetEffectiveRole(host) areSchedulable := common.AreMastersSchedulable(cluster) minSizeMessage := "" - if o.additionalDiskRequirementGB > 0 { - minSizeMessage = fmt.Sprintf(" of %dGB minimum", o.additionalDiskRequirementGB) + if minDiskSizeGb > 0 { + minSizeMessage = fmt.Sprintf(" of %dGB minimum", minDiskSizeGb) } message := fmt.Sprintf("Logical Volume Manager requires at least one non-installation HDD/SSD disk%s on the host", minSizeMessage) diff --git a/internal/operators/lvm/lvm_operator_test.go b/internal/operators/lvm/lvm_operator_test.go index adc4e7a2af23..692364b11d49 100644 --- a/internal/operators/lvm/lvm_operator_test.go +++ b/internal/operators/lvm/lvm_operator_test.go @@ -349,160 +349,136 @@ var _ = Describe("Lvm Operator", func() { } }) hostValidationTests := []struct { - name string - ocpVersion string - hosts []*models.Host - resultMessage []string - apiStatus api.ValidationStatus - hostRole models.HostRole - HighAvailabilityMode string - diskCount int - hostCPU int64 - hostMem float32 - additionalDiskRequirementGB int64 + name string + ocpVersion string + hosts []*models.Host + resultMessage []string + apiStatus api.ValidationStatus + hostRole models.HostRole + HighAvailabilityMode string + diskCount int + hostCPU int64 + hostMem float32 }{ { - name: "SNO with sufficient resources, 4.12", - ocpVersion: "4.12.0", - hosts: []*models.Host{}, - resultMessage: nil, - apiStatus: api.Success, - diskCount: 2, - hostCPU: 9, - HighAvailabilityMode: models.ClusterHighAvailabilityModeNone, - hostRole: models.HostRoleAutoAssign, - hostMem: 16*conversions.GiB + float32(lvmMemMB_pre4_13), - additionalDiskRequirementGB: int64(0), - }, - { - name: "SNO with sufficient resources", - ocpVersion: "4.13.0", - hosts: []*models.Host{}, - resultMessage: nil, - apiStatus: api.Success, - diskCount: 2, - hostCPU: 9, - HighAvailabilityMode: models.ClusterHighAvailabilityModeNone, - hostRole: models.HostRoleAutoAssign, - hostMem: 16*conversions.GiB + float32(lvmMemMB), - additionalDiskRequirementGB: int64(0), + name: "SNO with sufficient resources, 4.12", + ocpVersion: "4.12.0", + hosts: []*models.Host{}, + resultMessage: nil, + apiStatus: api.Success, + diskCount: 2, + hostCPU: 9, + HighAvailabilityMode: models.ClusterHighAvailabilityModeNone, + hostRole: models.HostRoleAutoAssign, + hostMem: 16*conversions.GiB + float32(lvmMemMB_pre4_13), }, { - name: "SNO with insufficient disks", - ocpVersion: "4.13.0", - hosts: []*models.Host{}, - resultMessage: []string{"Logical Volume Manager requires at least one non-installation HDD/SSD disk on the host"}, - apiStatus: api.Failure, - diskCount: 1, - hostCPU: 9, - HighAvailabilityMode: models.ClusterHighAvailabilityModeNone, - hostRole: models.HostRoleAutoAssign, - hostMem: 16*conversions.GiB + float32(lvmMemMB), - additionalDiskRequirementGB: int64(0), + name: "SNO with sufficient resources", + ocpVersion: "4.13.0", + hosts: []*models.Host{}, + resultMessage: nil, + apiStatus: api.Success, + diskCount: 2, + hostCPU: 9, + HighAvailabilityMode: models.ClusterHighAvailabilityModeNone, + hostRole: models.HostRoleAutoAssign, + hostMem: 16*conversions.GiB + float32(lvmMemMB), }, { - name: "Compact version 4.15, HostRoleAutoAssign", - ocpVersion: "4.15.0", - hosts: []*models.Host{masterCompactNode, masterCompactNode}, - resultMessage: nil, - apiStatus: api.Success, - diskCount: 2, - hostCPU: 7, - HighAvailabilityMode: models.ClusterHighAvailabilityModeFull, - hostRole: models.HostRoleAutoAssign, - hostMem: 24*conversions.GiB + float32(lvmMemMB), - additionalDiskRequirementGB: int64(0), + name: "SNO with insufficient disks", + ocpVersion: "4.13.0", + hosts: []*models.Host{}, + resultMessage: []string{"Logical Volume Manager requires at least one non-installation HDD/SSD disk on the host"}, + apiStatus: api.Failure, + diskCount: 1, + hostCPU: 9, + HighAvailabilityMode: models.ClusterHighAvailabilityModeNone, + hostRole: models.HostRoleAutoAssign, + hostMem: 16*conversions.GiB + float32(lvmMemMB), }, { - name: "Compact version 4.15, HostRoleMaster", - ocpVersion: "4.15.0", - hosts: []*models.Host{masterCompactNode, masterCompactNode}, - resultMessage: nil, - apiStatus: api.Success, - diskCount: 2, - hostCPU: 7, - HighAvailabilityMode: models.ClusterHighAvailabilityModeFull, - hostRole: models.HostRoleMaster, - hostMem: 24*conversions.GiB + float32(lvmMemMB), - additionalDiskRequirementGB: int64(0), + name: "Compact version 4.15, HostRoleAutoAssign", + ocpVersion: "4.15.0", + hosts: []*models.Host{masterCompactNode, masterCompactNode}, + resultMessage: nil, + apiStatus: api.Success, + diskCount: 2, + hostCPU: 7, + HighAvailabilityMode: models.ClusterHighAvailabilityModeFull, + hostRole: models.HostRoleAutoAssign, + hostMem: 24*conversions.GiB + float32(lvmMemMB), }, { - name: "Compact version 4.15, Worker", - ocpVersion: "4.15.0", - hosts: []*models.Host{masterCompactNode, masterCompactNode}, - resultMessage: nil, - apiStatus: api.Success, - diskCount: 2, - hostCPU: 7, - HighAvailabilityMode: models.ClusterHighAvailabilityModeFull, - hostRole: models.HostRoleWorker, - hostMem: 24*conversions.GiB + float32(lvmMemMB), - additionalDiskRequirementGB: int64(0), + name: "Compact version 4.15, HostRoleMaster", + ocpVersion: "4.15.0", + hosts: []*models.Host{masterCompactNode, masterCompactNode}, + resultMessage: nil, + apiStatus: api.Success, + diskCount: 2, + hostCPU: 7, + HighAvailabilityMode: models.ClusterHighAvailabilityModeFull, + hostRole: models.HostRoleMaster, + hostMem: 24*conversions.GiB + float32(lvmMemMB), }, { - name: "full version 4.15, Master", - ocpVersion: "4.15.0", - hosts: []*models.Host{masterNode, masterNode, workerNode, workerNode, workerNode}, - resultMessage: nil, - apiStatus: api.Success, - diskCount: 1, - hostCPU: 4, - HighAvailabilityMode: models.ClusterHighAvailabilityModeFull, - hostRole: models.HostRoleMaster, - hostMem: 16 * conversions.GiB, - additionalDiskRequirementGB: int64(0), + name: "Compact version 4.15, Worker", + ocpVersion: "4.15.0", + hosts: []*models.Host{masterCompactNode, masterCompactNode}, + resultMessage: nil, + apiStatus: api.Success, + diskCount: 2, + hostCPU: 7, + HighAvailabilityMode: models.ClusterHighAvailabilityModeFull, + hostRole: models.HostRoleWorker, + hostMem: 24*conversions.GiB + float32(lvmMemMB), }, { - name: "full version 4.15, Worker", - ocpVersion: "4.15.0", - hosts: []*models.Host{masterNode, masterNode, masterNode, workerNode, workerNode}, - resultMessage: nil, - apiStatus: api.Success, - diskCount: 2, - hostCPU: 3, - HighAvailabilityMode: models.ClusterHighAvailabilityModeFull, - hostRole: models.HostRoleWorker, - hostMem: 8*conversions.GiB + float32(lvmMemMB), - additionalDiskRequirementGB: int64(0), + name: "full version 4.15, Master", + ocpVersion: "4.15.0", + hosts: []*models.Host{masterNode, masterNode, workerNode, workerNode, workerNode}, + resultMessage: nil, + apiStatus: api.Success, + diskCount: 1, + hostCPU: 4, + HighAvailabilityMode: models.ClusterHighAvailabilityModeFull, + hostRole: models.HostRoleMaster, + hostMem: 16 * conversions.GiB, }, { - name: "full version 4.15, Worker HostRoleAutoAssign", - ocpVersion: "4.15.0", - hosts: []*models.Host{masterNode, masterNode, masterNode, workerNode, workerNode}, - resultMessage: []string{"For Logical Volume Manager Standard Mode, host role must be assigned to master or worker."}, - apiStatus: api.Failure, - diskCount: 2, - hostCPU: 3, - HighAvailabilityMode: models.ClusterHighAvailabilityModeFull, - hostRole: models.HostRoleAutoAssign, - hostMem: 8*conversions.GiB + float32(lvmMemMB), - additionalDiskRequirementGB: int64(0), + name: "full version 4.15, Worker", + ocpVersion: "4.15.0", + hosts: []*models.Host{masterNode, masterNode, masterNode, workerNode, workerNode}, + resultMessage: nil, + apiStatus: api.Success, + diskCount: 2, + hostCPU: 3, + HighAvailabilityMode: models.ClusterHighAvailabilityModeFull, + hostRole: models.HostRoleWorker, + hostMem: 8*conversions.GiB + float32(lvmMemMB), }, { - name: "full version 4.15, Worker, insufficient Disk", - ocpVersion: "4.15.0", - hosts: []*models.Host{masterNode, masterNode, masterNode, workerNode, workerNode}, - resultMessage: []string{"Logical Volume Manager requires at least one non-installation HDD/SSD disk on the host"}, - apiStatus: api.Failure, - diskCount: 1, - hostCPU: 3, - HighAvailabilityMode: models.ClusterHighAvailabilityModeFull, - hostRole: models.HostRoleWorker, - hostMem: 8*conversions.GiB + float32(lvmMemMB), - additionalDiskRequirementGB: int64(0), + name: "full version 4.15, Worker HostRoleAutoAssign", + ocpVersion: "4.15.0", + hosts: []*models.Host{masterNode, masterNode, masterNode, workerNode, workerNode}, + resultMessage: []string{"For Logical Volume Manager Standard Mode, host role must be assigned to master or worker."}, + apiStatus: api.Failure, + diskCount: 2, + hostCPU: 3, + HighAvailabilityMode: models.ClusterHighAvailabilityModeFull, + hostRole: models.HostRoleAutoAssign, + hostMem: 8*conversions.GiB + float32(lvmMemMB), }, { - name: "full version 4.15, Worker, insufficient Disk size", - ocpVersion: "4.15.0", - hosts: []*models.Host{masterNode, masterNode, masterNode, workerNode, workerNode}, - resultMessage: []string{"Logical Volume Manager requires at least one non-installation HDD/SSD disk of 50GB minimum on the host"}, - apiStatus: api.Failure, - diskCount: 1, - hostCPU: 3, - HighAvailabilityMode: models.ClusterHighAvailabilityModeFull, - hostRole: models.HostRoleWorker, - hostMem: 8*conversions.GiB + float32(lvmMemMB), - additionalDiskRequirementGB: int64(50), + name: "full version 4.15, Worker, insufficient Disk", + ocpVersion: "4.15.0", + hosts: []*models.Host{masterNode, masterNode, masterNode, workerNode, workerNode}, + resultMessage: []string{"Logical Volume Manager requires at least one non-installation HDD/SSD disk on the host"}, + apiStatus: api.Failure, + diskCount: 1, + hostCPU: 3, + HighAvailabilityMode: models.ClusterHighAvailabilityModeFull, + hostRole: models.HostRoleWorker, + hostMem: 8*conversions.GiB + float32(lvmMemMB), }, } @@ -539,8 +515,7 @@ var _ = Describe("Lvm Operator", func() { SchedulableMastersForcedTrue: &schedulableMasters, }} - operator.SetAdditionalDiskRequirements(test.additionalDiskRequirementGB) - res, _ := operator.ValidateHost(ctx, cluster, testHost) + res, _ := operator.ValidateHost(ctx, cluster, testHost, nil) Expect(test.resultMessage).Should(Equal(res.Reasons)) Expect(test.apiStatus).Should(Equal(res.Status)) diff --git a/internal/operators/manager.go b/internal/operators/manager.go index eaba57795613..0c4e392761d6 100644 --- a/internal/operators/manager.go +++ b/internal/operators/manager.go @@ -29,6 +29,8 @@ import ( const controllerManifestFile = "custom_manifests.json" +var storageOperatorsPriority = []string{odf.Operator.Name, lvm.Operator.Name} + // Manifest store the operator manifest used by assisted-installer to create CRs of the OLM. type Manifest struct { // Name of the operator the CR manifest we want create @@ -124,8 +126,7 @@ func (mgr *Manager) GetRequirementsBreakdownForHostInCluster(ctx context.Context } func (mgr *Manager) getStorageOperator(cluster *models.Cluster) (api.StorageOperator, error) { - priorityOrder := []string{odf.Operator.Name, lvm.Operator.Name} - for _, operatorName := range priorityOrder { + for _, operatorName := range storageOperatorsPriority { for _, operator := range cluster.MonitoredOperators { if operator.Name == operatorName { o := mgr.olmOperators[operatorName] @@ -241,13 +242,10 @@ func (mgr *Manager) AnyOLMOperatorEnabled(cluster *common.Cluster) bool { // ValidateHost validates host requirements func (mgr *Manager) ValidateHost(ctx context.Context, cluster *common.Cluster, host *models.Host) ([]api.ValidationResult, error) { results := make([]api.ValidationResult, 0, len(mgr.olmOperators)) + additionalOperatorRequirements := &models.ClusterHostRequirementsDetails{} if hasMCEAndStorage(cluster.Cluster.MonitoredOperators) { - storageOperator, err := mgr.getStorageOperator(&cluster.Cluster) - if err != nil { - mgr.log.Infof("Could not load storage operator while validating host (%v)", err.Error()) - } - storageOperator.SetAdditionalDiskRequirements(mce.GetMinDiskSizeGB(&cluster.Cluster)) + additionalOperatorRequirements.DiskSizeGb = mce.GetMinDiskSizeGB(&cluster.Cluster) } // To track operators that are disabled or not present in the cluster configuration, but have to be present @@ -264,7 +262,7 @@ func (mgr *Manager) ValidateHost(ctx context.Context, cluster *common.Cluster, h operator := mgr.olmOperators[clusterOperator.Name] if operator != nil { - result, err := operator.ValidateHost(ctx, cluster, host) + result, err := operator.ValidateHost(ctx, cluster, host, additionalOperatorRequirements) if err != nil { return nil, err } diff --git a/internal/operators/manager_test.go b/internal/operators/manager_test.go index 846c8665f773..839b6364775d 100644 --- a/internal/operators/manager_test.go +++ b/internal/operators/manager_test.go @@ -106,14 +106,7 @@ var _ = BeforeEach(func() { }, } cluster.ImageInfo = &models.ImageInfo{} - b, err := common.MarshalInventory(&models.Inventory{ - CPU: &models.CPU{Count: 8}, - Memory: &models.Memory{UsableBytes: 64 * conversions.GiB}, - Disks: []*models.Disk{ - {SizeBytes: 20 * conversions.GB, DriveType: models.DriveTypeHDD, ID: common.TestDiskId}, - {SizeBytes: 40 * conversions.GB, DriveType: models.DriveTypeSSD}}}) - Expect(err).To(Not(HaveOccurred())) - clusterHost = &models.Host{Inventory: b, Role: models.HostRoleMaster, InstallationDiskID: common.TestDiskId} + clusterHost = getMockHostWithDisks(int64(20), int64(40)) ctrl = gomock.NewController(GinkgoT()) manifestsAPI = manifestsapi.NewMockManifestsAPI(ctrl) @@ -489,6 +482,83 @@ var _ = Describe("Operators manager", func() { api.ValidationResult{Status: api.Success, ValidationId: string(models.ClusterValidationIDMceRequirementsSatisfied), Reasons: []string{"mce is disabled"}}, )) }) + + It("should be not valid if not enough disk space available for mce and odf", func() { + clusterHost = getMockHostWithDisks(int64(20), int64(20)) + + cluster.MonitoredOperators = []*models.MonitoredOperator{ + &odf.Operator, + &mce.Operator, + } + + results, err := manager.ValidateHost(context.TODO(), cluster, clusterHost) + + Expect(err).NotTo(HaveOccurred()) + Expect(results).To(ContainElements( + api.ValidationResult{Status: api.Success, ValidationId: string(models.HostValidationIDLsoRequirementsSatisfied), Reasons: []string{"lso is disabled"}}, + api.ValidationResult{Status: api.Failure, ValidationId: string(models.HostValidationIDOdfRequirementsSatisfied), Reasons: []string{"Insufficient resources to deploy ODF in compact mode. ODF requires a minimum of 3 hosts. Each host must have at least 1 additional disk of 75 GB minimum and an installation disk."}}, + api.ValidationResult{Status: api.Success, ValidationId: string(models.ClusterValidationIDMceRequirementsSatisfied), Reasons: nil}, + api.ValidationResult{Status: api.Success, ValidationId: string(models.HostValidationIDCnvRequirementsSatisfied), Reasons: []string{"cnv is disabled"}}, + api.ValidationResult{Status: api.Success, ValidationId: string(models.HostValidationIDLvmRequirementsSatisfied), Reasons: []string{"lvm is disabled"}}, + )) + }) + + It("should be valid if enough disk space available for mce and odf", func() { + clusterHost = getMockHostWithDisks(int64(20), int64(80)) + + cluster.MonitoredOperators = []*models.MonitoredOperator{ + &odf.Operator, + &mce.Operator, + } + + results, err := manager.ValidateHost(context.TODO(), cluster, clusterHost) + + Expect(err).NotTo(HaveOccurred()) + Expect(results).To(ContainElements( + api.ValidationResult{Status: api.Success, ValidationId: string(models.HostValidationIDLsoRequirementsSatisfied), Reasons: []string{"lso is disabled"}}, + api.ValidationResult{Status: api.Success, ValidationId: string(models.HostValidationIDOdfRequirementsSatisfied), Reasons: []string{}}, + api.ValidationResult{Status: api.Success, ValidationId: string(models.ClusterValidationIDMceRequirementsSatisfied), Reasons: nil}, + api.ValidationResult{Status: api.Success, ValidationId: string(models.HostValidationIDCnvRequirementsSatisfied), Reasons: []string{"cnv is disabled"}}, + api.ValidationResult{Status: api.Success, ValidationId: string(models.HostValidationIDLvmRequirementsSatisfied), Reasons: []string{"lvm is disabled"}}, + )) + }) + + It("should be valid if enough disk space available for mce and odf and then only odf", func() { + clusterHost = getMockHostWithDisks(int64(20), int64(80)) + + cluster.MonitoredOperators = []*models.MonitoredOperator{ + &odf.Operator, + &mce.Operator, + } + + results, err := manager.ValidateHost(context.TODO(), cluster, clusterHost) + + Expect(err).NotTo(HaveOccurred()) + Expect(results).To(ContainElements( + api.ValidationResult{Status: api.Success, ValidationId: string(models.HostValidationIDLsoRequirementsSatisfied), Reasons: []string{"lso is disabled"}}, + api.ValidationResult{Status: api.Success, ValidationId: string(models.HostValidationIDOdfRequirementsSatisfied), Reasons: []string{}}, + api.ValidationResult{Status: api.Success, ValidationId: string(models.ClusterValidationIDMceRequirementsSatisfied), Reasons: nil}, + api.ValidationResult{Status: api.Success, ValidationId: string(models.HostValidationIDCnvRequirementsSatisfied), Reasons: []string{"cnv is disabled"}}, + api.ValidationResult{Status: api.Success, ValidationId: string(models.HostValidationIDLvmRequirementsSatisfied), Reasons: []string{"lvm is disabled"}}, + )) + + clusterHost = getMockHostWithDisks(int64(20), int64(30)) + + cluster.MonitoredOperators = []*models.MonitoredOperator{ + &odf.Operator, + } + + results, err = manager.ValidateHost(context.TODO(), cluster, clusterHost) + + Expect(err).NotTo(HaveOccurred()) + Expect(results).To(ContainElements( + api.ValidationResult{Status: api.Success, ValidationId: string(models.HostValidationIDLsoRequirementsSatisfied), Reasons: []string{"lso is disabled"}}, + api.ValidationResult{Status: api.Success, ValidationId: string(models.HostValidationIDOdfRequirementsSatisfied), Reasons: []string{}}, + api.ValidationResult{Status: api.Success, ValidationId: string(models.ClusterValidationIDMceRequirementsSatisfied), Reasons: []string{"mce is disabled"}}, + api.ValidationResult{Status: api.Success, ValidationId: string(models.HostValidationIDCnvRequirementsSatisfied), Reasons: []string{"cnv is disabled"}}, + api.ValidationResult{Status: api.Success, ValidationId: string(models.HostValidationIDLvmRequirementsSatisfied), Reasons: []string{"lvm is disabled"}}, + )) + }) }) Context("ResolveDependencies", func() { @@ -641,3 +711,14 @@ func mockOperatorBase(operatorName string) *api.MockOperator { return operator1 } + +func getMockHostWithDisks(sizeDiskA, sizeDiskB int64) *models.Host { + b, err := common.MarshalInventory(&models.Inventory{ + CPU: &models.CPU{Count: 8}, + Memory: &models.Memory{UsableBytes: 64 * conversions.GiB}, + Disks: []*models.Disk{ + {SizeBytes: sizeDiskA * conversions.GB, DriveType: models.DriveTypeHDD, ID: common.TestDiskId}, + {SizeBytes: sizeDiskB * conversions.GB, DriveType: models.DriveTypeSSD}}}) + Expect(err).To(Not(HaveOccurred())) + return &models.Host{Inventory: b, Role: models.HostRoleMaster, InstallationDiskID: common.TestDiskId} +} diff --git a/internal/operators/mce/mce_operator.go b/internal/operators/mce/mce_operator.go index df3198a7afab..1eb826e9d54c 100644 --- a/internal/operators/mce/mce_operator.go +++ b/internal/operators/mce/mce_operator.go @@ -97,7 +97,7 @@ func (o *operator) ValidateCluster(_ context.Context, cluster *common.Cluster) ( } // ValidateHost returns validationResult based on node type requirements such as memory and cpu -func (o *operator) ValidateHost(ctx context.Context, cluster *common.Cluster, host *models.Host) (result api.ValidationResult, err error) { +func (o *operator) ValidateHost(ctx context.Context, cluster *common.Cluster, host *models.Host, _ *models.ClusterHostRequirementsDetails) (result api.ValidationResult, err error) { if host.Inventory == "" { message := "Missing Inventory in the host" return api.ValidationResult{Status: api.Pending, ValidationId: o.GetHostValidationID(), Reasons: []string{message}}, nil diff --git a/internal/operators/mce/mce_operator_test.go b/internal/operators/mce/mce_operator_test.go index 243b80a5efa6..3419762379fa 100644 --- a/internal/operators/mce/mce_operator_test.go +++ b/internal/operators/mce/mce_operator_test.go @@ -56,7 +56,7 @@ var _ = Describe("MCE Operator", func() { Context("ValidateHost", func() { table.DescribeTable("validate host when ", func(cluster *common.Cluster, host *models.Host, expectedResult api.ValidationResult) { - res, _ := operator.ValidateHost(ctx, cluster, host) + res, _ := operator.ValidateHost(ctx, cluster, host, nil) Expect(res).Should(Equal(expectedResult)) }, table.Entry("host with no inventory", diff --git a/internal/operators/odf/odf_operator.go b/internal/operators/odf/odf_operator.go index a00a21607f63..f554279350f4 100644 --- a/internal/operators/odf/odf_operator.go +++ b/internal/operators/odf/odf_operator.go @@ -35,10 +35,9 @@ func NewOcsOperator(log logrus.FieldLogger) *operator { // operator is an ODF OLM operator plugin; it implements api.Operator type operator struct { - log logrus.FieldLogger - config *Config - extracter oc.Extracter - additionalDiskRequirementGB int64 + log logrus.FieldLogger + config *Config + extracter oc.Extracter } var Operator = models.MonitoredOperator{ @@ -110,29 +109,29 @@ func (o *operator) StorageClassName() string { return defaultStorageClassName } -func (o *operator) SetAdditionalDiskRequirements(additionalSizeGB int64) { - o.additionalDiskRequirementGB = additionalSizeGB -} - -func (o *operator) getMinDiskSizeGB() int64 { - return o.config.ODFMinDiskSizeGB + o.additionalDiskRequirementGB +func (o *operator) getMinDiskSizeGB(additionalDiskRequirementsGb int64) int64 { + return o.config.ODFMinDiskSizeGB + additionalDiskRequirementsGb } // Get valid disk count, and return error if no disk available -func (o *operator) getValidDiskCount(disks []*models.Disk, installationDiskID string, cluster *models.Cluster) (int64, error) { +func (o *operator) getValidDiskCount(disks []*models.Disk, installationDiskID string, cluster *models.Cluster, additionalOperatorRequirements *models.ClusterHostRequirementsDetails) (int64, error) { numOfHosts := len(cluster.Hosts) mode := getODFDeploymentMode(numOfHosts) - eligibleDisks, availableDisks := operatorscommon.NonInstallationDiskCount(disks, installationDiskID, o.getMinDiskSizeGB()) + minDiskSizeGb := int64(0) + if additionalOperatorRequirements != nil { + minDiskSizeGb = additionalOperatorRequirements.DiskSizeGb + } + eligibleDisks, availableDisks := operatorscommon.NonInstallationDiskCount(disks, installationDiskID, o.getMinDiskSizeGB(minDiskSizeGb)) var err error if eligibleDisks == 0 && availableDisks > 0 { - err = fmt.Errorf("Insufficient resources to deploy ODF in %s mode. ODF requires a minimum of 3 hosts. Each host must have at least 1 additional disk of %d GB minimum and an installation disk.", strings.ToLower(string(mode)), o.getMinDiskSizeGB()) + err = fmt.Errorf("Insufficient resources to deploy ODF in %s mode. ODF requires a minimum of 3 hosts. Each host must have at least 1 additional disk of %d GB minimum and an installation disk.", strings.ToLower(string(mode)), o.getMinDiskSizeGB(minDiskSizeGb)) } return eligibleDisks, err } // ValidateHost verifies whether this operator is valid for given host -func (o *operator) ValidateHost(_ context.Context, cluster *common.Cluster, host *models.Host) (api.ValidationResult, error) { +func (o *operator) ValidateHost(_ context.Context, cluster *common.Cluster, host *models.Host, additionalOperatorRequirements *models.ClusterHostRequirementsDetails) (api.ValidationResult, error) { numOfHosts := len(cluster.Hosts) if host.Inventory == "" { return api.ValidationResult{Status: api.Pending, ValidationId: o.GetHostValidationID(), Reasons: []string{"Missing Inventory in the host."}}, nil @@ -146,7 +145,7 @@ func (o *operator) ValidateHost(_ context.Context, cluster *common.Cluster, host mode := getODFDeploymentMode(numOfHosts) // getValidDiskCount counts the total number of valid disks in each host and return a error if we don't have the disk of required size - diskCount, err := o.getValidDiskCount(inventory.Disks, host.InstallationDiskID, &cluster.Cluster) + diskCount, err := o.getValidDiskCount(inventory.Disks, host.InstallationDiskID, &cluster.Cluster, additionalOperatorRequirements) if err != nil { return api.ValidationResult{Status: api.Failure, ValidationId: o.GetHostValidationID(), Reasons: []string{err.Error()}}, nil } @@ -208,7 +207,7 @@ func (o *operator) GetHostRequirements(_ context.Context, cluster *common.Cluste /* getValidDiskCount counts the total number of valid disks in each host and return a error if we don't have the disk of required size, we ignore the error as its treated as 500 in the UI */ - diskCount, _ = o.getValidDiskCount(inventory.Disks, host.InstallationDiskID, &cluster.Cluster) + diskCount, _ = o.getValidDiskCount(inventory.Disks, host.InstallationDiskID, &cluster.Cluster, nil) } role := common.GetEffectiveRole(host) diff --git a/internal/operators/odf/odf_operator_test.go b/internal/operators/odf/odf_operator_test.go index 23076f2c2b7f..306f637236c1 100644 --- a/internal/operators/odf/odf_operator_test.go +++ b/internal/operators/odf/odf_operator_test.go @@ -221,7 +221,7 @@ var _ = Describe("Odf Operator", func() { Context("ValidateHost", func() { table.DescribeTable("compact mode scenario: validateHost when ", func(cluster *common.Cluster, host *models.Host, expectedResult api.ValidationResult) { - res, _ := operator.ValidateHost(ctx, cluster, host) + res, _ := operator.ValidateHost(ctx, cluster, host, nil) Expect(res).Should(Equal(expectedResult)) }, table.Entry("Single master", @@ -297,7 +297,7 @@ var _ = Describe("Odf Operator", func() { ) table.DescribeTable("standard mode scenario: validateHosts when ", func(cluster *common.Cluster, host *models.Host, expectedResult api.ValidationResult) { - res, _ := operator.ValidateHost(ctx, cluster, host) + res, _ := operator.ValidateHost(ctx, cluster, host, nil) Expect(res).Should(Equal(expectedResult)) }, table.Entry("there are 4 hosts, role of one as auto-assign", diff --git a/internal/operators/odf/validations.go b/internal/operators/odf/validations.go index 4c6e3d2f0f63..634a37bc9efe 100644 --- a/internal/operators/odf/validations.go +++ b/internal/operators/odf/validations.go @@ -116,7 +116,7 @@ func (o *operator) computeNodeResourceUtil(cluster *models.Cluster, host *models return status, err } - diskCount, err := o.getValidDiskCount(inventory.Disks, host.InstallationDiskID, cluster) + diskCount, err := o.getValidDiskCount(inventory.Disks, host.InstallationDiskID, cluster, nil) if err != nil { return err.Error(), err }