Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MGMT-15264: Block installation if no custom manifests in OCI cluster #5364

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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions internal/cluster/refresh_status_preprocessor.go
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,10 @@ func newValidations(v *clusterValidator) []validation {
id: NetworksSameAddressFamilies,
condition: v.isNetworksSameAddressFamilies,
},
{
id: PlatformRequirementsSatisfied,
condition: v.platformRequirementsSatisfied,
},
}
return ret
}
Expand Down
3 changes: 2 additions & 1 deletion internal/cluster/validation_id.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ const (
IsCnvRequirementsSatisfied = ValidationID(models.ClusterValidationIDCnvRequirementsSatisfied)
IsLvmRequirementsSatisfied = ValidationID(models.ClusterValidationIDLvmRequirementsSatisfied)
IsMceRequirementsSatisfied = ValidationID(models.ClusterValidationIDMceRequirementsSatisfied)
PlatformRequirementsSatisfied = ValidationID(models.ClusterValidationIDPlatformRequirementsSatisfied)
)

func (v ValidationID) Category() (string, error) {
Expand All @@ -43,7 +44,7 @@ func (v ValidationID) Category() (string, error) {
return "network", nil
case AllHostsAreReadyToInstall, SufficientMastersCount:
return "hosts-data", nil
case IsPullSecretSet:
case IsPullSecretSet, PlatformRequirementsSatisfied:
return "configuration", nil
case IsOdfRequirementsSatisfied, IsLsoRequirementsSatisfied, IsCnvRequirementsSatisfied, IsLvmRequirementsSatisfied, IsMceRequirementsSatisfied:
return "operators", nil
Expand Down
22 changes: 22 additions & 0 deletions internal/cluster/validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"github.com/openshift/assisted-service/internal/common"
"github.com/openshift/assisted-service/internal/host"
"github.com/openshift/assisted-service/internal/network"
"github.com/openshift/assisted-service/internal/usage"
"github.com/openshift/assisted-service/models"
"github.com/pkg/errors"
"github.com/sirupsen/logrus"
Expand Down Expand Up @@ -400,6 +401,27 @@ func (v *clusterValidator) allHostsAreReadyToInstall(c *clusterPreprocessContext
return ValidationFailure, "The cluster has hosts that are not ready to install."
}

func (v *clusterValidator) platformRequirementsSatisfied(c *clusterPreprocessContext) (ValidationStatus, string) {
// If cluster platform type is not OCI ignore that validation
if c.cluster.Platform != nil && common.PlatformTypeValue(c.cluster.Platform.Type) != models.PlatformTypeOci {
return ValidationSuccess, "Platform requirements satisfied"
}

usages, err := usage.Unmarshal(c.cluster.Cluster.FeatureUsage)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why we are using the feature usage for checking it and not data from the cluster? what will happen if there will be a bug in the feature usage?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There isn't a bug thanks to @avishayt #5363 :)
But the alternative is adding a boolean read only parameter to the cluster

if err != nil {
v.log.Errorf("platform validation failure, failed to parse feature usages, %s", err.Error())
return ValidationFailure, "Failed to parse feature usages"
}

for _, usg := range usages {
if usg.Name == usage.CustomManifest {
eliorerz marked this conversation as resolved.
Show resolved Hide resolved
return ValidationSuccess, "Platform requirements satisfied"
}
}

return ValidationFailure, "The custom manifest required for Oracle Cloud Infrastructure platform integration has not been added. Add a custom manifest to continue."
}

func (v *clusterValidator) isDNSDomainDefined(c *clusterPreprocessContext) (ValidationStatus, string) {
if c.cluster.BaseDNSDomain != "" {
return ValidationSuccess, "The base domain is defined."
Expand Down
58 changes: 58 additions & 0 deletions internal/cluster/validator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -554,3 +554,61 @@ var _ = Describe("Validator tests", func() {
})

})

var _ = Describe("Platform validations", func() {
var (
validator clusterValidator
preprocessContext *clusterPreprocessContext
clusterID strfmt.UUID
)

BeforeEach(func() {
validator = clusterValidator{logrus.New(), nil}
clusterID = strfmt.UUID(uuid.New().String())
})

It("Should fail validation OCI cluster with no custom manifests", func() {
preprocessContext = &clusterPreprocessContext{hasHostsWithInventories: true}
preprocessContext.cluster = &common.Cluster{Cluster: models.Cluster{
ID: &clusterID,
Platform: &models.Platform{
IsExternal: swag.Bool(true),
Type: common.PlatformTypePtr(models.PlatformTypeOci),
},
UserManagedNetworking: swag.Bool(true),
}}
status, message := validator.platformRequirementsSatisfied(preprocessContext)
Expect(status).To(Equal(ValidationFailure))
Expect(message).To(Equal("The custom manifest required for Oracle Cloud Infrastructure platform integration has not been added. Add a custom manifest to continue."))
})

It("Should pass platform validation on OCI cluster with no custom manifests", func() {
preprocessContext = &clusterPreprocessContext{hasHostsWithInventories: true}
preprocessContext.cluster = &common.Cluster{Cluster: models.Cluster{
ID: &clusterID,
Platform: &models.Platform{
IsExternal: swag.Bool(true),
Type: common.PlatformTypePtr(models.PlatformTypeOci),
},
UserManagedNetworking: swag.Bool(true),
FeatureUsage: "{\"Custom manifest\":{\"id\":\"CUSTOM_MANIFEST\",\"name\":\"Custom manifest\"}}",
}}
status, message := validator.platformRequirementsSatisfied(preprocessContext)
Expect(status).To(Equal(ValidationSuccess))
Expect(message).To(Equal("Platform requirements satisfied"))
})

It("Should pass validation if platform is baremetal", func() {
preprocessContext = &clusterPreprocessContext{hasHostsWithInventories: true}
preprocessContext.cluster = &common.Cluster{Cluster: models.Cluster{
ID: &clusterID,
Platform: &models.Platform{
IsExternal: swag.Bool(false),
Type: common.PlatformTypePtr(models.PlatformTypeBaremetal),
},
}}
status, message := validator.platformRequirementsSatisfied(preprocessContext)
Expect(status).To(Equal(ValidationSuccess))
Expect(message).To(Equal("Platform requirements satisfied"))
})
})
5 changes: 4 additions & 1 deletion models/cluster_validation_id.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 4 additions & 2 deletions restapi/embedded_spec.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions swagger.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -6614,6 +6614,7 @@ definitions:
- 'lvm-requirements-satisfied'
- 'mce-requirements-satisfied'
- 'network-type-valid'
- 'platform-requirements-satisfied'

logs_type:
type: string
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.