PRICE-553: add terraform support for pod mutations#662
PRICE-553: add terraform support for pod mutations#662pawelpeksa wants to merge 8 commits intomasterfrom
Conversation
castai/resource_pod_mutation.go
Outdated
| d.SetId("") | ||
| return nil | ||
| } | ||
| if checkErr := sdk.CheckOKResponse(resp, err); checkErr != nil { |
There was a problem hiding this comment.
err is always nil at this point -> so maybe (resp, nil)?
castai/resource_pod_mutation.go
Outdated
| Type: schema.TypeInt, | ||
| Computed: true, | ||
| Description: "Number of pods mutated by this pod mutation.", | ||
| }, |
There was a problem hiding this comment.
should this field be in state at all? it's changing dynamically in the API
castai/resource_pod_mutation.go
Outdated
| if v, ok := d.GetOk(FieldPodMutationSpotDistributionPct); ok { | ||
| pct := int32(v.(int)) | ||
| mutation.SpotDistributionPercentage = &pct | ||
| } |
There was a problem hiding this comment.
will GetOk handle correctly zero value? I mean won't it be ignored by TF?
There was a problem hiding this comment.
it will be ignored, changed
castai/resource_pod_mutation.go
Outdated
| } | ||
|
|
||
| func podMutationStateImporter(ctx context.Context, d *schema.ResourceData, meta any) ([]*schema.ResourceData, error) { | ||
| organizationID, id := parseImportID(d) |
There was a problem hiding this comment.
parseImportID is defined in resource_hibernation_schedule.go. Maybe it should be moved to some shared file?
There was a problem hiding this comment.
not used anymore after other changes
| } | ||
| } | ||
|
|
||
| func podMutationStateImporter(ctx context.Context, d *schema.ResourceData, meta any) ([]*schema.ResourceData, error) { |
There was a problem hiding this comment.
should podMutationStateImporter handle also cluster ID or there is no need? 🤔
There was a problem hiding this comment.
it definitely should, fixing
There was a problem hiding this comment.
I mean, cluster ID is required to fetch pod mutation anyway
castai/resource_pod_mutation.go
Outdated
| } | ||
|
|
||
| // Restart matching workloads | ||
| if v, ok := d.GetOk(FieldPodMutationRestartWorkloads); ok { |
There was a problem hiding this comment.
Same here - does it handle false value properly?
| patchJSON, err := json.Marshal(*g.Config.Patch) | ||
| if err == nil { | ||
| configMap[FieldPodMutationPatch] = string(patchJSON) | ||
| } |
There was a problem hiding this comment.
No Marshal error handling
castai/resource_pod_mutation.go
Outdated
| } | ||
| em := e.(map[string]interface{}) | ||
| key := em[FieldPodMutationTolerationKey].(string) | ||
| operator := em[FieldPodMutationTolerationOperator].(string) |
There was a problem hiding this comment.
should NodeSelectorRequirement have it own specific key and operator constants, and not re use the one from toleration?
castai/resource_pod_mutation.go
Outdated
| Optional: true, | ||
| Elem: &schema.Resource{ | ||
| Schema: map[string]*schema.Schema{ | ||
| FieldPodMutationTolerationKey: { |
There was a problem hiding this comment.
should Affinity/MatchExpressions have it own specific key and operator constants, and not re use the one from toleration?
2 participants
PRICE-553: add terraform support for pod mutations