Repository: hashicorp/terraform-provider-kubernetes

Issue:
Daemonset wait_for_rollout does not appear to have any impact.
### Terraform Version, Provider Version and Kubernetes Version
```
Terraform version: Terraform v1.4.5
Kubernetes provider version: 2.20.0
Kubernetes version: 1.24.11
```

### Affected Resource(s)
- kubernetes_daemonset

### Steps to Reproduce
1. `terraform apply` 

### Expected Behavior
What should have happened?
A kubernetes_daemonset should wait on creation/update until the pods are ready before allowing terraform to procede

### Actual Behavior
What actually happened?
Create/Update is instant and terraform moves on to any dependent resources without waiting.

### Important Factoids
<!-- Are there anything atypical about your accounts that we should know? For example: Running in EC2 Classic? Custom version of OpenStack? Tight ACLs?-->

### References
These issues:
https://github.com/hashicorp/terraform-provider-kubernetes/issues/919
https://github.com/hashicorp/terraform-provider-kubernetes/pull/1053

And this documentation:
https://registry.terraform.io/providers/hashicorp/kubernetes/latest/docs/resources/daemonset#wait_for_rollout

All indicate that kubernetes_daemonset wait_for_rollout should do something.

### Community Note
<!--- Please keep this note for the community --->
* Please vote on this issue by adding a 👍 [reaction](https://blog.github.com/2016-03-10-add-reactions-to-pull-requests-issues-and-comments/) to the original issue to help the community and maintainers prioritize this request
* If you are interested in working on this issue or have submitted a pull request, please leave a comment


Hints from issue comments:
@matthi-g: I noticed the same problem. My current workaround is to use a manifest instead and define the wait block with 
`wait {
  rollout = true
}`

The problem with that approach is that you need permissions to list custom resource definitions. I hope either the necessity for CRDs in the manifest or the bug with the wait_for_rollout get fixed soon.

For reference of the manaifest CRD permissions:
#1665@BBBmau: Hello, thank you for opening this issue @matthi-g. Could you provide us with a tfconfig that reproduces this issue?@matthi-g: Hi @BBBmau,
the goal of my daemonset is to pre pull images and keep them at least until the daemon set is deleted. The first (buggy) version of my implementation uses the daemon_set_v1 resource:
[daemon_set_v1.txt](https://github.com/hashicorp/terraform-provider-kubernetes/files/11445656/daemon_set_v1.txt)

The second version does the same thing but instead makes use of the manifest resource:
[manifest.txt](https://github.com/hashicorp/terraform-provider-kubernetes/files/11445716/manifest.txt)

Creation of the daemonset_v1 finishes after 0s but observing the state of the created pods on the k8s node indicates that the pods are not up and in running state. The creation of manifest finishes after 28 s (depends on the used images and bandwidth) and this seems to line up with the state of the pods.

@pidydx: @BBBmau I don't have an example on hand, but as noted above daemonsets complete instantly and don't wait for the nodes to be ready.  Without diving into the code to see how the check is supposed to be happening, my guess is that the Daemonset check is not validating that the number pod pods running is equal to the number of current nodes rather than checking if the cluster is simply ready to deploy the pods to any node that comes online.@sbocinec: @BBBmau I confirm, the issue is valid and still present. Here is a short reproducer:
```
terraform {
  required_version = "~> 1.7"
    kubernetes = {
      source  = "hashicorp/kubernetes"
      version = "2.25.2"
    }
  }
}

# This should be configured individually depending in your setup
# the locals used here are omitted in the reproducer
provider "kubernetes" {
  host                   = local.cluster_config.cluster_endpoint
  cluster_ca_certificate = base64decode(local.cluster_config.cluster_ca)

  exec {
    api_version = "client.authentication.k8s.io/v1beta1"
    args        = ["eks", "get-token", "--cluster-name", local.cluster_config.cluster_name]
    command     = "awscli2"
  }
}

resource "kubernetes_daemon_set_v1" "i-am-not-waiting-for-rollout" {
  metadata {
    name      = "i-am-not-waiting-for-rollout"
    namespace = "default"
  }

  spec {
    selector {
      match_labels = {
        name = "i-am-not-waiting-for-rollout"
      }
    }
    template {
      metadata {
        labels = {
          name = "i-am-not-waiting-for-rollout"
        }
      }
      spec {
        container {
          name              = "pause"
          image             = "public.ecr.aws/eks-distro/kubernetes/pause:3.9@sha256:4668ea92d0ce9b5e5d8e84a4d3875d99aea7892e136a873425095f60bc22c49a"
          image_pull_policy = "IfNotPresent"
        }
        host_pid = true
        init_container {
          name              = "tuner"
          args = [
            "sysctl fs.inotify.max_user_instances=518"
          ]
          command = [
            "/bin/chroot",
            "/host",
            "/bin/bash",
            "-c",
            "--",
          ]
          image             = "public.ecr.aws/docker/library/busybox:1.36-glibc@sha256:e046063223f7eaafbfbc026aa3954d9a31b9f1053ba5db04a4f1fdc97abd8963"
          image_pull_policy = "IfNotPresent"
          security_context {
            capabilities {
              add = [
                "SYS_CHROOT",
              ]
              drop = [
                "all",
              ]
            }
            privileged = true
          }
          volume_mount {
            mount_path        = "/host"
            mount_propagation = "Bidirectional"
            name              = "hostfs"
          }
        }
        volume {
          host_path {
            path = "/"
          }
          name = "hostfs"
        }
      }
    }
  }
  wait_for_rollout = true
}
```

Initial apply is instant:
```
  Enter a value: yes                                                                                                           
                                                                                                                               
kubernetes_daemon_set_v1.i-am-not-waiting-for-rollout: Creating...                                                             
kubernetes_daemon_set_v1.i-am-not-waiting-for-rollout: Creation complete after 1s [id=default/i-am-not-waiting-for-rollout]
Apply complete! Resources: 1 added, 0 changed, 0 destroyed.                                                                    
```

Any further change is applied instantly, without waiting for the DS to rollout new pods.
```
  Enter a value: yes                                                                                                           
                                                                                                                               
kubernetes_daemon_set_v1.i-am-not-waiting-for-rollout: Modifying... [id=default/i-am-not-waiting-for-rollout]                  
kubernetes_daemon_set_v1.i-am-not-waiting-for-rollout: Modifications complete after 2s [id=default/i-am-not-waiting-for-rollout]                                                                                                                              
                                                                                                                               
Apply complete! Resources: 0 added, 1 changed, 0 destroyed.  
```

:warning:  As others mentioned, workaround is to use `kubernetes_manifest` resource, but if you define a DaemonSet  and trigger any change, it also fails as there is a bug with computed fields not being respected https://github.com/hashicorp/terraform-provider-kubernetes/issues/1591 . Currently, there is no way to manage DaemonSets with `wait_for_rollout = true` in this provider.@sbocinec: I have identified the issue and trying to prepare a fix https://github.com/hashicorp/terraform-provider-kubernetes/pull/2419 - need to test the fix yet.@sbocinec: I have meanwhile tested the PR, updated acceptance tests and the PR is from my POV ready. Could you please have a look @BBBmau? 

Though, the fix is going to break many existing TF configs as the ineffective `wait_for_rollout` is set to `true` by default... I have wrote more in the PR description.@github-actions: Marking this issue as stale due to inactivity. If this issue receives no comments in the next 30 days it will automatically be closed. If this issue was automatically closed and you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. This helps our maintainers find and focus on the active issues. Maintainers may also remove the stale label at their discretion. Thank you!

Solution patch:
diff --git a/.changelog/2789.txt b/.changelog/2789.txt
new file mode 100644
index 00000000..e1af2f12
--- /dev/null
+++ b/.changelog/2789.txt
@@ -0,0 +1,3 @@
+```release-note:bug
+`resource/kubernetes_daemon_set_v1`: fix an issue with the provider not waiting for rollout with `wait_for_rollout = true`.
+```
\ No newline at end of file
diff --git a/kubernetes/resource_kubernetes_daemon_set_v1.go b/kubernetes/resource_kubernetes_daemon_set_v1.go
index edbb17a1..c3094480 100644
--- a/kubernetes/resource_kubernetes_daemon_set_v1.go
+++ b/kubernetes/resource_kubernetes_daemon_set_v1.go
@@ -172,7 +172,7 @@ func resourceKubernetesDaemonSetV1Create(ctx context.Context, d *schema.Resource
 
 	if d.Get("wait_for_rollout").(bool) {
 		err = retry.RetryContext(ctx, d.Timeout(schema.TimeoutCreate),
-			waitForDaemonSetReplicasFunc(ctx, conn, metadata.Namespace, metadata.Name))
+			waitForDaemonSetPodsFunc(ctx, conn, metadata.Namespace, metadata.Name))
 		if err != nil {
 			return diag.FromErr(err)
 		}
@@ -223,7 +223,7 @@ func resourceKubernetesDaemonSetV1Update(ctx context.Context, d *schema.Resource
 
 	if d.Get("wait_for_rollout").(bool) {
 		err = retry.RetryContext(ctx, d.Timeout(schema.TimeoutUpdate),
-			waitForDaemonSetReplicasFunc(ctx, conn, namespace, name))
+			waitForDaemonSetPodsFunc(ctx, conn, namespace, name))
 		if err != nil {
 			return diag.FromErr(err)
 		}
@@ -333,22 +333,26 @@ func resourceKubernetesDaemonSetV1Exists(ctx context.Context, d *schema.Resource
 	return true, err
 }
 
-func waitForDaemonSetReplicasFunc(ctx context.Context, conn *kubernetes.Clientset, ns, name string) retry.RetryFunc {
+func waitForDaemonSetPodsFunc(ctx context.Context, conn *kubernetes.Clientset, ns, name string) retry.RetryFunc {
 	return func() *retry.RetryError {
 		daemonSet, err := conn.AppsV1().DaemonSets(ns).Get(ctx, name, metav1.GetOptions{})
 		if err != nil {
 			return retry.NonRetryableError(err)
 		}
 
-		desiredReplicas := daemonSet.Status.DesiredNumberScheduled
-		log.Printf("[DEBUG] Current number of labelled replicas of %q: %d (of %d)\n",
-			daemonSet.GetName(), daemonSet.Status.CurrentNumberScheduled, desiredReplicas)
+		desiredPods := daemonSet.Status.DesiredNumberScheduled
 
-		if daemonSet.Status.CurrentNumberScheduled == desiredReplicas {
-			return nil
+		if daemonSet.Generation > daemonSet.Status.ObservedGeneration {
+			return retry.RetryableError(fmt.Errorf("waiting for rollout to start."))
 		}
 
-		return retry.RetryableError(fmt.Errorf("Waiting for %d replicas of %q to be scheduled (%d)",
-			desiredReplicas, daemonSet.GetName(), daemonSet.Status.CurrentNumberScheduled))
+		if daemonSet.Generation == daemonSet.Status.ObservedGeneration {
+			if daemonSet.Status.NumberReady == desiredPods {
+				return nil
+			}
+			return retry.RetryableError(fmt.Errorf("waiting for rollout to finish: %d pods desired; %d pods ready",
+				desiredPods, daemonSet.Status.NumberReady))
+		}
+		return retry.NonRetryableError(fmt.Errorf("observed generation %d is not expected to be greater than generation %d", daemonSet.Status.ObservedGeneration, daemonSet.Generation))
 	}
 }



Test patch:
diff --git a/kubernetes/resource_kubernetes_daemon_set_v1_test.go b/kubernetes/resource_kubernetes_daemon_set_v1_test.go
index 3ad1fb57..2fbad42b 100644
--- a/kubernetes/resource_kubernetes_daemon_set_v1_test.go
+++ b/kubernetes/resource_kubernetes_daemon_set_v1_test.go
@@ -890,8 +890,9 @@ func testAccKubernetesDaemonSetV1ConfigWithTemplateMetadata(depName, imageName s
 
       spec {
         container {
-          image = "%s"
-          name  = "containername"
+          image   = "%s"
+          name    = "containername"
+          command = ["sleep", "infinity"]
         }
         termination_grace_period_seconds = 1
       }
@@ -977,8 +978,9 @@ func testAccKubernetesDaemonSetV1WithInitContainer(depName, imageName string) st
         }
 
         container {
-          image = "%s"
-          name  = "containername"
+          image   = "%s"
+          name    = "containername"
+          command = ["sleep", "infinity"]
         }
         termination_grace_period_seconds = 1
       }
@@ -1010,8 +1012,9 @@ func testAccKubernetesDaemonSetV1WithNoTopLevelLabels(depName, imageName string)
 
       spec {
         container {
-          image = "%s"
-          name  = "containername"
+          image   = "%s"
+          name    = "containername"
+          command = ["sleep", "infinity"]
         }
         termination_grace_period_seconds = 1
       }
@@ -1064,8 +1067,9 @@ func testAccKubernetesDaemonSetV1ConfigWithTolerations(rcName, imageName string,
         }
 
         container {
-          image = "%s"
-          name  = "containername"
+          image   = "%s"
+          name    = "containername"
+          command = ["sleep", "infinity"]
         }
         termination_grace_period_seconds = 1
       }
@@ -1119,6 +1123,7 @@ func testAccKubernetesDaemonSetV1ConfigWithContainerSecurityContextSeccompProfil
       }
     }
   }
+  wait_for_rollout = false
 }
 `, deploymentName, seccompProfileType, imageName, seccompProfileType)
 }
@@ -1169,6 +1174,7 @@ func testAccKubernetesDaemonSetV1ConfigWithContainerSecurityContextSeccompProfil
       }
     }
   }
+  wait_for_rollout = false
 }
 `, deploymentName, imageName)
 }
@@ -1199,20 +1205,19 @@ func testAccKubernetesDaemonSetV1ConfigWithResourceRequirements(deploymentName,
 
       spec {
         container {
-          image = "%s"
-          name  = "containername"
+          image   = "%s"
+          name    = "containername"
+          command = ["sleep", "infinity"]
 
           resources {
             limits = {
-              cpu          = "0.5"
-              memory       = "512Mi"
-              "nvidia/gpu" = "1"
+              cpu    = "0.5"
+              memory = "512Mi"
             }
 
             requests = {
-              cpu          = "250m"
-              memory       = "50Mi"
-              "nvidia/gpu" = "1"
+              cpu    = "250m"
+              memory = "50Mi"
             }
           }
         }
@@ -1250,8 +1255,9 @@ func testAccKubernetesDaemonSetV1ConfigWithEmptyResourceRequirements(deploymentN
 
       spec {
         container {
-          image = "%s"
-          name  = "containername"
+          image   = "%s"
+          name    = "containername"
+          command = ["sleep", "infinity"]
 
           resources {
             limits   = {}
@@ -1292,8 +1298,9 @@ func testAccKubernetesDaemonSetV1ConfigWithResourceRequirementsLimitsOnly(deploy
 
       spec {
         container {
-          image = "%s"
-          name  = "containername"
+          image   = "%s"
+          name    = "containername"
+          command = ["sleep", "infinity"]
 
           resources {
             limits = {