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

digitalocean_kubernetes_cluster: does not trigger k8s upgrade on version change #823

Closed
gizero opened this issue Apr 30, 2022 · 3 comments
Closed
Labels

Comments

@gizero
Copy link
Contributor

gizero commented Apr 30, 2022

Bug Report


Describe the bug

We have k8s clusters with minor version pinned by terraform and DO automatic patch upgrades enabled.

When we wanted to trigger a k8s minor version upgrade, we noticed that several changes to the minor version in TF manifests did not trigger the cluster upgrade on DO. When that was the case, following terraform apply runs reported the change was still unapplied but never triggered the actual upgrade.

After digging a bit, it turned out to be related to the control flow in resourceDigitalOceanKubernetesClusterUpdate(), where the execution never even reaches a cluster version check unless changes to the cluster's node_pool are detected as well. This does not seem to be the correct behaviour.

This was a bit hard to figure out because we were pretty sure it worked in the past. When it happened to work, it was probably due to those being the first terraform apply runs after some automatic patch upgrade was performed by DO, yet triggering a node_pool change that required the TF state to be synched.

Is the above analysis correct?

Affected Resource(s)

  • digitalocean_kubernetes_cluster

Expected Behavior

A cluster upgrade should be triggered for any version change.

Actual Behavior

No upgrade is triggered if there is a version change but no contingent node_pool changes

@gizero gizero added the bug label Apr 30, 2022
macno added a commit to authkeys/terraform-provider-digitalocean that referenced this issue Apr 30, 2022
version check does not depend on node_pool changes
macno added a commit to authkeys/terraform-provider-digitalocean that referenced this issue Apr 30, 2022
macno added a commit to authkeys/terraform-provider-digitalocean that referenced this issue Apr 30, 2022
version check does not depend on node_pool changes
@gizero
Copy link
Contributor Author

gizero commented May 1, 2022

Looks like it could have been caught already by acceptance test. At least on a local env TestAccDigitalOceanKubernetesCluster_UpgradeVersion() fails with:

make testacc TESTARGS='-run=Kubernetes
-count=1 -parallel=5 -run=TestAccDigitalOceanKubernetesCluster_UpgradeVersion'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test $(go list ./... |grep -v 'vendor') -v -run=Kubernetes -count=1 -parallel=5 -run=TestAccDigitalOceanKubernetesCluster_UpgradeVersion -timeout 120m
?       github.com/digitalocean/terraform-provider-digitalocean [no test files]
=== RUN   TestAccDigitalOceanKubernetesCluster_UpgradeVersion
=== PAUSE TestAccDigitalOceanKubernetesCluster_UpgradeVersion
=== CONT  TestAccDigitalOceanKubernetesCluster_UpgradeVersion
    resource_digitalocean_kubernetes_cluster_test.go:624: Step 2/2 error: Check failed: 1 error occurred:
                * Check 3/3 error: digitalocean_kubernetes_cluster.foobar: Attribute 'version' expected "1.22.8-do.1", got "1.21.11-do.1"

--- FAIL: TestAccDigitalOceanKubernetesCluster_UpgradeVersion (442.32s)

Unfortunately I wasn't able to find any non-canceled run of the acceptance tests within CI env to confirm.

#824 is a tentative PR that seem to solve the issue.

macno added a commit to authkeys/terraform-provider-digitalocean that referenced this issue May 5, 2022
macno added a commit to authkeys/terraform-provider-digitalocean that referenced this issue May 5, 2022
version check does not depend on node_pool changes
macno added a commit to authkeys/terraform-provider-digitalocean that referenced this issue May 5, 2022
version check does not depend on node_pool changes
scotchneat pushed a commit that referenced this issue May 11, 2022
version check does not depend on node_pool changes
@macno
Copy link
Contributor

macno commented May 11, 2022

The fix has been merged in main, this issue can be closed 🙏

@andrewsomething
Copy link
Member

Fixed via #824

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

No branches or pull requests

3 participants