Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/enhancements/arbiter-clusters.md
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ to s3. The list of patches arbiter.ign needs:
Our validations will need to be updated as follows:
- Clusters can set control_plane_count to be 2. Clusters that do that must
add at least one more host that will be assigned the arbiter role.
- TNA clusters' platform must be baremetal.
- TNA clusters' platform must be baremetal or none.
- TNA clusters' ocp version must be at least 4.19.

In the cluster's transition handler, the function enoughMastersAndWorkers
Expand Down
8 changes: 4 additions & 4 deletions internal/bminventory/inventory_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10802,13 +10802,13 @@ var _ = Describe("infraEnvs host", func() {
Expect(resp.(*common.ApiErrorResponse).Error()).To(Equal(fmt.Sprintf("Cannot set role arbiter to host %s in infra-env %s: cluster's openshift version must be at least %s", hostID, infraEnvID, common.MinimumVersionForArbiterClusters)))
})

It("update host role arbiter failure - cluster's platform is not baremetal", func() {
It("update host role arbiter failure - cluster's platform is not allowed", func() {
mockHostApi.EXPECT().UpdateHostname(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Times(0)
mockHostApi.EXPECT().UpdateInstallationDisk(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Times(0)
mockHostApi.EXPECT().UpdateMachineConfigPoolName(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Times(0)
mockHostApi.EXPECT().UpdateIgnitionEndpointToken(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Times(0)
cluster.OpenshiftVersion = common.MinimumVersionForArbiterClusters
cluster.Platform = &models.Platform{Type: models.NewPlatformType(models.PlatformTypeNone)}
cluster.Platform = &models.Platform{Type: models.NewPlatformType(models.PlatformTypeVsphere)}
db.Save(&cluster)
resp := bm.V2UpdateHost(ctx, installer.V2UpdateHostParams{
InfraEnvID: infraEnvID,
Expand All @@ -10819,7 +10819,7 @@ var _ = Describe("infraEnvs host", func() {
})
Expect(resp).To(BeAssignableToTypeOf(&common.ApiErrorResponse{}))
Expect(resp.(*common.ApiErrorResponse).StatusCode()).To(Equal(int32(http.StatusBadRequest)))
Expect(resp.(*common.ApiErrorResponse).Error()).To(Equal(fmt.Sprintf("Cannot set role arbiter to host %s in infra-env %s: cluster's platform must be baremetal", hostID, infraEnvID)))
Expect(resp.(*common.ApiErrorResponse).Error()).To(Equal(fmt.Sprintf("Cannot set role arbiter to host %s in infra-env %s: cluster's platform must be baremetal or none", hostID, infraEnvID)))
})

Context("Hostname", func() {
Expand Down Expand Up @@ -18635,7 +18635,7 @@ var _ = Describe("BindHost", func() {
var clusterObj models.Cluster
Expect(db.First(&clusterObj, "id = ?", clusterID).Error).ShouldNot(HaveOccurred())
Expect(db.Model(&clusterObj).Update("openshift_version", common.MinimumVersionForArbiterClusters).Error).ShouldNot(HaveOccurred())
Expect(db.Model(&clusterObj).Update("platform_type", models.PlatformTypeNone).Error).ShouldNot(HaveOccurred())
Expect(db.Model(&clusterObj).Update("platform_type", models.PlatformTypeVsphere).Error).ShouldNot(HaveOccurred())

var hostObj models.Host
Expect(db.First(&hostObj, "id = ?", hostID).Error).ShouldNot(HaveOccurred())
Expand Down
6 changes: 3 additions & 3 deletions internal/common/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -851,7 +851,7 @@ func GetMultipleYamls[T any](contents []byte) ([]T, error) {
// ValidateClusterSupportsArbiterHosts checks if the given cluster is allowed to have arbiter hosts.
// A day1 cluster can have arbiter hosts if:
// 1. Its OCP version is at least MinimumVersionForArbiterClusters
// 2. Its platform is baremetal
// 2. Its platform is baremetal or none
// Day2 clusters can also have arbiter hosts added to them if they were installed as TNA clusters, but we can't check that.
func ValidateClusterSupportsArbiterHosts(cluster *Cluster) error {
if cluster == nil || IsDay2Cluster(cluster) {
Expand All @@ -867,8 +867,8 @@ func ValidateClusterSupportsArbiterHosts(cluster *Cluster) error {
}

platform := cluster.Platform
if platform == nil || platform.Type == nil || *platform.Type != models.PlatformTypeBaremetal {
return errors.New("cluster's platform must be baremetal")
if platform == nil || platform.Type == nil || (*platform.Type != models.PlatformTypeBaremetal && *platform.Type != models.PlatformTypeNone) {
return errors.New("cluster's platform must be baremetal or none")
}

return nil
Expand Down
13 changes: 11 additions & 2 deletions internal/featuresupport/feature_support_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,7 @@ var _ = Describe("V2ListFeatureSupportLevels API", func() {
Entry(
"none platform",
[]SupportLevelFeature{&NonePlatformFeature{}},
false,
true,
),

Entry(
Expand Down Expand Up @@ -396,11 +396,20 @@ var _ = Describe("V2ListFeatureSupportLevels API", func() {
),

Entry(
"unavailable - platform is not baremetal",
"tech preview openshift version with none platform",
SupportLevelFilters{
OpenshiftVersion: common.MinimumVersionForArbiterClusters,
PlatformType: models.PlatformTypeNone.Pointer(),
},
models.SupportLevelTechPreview,
),

Entry(
"unavailable - platform is not allowed",
SupportLevelFilters{
OpenshiftVersion: common.MinimumVersionForArbiterClusters,
PlatformType: models.PlatformTypeVsphere.Pointer(),
},
models.SupportLevelUnavailable,
),

Expand Down
5 changes: 2 additions & 3 deletions internal/featuresupport/features_misc.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,8 +98,8 @@ func (feature *TnaFeature) GetName() string {
}

func (feature *TnaFeature) getSupportLevel(filters SupportLevelFilters) (models.SupportLevel, models.IncompatibilityReason) {
//TNA is only available with baremetal platform
if filters.PlatformType != nil && *filters.PlatformType != models.PlatformTypeBaremetal {
//TNA is only available with baremetal/none platform
if filters.PlatformType != nil && *filters.PlatformType != models.PlatformTypeBaremetal && *filters.PlatformType != models.PlatformTypeNone {
return models.SupportLevelUnavailable, models.IncompatibilityReasonPlatform
}

Expand All @@ -118,7 +118,6 @@ func (feature *TnaFeature) getSupportLevel(filters SupportLevelFilters) (models.

func (feature *TnaFeature) getIncompatibleFeatures(string) []models.FeatureSupportLevelID {
return []models.FeatureSupportLevelID{
models.FeatureSupportLevelIDNONEPLATFORM,
models.FeatureSupportLevelIDNUTANIXINTEGRATION,
models.FeatureSupportLevelIDVSPHEREINTEGRATION,
models.FeatureSupportLevelIDEXTERNALPLATFORM,
Expand Down
1 change: 0 additions & 1 deletion internal/featuresupport/features_platforms.go
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,6 @@ func (feature *NonePlatformFeature) getFeatureActiveLevel(cluster *common.Cluste

func (feature *NonePlatformFeature) getIncompatibleFeatures(string) []models.FeatureSupportLevelID {
return []models.FeatureSupportLevelID{
models.FeatureSupportLevelIDTNA,
models.FeatureSupportLevelIDVIPAUTOALLOC,
models.FeatureSupportLevelIDCLUSTERMANAGEDNETWORKING,
models.FeatureSupportLevelIDUSERMANAGEDLOADBALANCER,
Expand Down
2 changes: 1 addition & 1 deletion internal/provider/platform.go
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,7 @@ func GetClusterPlatformByControlPlaneCount(platform *models.Platform, userManage
return nil, nil, common.NewApiError(http.StatusBadRequest, errors.Errorf("Invalid value for controlPlaneCount: nil"))
}

// Currently arbiter clusters can only be used in baremetal platforms, but we will still allow this function to set the platform to other values because of 2 reasons:
// Currently arbiter clusters can only be used in baremetal or none platforms, but we will still allow this function to set the platform to other values because of 2 reasons:
// 1. The validation comes afterward anyway, since arbiter cluster are allowed to have controlPlaneCount valid for normal HA cluster.
// 2. It will make it easier to add support for other platforms in the future.
// In practice the second condition is unnecessary since if the first condition is false then the second is also false, but it's clearer to keep it explicitly.
Expand Down