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
azurerm_kubernetes_cluster
: Expose automatic_channel_upgrade
#10530
azurerm_kubernetes_cluster
: Expose automatic_channel_upgrade
#10530
Conversation
azurerm/internal/services/containers/kubernetes_cluster_resource.go
Outdated
Show resolved
Hide resolved
c7b2c66
to
0275f9f
Compare
0275f9f
to
4d7d6f3
Compare
azurerm_kubernetes_cluster
: Expose auto_upgrade_channel
azurerm_kubernetes_cluster
: Expose automatic_channel_upgrade
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hey @Brunhil
Thanks for pushing those changes.
Taking a look through on the whole this looks pretty good - but we should remove the none
option here in favour of inferring that when the field is omitted (since this is Optional and will be unset by default). Since this otherwise looks good and I've got a few other Kubernetes PR's to get in today, I hope you don't mind but I'm going to push a commit to fix up the comments so that we can get this merged.
Thanks!
azurerm/internal/services/containers/kubernetes_cluster_resource.go
Outdated
Show resolved
Hide resolved
Optional: true, | ||
Computed: true, | ||
ValidateFunc: validation.StringInSlice([]string{ | ||
string(containerservice.UpgradeChannelNone), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(as above) we can make this field empty when it's None
:
string(containerservice.UpgradeChannelNone), |
azurerm/internal/services/containers/kubernetes_cluster_resource.go
Outdated
Show resolved
Hide resolved
@@ -72,6 +72,12 @@ In addition, one of either `identity` or `service_principal` blocks must be spec | |||
|
|||
--- | |||
|
|||
* `automatic_channel_upgrade` - (Optional) The upgrade channel for this Kubernetes Cluster. Possible values are `none`, `patch`, `rapid`, and `stable`. The default value is `none`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
since we're inferring none
based on the field being omitted - can we make this:
* `automatic_channel_upgrade` - (Optional) The upgrade channel for this Kubernetes Cluster. Possible values are `none`, `patch`, `rapid`, and `stable`. The default value is `none`. | |
* `automatic_channel_upgrade` - (Optional) The upgrade channel for this Kubernetes Cluster. Possible values are `patch`, `rapid`, and `stable`. |
Check: resource.ComposeTestCheckFunc( | ||
check.That(data.ResourceName).ExistsInAzure(r), | ||
check.That(data.ResourceName).Key("kubernetes_version").HasValue(olderKubernetesVersion), | ||
check.That(data.ResourceName).Key("automatic_channel_upgrade").HasValue("rapid"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
since this value is updateable, could we take this from: rapid -> patch -> none -> stable to ensure the lifecycle works?
dismissing since changes have been pushed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @Brunhil & @tombuildsstuff - LGTM 👍
The basicVMSS and Upgrade Channel tests look good:
|
This has been released in version 2.48.0 of the provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. As an example: provider "azurerm" {
version = "~> 2.48.0"
}
# ... other configuration ... |
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 👉 hashibot-feedback@hashicorp.com. Thanks! |
Fixes #10407
=== RUN TestAccKubernetesCluster_upgradeChannel
=== PAUSE TestAccKubernetesCluster_upgradeChannel
=== CONT TestAccKubernetesCluster_upgradeChannel
--- PASS: TestAccKubernetesCluster_upgradeChannel (824.85s)
PASS
ok github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/containers 828.691s