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

Minimum validation for multitenant formations #5199

Merged
merged 14 commits into from
Mar 25, 2024
Merged
Changes from 4 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
125 changes: 103 additions & 22 deletions ibm/service/database/resource_ibm_database.go
Original file line number Diff line number Diff line change
Expand Up @@ -596,6 +596,16 @@
Computed: true,
Description: "Can memory scale down as well as up.",
},
"cpu_enforcement_ratio_ceiling_mb": {
Type: schema.TypeInt,
Optional: true,
Description: "The amount of memory required before the cpu/memory ratio is no longer enforced. (multitenant only).",
},
"cpu_enforcement_ratio_mb": {
Type: schema.TypeInt,
Optional: true,
Description: "The maximum memory allowed per CPU until the ratio ceiling is reached. (multitenant only).",
},
},
},
},
Expand Down Expand Up @@ -932,7 +942,7 @@
Identifier: "service",
ValidateFunctionIdentifier: validate.ValidateAllowedStringValue,
Type: validate.TypeString,
AllowedValues: "databases-for-etcd, databases-for-postgresql, databases-for-redis, databases-for-elasticsearch, databases-for-mongodb, messages-for-rabbitmq, databases-for-mysql, databases-for-cassandra, databases-for-enterprisedb",
AllowedValues: "databases-for-etcd, databases-for-postgresql, databases-for-redis, databases-for-redis-dev-yp-03, databases-for-elasticsearch, databases-for-mongodb, messages-for-rabbitmq, databases-for-mysql, databases-for-cassandra, databases-for-enterprisedb",
Required: true})
validateSchema = append(validateSchema,
validate.ValidateSchema{
Expand Down Expand Up @@ -987,21 +997,23 @@
}

type GroupResource struct {
Units string
Allocation int
Minimum int
Maximum int
StepSize int
IsAdjustable bool
IsOptional bool
CanScaleDown bool
Units string
Allocation int
Minimum int
Maximum int
StepSize int
IsAdjustable bool
IsOptional bool
CanScaleDown bool
CPUEnforcementRatioCeilingMb int
CPUEnforcementRatioMb int
}

type HostFlavorGroupResource struct {
ID string
}

func getDefaultScalingGroups(_service string, _plan string, meta interface{}) (groups []clouddatabasesv5.Group, err error) {
func getDefaultScalingGroups(_service string, _plan string, _hostFlavor string, meta interface{}) (groups []clouddatabasesv5.Group, err error) {
cloudDatabasesClient, err := meta.(conns.ClientSession).CloudDatabasesV5()
if err != nil {
return groups, fmt.Errorf("[ERROR] Error getting database client settings: %s", err)
Expand Down Expand Up @@ -1033,6 +1045,9 @@
}

getDefaultScalingGroupsOptions := cloudDatabasesClient.NewGetDefaultScalingGroupsOptions(service)
if _hostFlavor != "" {
getDefaultScalingGroupsOptions.SetHostFlavor(_hostFlavor)

Check failure on line 1049 in ibm/service/database/resource_ibm_database.go

View workflow job for this annotation

GitHub Actions / Build

getDefaultScalingGroupsOptions.SetHostFlavor undefined (type *clouddatabasesv5.GetDefaultScalingGroupsOptions has no field or method SetHostFlavor)
}

getDefaultScalingGroupsResponse, _, err := cloudDatabasesClient.GetDefaultScalingGroups(getDefaultScalingGroupsOptions)
if err != nil {
Expand All @@ -1043,7 +1058,7 @@
}

func getInitialNodeCount(service string, plan string, meta interface{}) (int, error) {
groups, err := getDefaultScalingGroups(service, plan, meta)
groups, err := getDefaultScalingGroups(service, plan, "", meta)

if err != nil {
return 0, err
Expand Down Expand Up @@ -1120,6 +1135,8 @@
unmarshalFn = clouddatabasesv5.UnmarshalConfigurationPgConfiguration
case "databases-for-enterprisedb":
unmarshalFn = clouddatabasesv5.UnmarshalConfigurationPgConfiguration
case "databases-for-redis-dev-yp-03":
unmarshalFn = clouddatabasesv5.UnmarshalConfigurationRedisConfiguration
case "databases-for-redis":
unmarshalFn = clouddatabasesv5.UnmarshalConfigurationRedisConfiguration
case "databases-for-mysql":
Expand Down Expand Up @@ -1227,6 +1244,7 @@
}

initialNodeCount, err := getInitialNodeCount(serviceName, plan, meta)
fmt.Printf("LALALA initialNodeCount %d", initialNodeCount)
if err != nil {
return diag.FromErr(err)
}
Expand Down Expand Up @@ -1772,7 +1790,7 @@
}
d.Set("connectionstrings", flex.FlattenConnectionStrings(connectionStrings))

if serviceOff == "databases-for-postgresql" || serviceOff == "databases-for-redis" || serviceOff == "databases-for-enterprisedb" {
if serviceOff == "databases-for-postgresql" || serviceOff == "databases-for-redis" || serviceOff == "databases-for-redis-dev-yp-03" || serviceOff == "databases-for-enterprisedb" {
configSchema, err := icdClient.Configurations().GetConfiguration(icdId)
if err != nil {
return diag.FromErr(fmt.Errorf("[ERROR] Error getting database (%s) configuration schema : %s", icdId, err))
Expand Down Expand Up @@ -2218,6 +2236,8 @@
dbConnection = connection.Postgres
case "databases-for-redis":
dbConnection = connection.Rediss
case "databases-for-redis-dev-yp-03":
dbConnection = connection.Rediss
case "databases-for-mongodb":
dbConnection = connection.Mongo
case "databases-for-mysql":
Expand Down Expand Up @@ -2702,14 +2722,16 @@
}

group.Memory = &GroupResource{
Units: *g.Memory.Units,
Allocation: int(*g.Memory.AllocationMb),
Minimum: int(*g.Memory.MinimumMb),
Maximum: int(*g.Memory.MaximumMb),
StepSize: int(*g.Memory.StepSizeMb),
IsAdjustable: *g.Memory.IsAdjustable,
IsOptional: *g.Memory.IsOptional,
CanScaleDown: *g.Memory.CanScaleDown,
Units: *g.Memory.Units,
Allocation: int(*g.Memory.AllocationMb),
Minimum: int(*g.Memory.MinimumMb),
Maximum: int(*g.Memory.MaximumMb),
StepSize: int(*g.Memory.StepSizeMb),
IsAdjustable: *g.Memory.IsAdjustable,
IsOptional: *g.Memory.IsOptional,
CanScaleDown: *g.Memory.CanScaleDown,
CPUEnforcementRatioCeilingMb: getCPUEnforcementRatioCeilingMb(g.Memory),
CPUEnforcementRatioMb: getCPUEnforcementRatioMb(g.Memory),
}

group.Disk = &GroupResource{
Expand Down Expand Up @@ -2745,6 +2767,22 @@
return groups
}

func getCPUEnforcementRatioCeilingMb(groupMemory *clouddatabasesv5.GroupMemory) int {
if groupMemory.CPUEnforcementRatioCeilingMb != nil {

Check failure on line 2771 in ibm/service/database/resource_ibm_database.go

View workflow job for this annotation

GitHub Actions / Build

groupMemory.CPUEnforcementRatioCeilingMb undefined (type *clouddatabasesv5.GroupMemory has no field or method CPUEnforcementRatioCeilingMb)
return int(*groupMemory.CPUEnforcementRatioCeilingMb)

Check failure on line 2772 in ibm/service/database/resource_ibm_database.go

View workflow job for this annotation

GitHub Actions / Build

groupMemory.CPUEnforcementRatioCeilingMb undefined (type *clouddatabasesv5.GroupMemory has no field or method CPUEnforcementRatioCeilingMb)
}

return 0
}

func getCPUEnforcementRatioMb(groupMemory *clouddatabasesv5.GroupMemory) int {
if groupMemory.CPUEnforcementRatioMb != nil {

Check failure on line 2779 in ibm/service/database/resource_ibm_database.go

View workflow job for this annotation

GitHub Actions / Build

groupMemory.CPUEnforcementRatioMb undefined (type *clouddatabasesv5.GroupMemory has no field or method CPUEnforcementRatioMb)
return int(*groupMemory.CPUEnforcementRatioMb)

Check failure on line 2780 in ibm/service/database/resource_ibm_database.go

View workflow job for this annotation

GitHub Actions / Build

groupMemory.CPUEnforcementRatioMb undefined (type *clouddatabasesv5.GroupMemory has no field or method CPUEnforcementRatioMb)
}

return 0
}

func expandGroups(_groups []interface{}) []*Group {
if len(_groups) == 0 {
return nil
Expand Down Expand Up @@ -2837,6 +2875,30 @@
return nil
}

func validateMultitenantMemoryCpu(groupId string, resourceName string, resourceDefaults *Group, group *Group) error {

Choose a reason for hiding this comment

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

Not sure what Go conventions are (just learning), but you might be able to put guard clauses in here to simplify the rest of your code. Like in the first line of this method:

if group.CPU != nil {
  return nil
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

Do you need resourceName parameter here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Nope! Thank you for catching that πŸ˜„

// TODO: Replace this with resourceDefaults.Memory.CPUEnforcementRatioCeiling when it is fixed
cpuEnforcementRatioCeiling := 16384
Copy link
Collaborator

Choose a reason for hiding this comment

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

When will this fix go in? Will that be before merging this PR?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@obai-1 This fix won't go in until the next release cut and deployment, so we're looking at around May.

This PR will be merged sometime thing week. So that's why we gotta hard code the cpuEnforcementRatioCeiling for now.

// members := resourceDefaults.Members.Minimum

// if group.Members != nil && members < resourceDefaults.Members.Allocation {
// members = group.Members.Allocation
// }

if group.CPU != nil && group.CPU.Allocation > 2 {
return nil
}

if group.Memory.Allocation < resourceDefaults.Memory.Minimum {
return fmt.Errorf("We were unable to complete your request: group.memory requires a minimum of %d megabytes. Try again with valid values or contact support if the issue persists.", resourceDefaults.Memory.Minimum)
}

if group.CPU != nil && group.Memory.Allocation < cpuEnforcementRatioCeiling*resourceDefaults.Members.Minimum && group.Memory.Allocation > group.CPU.Allocation*resourceDefaults.Memory.CPUEnforcementRatioMb {

Choose a reason for hiding this comment

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

cpuEnforcementRatioCeiling*resourceDefaults.Members.Minimum will work for provisioning, but a customer might have horizontally scaled, so we'd need you use cpuEnforcementRatioCeiling*resourceDefaults.Members.allocationCount, right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@bpeicher yup you're correct! Thank you for catching that!

return fmt.Errorf("We were unable to complete your request: group.memory %d with group.cpu %d is not valid. Try again with valid values or contact support if the issue persists.", group.Memory.Allocation, group.CPU.Allocation)
}

return nil
}

func validateGroupsDiff(_ context.Context, diff *schema.ResourceDiff, meta interface{}) (err error) {
instanceID := diff.Id()
service := diff.Get("service").(string)
Expand All @@ -2846,11 +2908,23 @@
var currentGroups []Group
var groupList []clouddatabasesv5.Group
var groupIds []string
groups := expandGroups(group.(*schema.Set).List())
var memberGroup *Group
for _, g := range groups {
if g.ID == "member" {
memberGroup = g
break
}
}

if instanceID != "" {
groupList, err = getGroups(instanceID, meta)
} else {
groupList, err = getDefaultScalingGroups(service, plan, meta)
if memberGroup.HostFlavor != nil {
groupList, err = getDefaultScalingGroups(service, plan, memberGroup.HostFlavor.ID, meta)
} else {
groupList, err = getDefaultScalingGroups(service, plan, "", meta)
}
}

if err != nil {
Expand Down Expand Up @@ -2906,6 +2980,13 @@
}
}

if group.Memory != nil && group.HostFlavor != nil && group.HostFlavor.ID != "" && group.HostFlavor.ID == "multitenant" && groupDefaults.Memory.CPUEnforcementRatioCeilingMb != 0 && groupDefaults.Memory.CPUEnforcementRatioMb != 0 {
err = validateMultitenantMemoryCpu(groupId, "memory", groupDefaults, group)
if err != nil {
return err
}
}

if group.Memory != nil {
err = validateGroupScaling(groupId, "memory", group.Memory.Allocation, groupDefaults.Memory, nodeCount)
if err != nil {
Expand Down Expand Up @@ -2972,7 +3053,7 @@

// TODO: Use Capability API
// RBAC roles supported for Redis 6.0 and above
if service == "databases-for-redis" && !(version > 0 && version < 6) {
if (service == "databases-for-redis" || service == "databases-for-redis-dev-yp-03") && !(version > 0 && version < 6) {
err = change.New.ValidateRBACRole()
} else if service == "databases-for-mongodb" && change.New.Type == "ops_manager" {
err = change.New.ValidateOpsManagerRole()
Expand Down