Service fabric cluster upgrade policy #6273
Conversation
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Hey @JSchenken - thank your for this PR and we're really sorry its taken this long to give it a review, it seems to have fallen through the cracks 😞
I've given the schema a look at and i left some suggestions inline to clean it up and improve the UX, let me know what you think
| @@ -273,6 +273,123 @@ func resourceArmServiceFabricCluster() *schema.Resource { | |||
| }, | |||
| }, | |||
|
|
|||
| "upgrade_description": { | |||
There was a problem hiding this comment.
description doesn't make sense, could we chanege thi to
| "upgrade_description": { | |
| "upgrade_policy": { |
| MaxItems: 1, | ||
| Elem: &schema.Resource{ | ||
| Schema: map[string]*schema.Schema{ | ||
| "force_restart": { |
There was a problem hiding this comment.
we usually put enabled in the name on booleans
| "force_restart": { | |
| "force_restart_enabled": { |
| Type: schema.TypeBool, | ||
| Optional: true, | ||
| }, | ||
| "health_check_retry_timeout": { |
There was a problem hiding this comment.
could we add the units here?
| "health_check_retry_timeout": { | |
| "health_check_retry_timeout_min|hours|seccons": { |
| Optional: true, | ||
| }, | ||
| "health_check_retry_timeout": { | ||
| Type: schema.TypeString, |
There was a problem hiding this comment.
should this be an int instead of a string?
| "health_check_retry_timeout": { | ||
| Type: schema.TypeString, | ||
| Optional: true, | ||
| }, |
There was a problem hiding this comment.
could we validate this property?
| Optional: true, | ||
| Elem: &schema.Resource{ | ||
| Schema: map[string]*schema.Schema{ | ||
| "application_type": { |
There was a problem hiding this comment.
and remove application here
| "application_type": { | |
| "type": { |
| Type: schema.TypeString, | ||
| Required: true, | ||
| }, | ||
| "default_service_type_delta_health_policy": { |
There was a problem hiding this comment.
same goes here
| "default_service_type_delta_health_policy": { | |
| "default_service_policy": { |
| }, | ||
| }, | ||
| }, | ||
| "service_type_delta_health_policy": { |
There was a problem hiding this comment.
and here
| "service_type_delta_health_policy": { | |
| "service_policy": { |
| Optional: true, | ||
| Elem: &schema.Resource{ | ||
| Schema: map[string]*schema.Schema{ | ||
| "service_type": { |
There was a problem hiding this comment.
| "service_type": { | |
| "type": { |
| Type: schema.TypeString, | ||
| Required: true, | ||
| }, | ||
| "max_percent_delta_unhealthy_services": { |
There was a problem hiding this comment.
| "max_percent_delta_unhealthy_services": { | |
| "max_unhealthy_services_percent": { |
|
closing as this was superseded by #10713 |
|
I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 [email protected]. Thanks! |
ghost
locked as resolved and limited conversation to collaborators
Address #6152
Add upgrade_description property, and necessary other blocks, to service_fabric_cluster resource. Documentation also added.