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

feat: add woop apply threshold strategy to tf scaling policy #438

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

UOndro
Copy link
Contributor

@UOndro UOndro commented Jan 14, 2025

No description provided.

@UOndro UOndro requested a review from a team as a code owner January 14, 2025 16:49
@UOndro UOndro requested review from jansyk13 and mszostok and removed request for a team and jansyk13 January 14, 2025 16:49
overhead = 0.15
apply_threshold_strategy {
type = "PERCENTAGE"
percentrage = 0.1
Copy link
Contributor

@mszostok mszostok Jan 14, 2025

Choose a reason for hiding this comment

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

both here and in line 23 👍

Suggested change
percentrage = 0.1
percentage = 0.1

percent-rage

1

when you're working with TF and your emotions sneak out as you write 😄

Comment on lines +499 to 506
func validatePercentageThresholdStrategy(r *sdk.WorkloadoptimizationV1ApplyThresholdStrategyPercentageThreshold) error {
if r.Percentage < minApplyThresholdValue || r.Percentage > maxApplyThresholdValue {
return fmt.Errorf(
`field %q: value must be between %v and %v`,
FieldApplyThresholdStrategyPercentage, minApplyThresholdValue, maxApplyThresholdValue)
}
return nil
}
Copy link
Contributor

Choose a reason for hiding this comment

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

that's already validate via validation.ToDiagFunc(validation.FloatBetween(minApplyThresholdValue, maxApplyThresholdValue)) so I would remove that.

When I removed, that validation I still got a readable error:

╷
│ Error: expected percentage to be in the range (0.010000 - 2.500000), got 0.000100
│ 
│   with castai_workload_scaling_policy.woop-541-e2e-test,
│   on main.tf line 16, in resource "castai_workload_scaling_policy" "woop-541-e2e-test":
│   16:                         percentage = 0.0001
│ 
╵

@@ -251,6 +269,27 @@ func workloadScalingPolicyResourceLimitSchema() *schema.Resource {
}
}

func workloadScalingPolicyResourceApplyThresholdStrategySchema() *schema.Resource {
Copy link
Contributor

@mszostok mszostok Jan 14, 2025

Choose a reason for hiding this comment

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

ok, so the most problematic part is the proper resource mapping. Running terraform apply -var-file=tf.vars twice without changing anything result in invalid diff:

  1. When I use only the new strategy field -API returns the one that was populated by strategy but in my TF code the missing apply_threshold is set to default 0.1 resulting in:

      ~ resource "castai_workload_scaling_policy" "woop-541-e2e-test" {
            id                = "6ac1821a-36a8-4bbd-9d74-49b8a9808aae"
            name              = "woop-541-e2e-test"
            # (3 unchanged attributes hidden)
    
          ~ memory {
              ~ apply_threshold          = 0.2 -> 0.1
                # (6 unchanged attributes hidden)
    
                # (1 unchanged block hidden)
            }
    
            # (5 unchanged blocks hidden)
        }
  2. When I use the old apply_threshold or don't specify it (use default) - API returns corresponding new strategy, but it's missing from TF definition, so it wants to remove it:

      ~ resource "castai_workload_scaling_policy" "woop-541-e2e-test" {
            id                = "423635de-1a54-4fc6-9a10-6039a273e76c"
            name              = "woop-541-e2e-test"
            # (3 unchanged attributes hidden)
    
          ~ cpu {
                # (7 unchanged attributes hidden)
    
              - apply_threshold_strategy {
                  - percentage = 0.1 -> null
                  - type       = "PERCENTAGE" -> null
                }
    
                # (1 unchanged block hidden)
            }
    
          ~ memory {
                # (7 unchanged attributes hidden)
    
              - apply_threshold_strategy {
                  - percentage = 0.2 -> null
                  - type       = "PERCENTAGE" -> null
                }
            }
    
            # (4 unchanged blocks hidden)
        }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants