From ff6c89b85347ce00d43efdd2904d9488af96e4e6 Mon Sep 17 00:00:00 2001 From: Jaylon McShan Date: Fri, 11 Oct 2024 11:24:39 -0500 Subject: [PATCH] Fixed edge case to where we update ttl from 0 to another value --- kubernetes/resource_kubernetes_job_v1.go | 89 ++++++++++++++++--- kubernetes/resource_kubernetes_job_v1_test.go | 67 ++++++++++++++ 2 files changed, 143 insertions(+), 13 deletions(-) diff --git a/kubernetes/resource_kubernetes_job_v1.go b/kubernetes/resource_kubernetes_job_v1.go index caa7bad7dd..ae6fbd46d7 100644 --- a/kubernetes/resource_kubernetes_job_v1.go +++ b/kubernetes/resource_kubernetes_job_v1.go @@ -7,6 +7,7 @@ import ( "context" "fmt" "log" + "strconv" "time" "github.com/hashicorp/terraform-plugin-sdk/v2/diag" @@ -16,6 +17,7 @@ import ( batchv1 "k8s.io/api/batch/v1" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" + apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" pkgApi "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/kubernetes" @@ -55,6 +57,27 @@ func resourceKubernetesJobV1CustomizeDiff(ctx context.Context, d *schema.Resourc return nil } + // Retrieve old and new TTL values as strings + oldTTLRaw, newTTLRaw := d.GetChange("spec.0.ttl_seconds_after_finished") + + var oldTTLStr, newTTLStr string + + if oldTTLRaw != nil { + oldTTLStr, _ = oldTTLRaw.(string) + } + if newTTLRaw != nil { + newTTLStr, _ = newTTLRaw.(string) + } + + oldTTLInt, err := strconv.Atoi(oldTTLStr) + if err != nil { + oldTTLInt = 0 + } + newTTLInt, err := strconv.Atoi(newTTLStr) + if err != nil { + newTTLInt = 0 + } + conn, err := meta.(KubeClientsets).MainClientset() if err != nil { return err @@ -65,23 +88,40 @@ func resourceKubernetesJobV1CustomizeDiff(ctx context.Context, d *schema.Resourc return err } - ttlAttr := d.Get("spec.0.ttl_seconds_after_finished") - ttlSeconds, ok := ttlAttr.(int) - if !ok || ttlSeconds != 0 { - return nil - } - - // getting the job + // Check if the Job exists _, err = conn.BatchV1().Jobs(namespace).Get(ctx, name, metav1.GetOptions{}) if err != nil { - if errors.IsNotFound(err) { - // Suppress diff - d.Clear("spec") - d.Clear("metadata") + if apierrors.IsNotFound(err) { + // Job is missing + if oldTTLInt == 0 { + if newTTLInt == 0 { + // TTL remains 0; suppress diff to prevent recreation + log.Printf("[DEBUG] Job %s not found and ttl_seconds_after_finished remains 0; suppressing diff", d.Id()) + d.Clear("spec") + d.Clear("metadata") + return nil + } else { + // TTL changed from 0 to non-zero; force recreation + log.Printf("[DEBUG] Job %s not found and ttl_seconds_after_finished changed from 0 to %d; forcing recreation", d.Id(), newTTLInt) + d.ForceNew("spec.0.ttl_seconds_after_finished") + return nil + } + } else { + return nil + } + } else { + return err + } + } else { + // Job exists + if oldTTLInt == 0 && newTTLInt != 0 { + // TTL changing from 0 to non-zero; force recreation + log.Printf("[DEBUG] Job %s exists and ttl_seconds_after_finished changed from 0 to %d; forcing recreation", d.Id(), newTTLInt) + d.ForceNew("spec.0.ttl_seconds_after_finished") return nil } - return err } + return nil } @@ -219,6 +259,30 @@ func resourceKubernetesJobV1Update(ctx context.Context, d *schema.ResourceData, return diag.FromErr(err) } + // Attempt to get the Job + _, err = conn.BatchV1().Jobs(namespace).Get(ctx, name, metav1.GetOptions{}) + if err != nil { + if apierrors.IsNotFound(err) { + // Job is missing; cannot update + ttlAttr := d.Get("spec.0.ttl_seconds_after_finished") + ttlStr, _ := ttlAttr.(string) + ttlInt, err := strconv.Atoi(ttlStr) + if err != nil { + ttlInt = 0 + } + if ttlInt == 0 { + // Job was deleted due to TTL = 0; nothing to update + log.Printf("[INFO] Job %s not found but ttl_seconds_after_finished = 0; nothing to update", d.Id()) + return nil + } else { + // Job was deleted unexpectedly; return an error + return diag.Errorf("Job %s not found; cannot update because it has been deleted", d.Id()) + } + } + return diag.Errorf("Error retrieving Job: %s", err) + } + + // Proceed with the update as usual ops := patchMetadata("metadata.0.", "/metadata/", d) if d.HasChange("spec") { @@ -250,7 +314,6 @@ func resourceKubernetesJobV1Update(ctx context.Context, d *schema.ResourceData, } return resourceKubernetesJobV1Read(ctx, d, meta) } - func resourceKubernetesJobV1Delete(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { conn, err := meta.(KubeClientsets).MainClientset() if err != nil { diff --git a/kubernetes/resource_kubernetes_job_v1_test.go b/kubernetes/resource_kubernetes_job_v1_test.go index 5d6d2393fd..ff98d94930 100644 --- a/kubernetes/resource_kubernetes_job_v1_test.go +++ b/kubernetes/resource_kubernetes_job_v1_test.go @@ -271,6 +271,48 @@ func TestAccKubernetesJobV1_customizeDiff_ttlZero(t *testing.T) { }) } +func TestAccKubernetesJobV1_updateTTLFromZero(t *testing.T) { + var conf batchv1.Job + name := fmt.Sprintf("tf-acc-test-%s", acctest.RandString(10)) + imageName := busyboxImage + resourceName := "kubernetes_job_v1.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { + testAccPreCheck(t) + skipIfClusterVersionLessThan(t, "1.21.0") + }, + ProviderFactories: testAccProviderFactories, + Steps: []resource.TestStep{ + // Step 1: Create the Job with ttl_seconds_after_finished = 0 + { + Config: testAccKubernetesJobV1Config_customizeDiff_ttlZero(name, imageName), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckKubernetesJobV1Exists(resourceName, &conf), + resource.TestCheckResourceAttr(resourceName, "spec.0.ttl_seconds_after_finished", "0"), + ), + }, + // Step 2: Wait for the Job to complete and be deleted + { + PreConfig: func() { + time.Sleep(70 * time.Second) + }, + Config: testAccKubernetesJobV1Config_customizeDiff_ttlZero(name, imageName), + PlanOnly: true, + ExpectNonEmptyPlan: false, + }, + // Step 3: Update the Job to ttl_seconds_after_finished = 5 + { + Config: testAccKubernetesJobV1Config_customizeDiff_ttlFive(name, imageName), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckKubernetesJobV1Exists(resourceName, &conf), + resource.TestCheckResourceAttr(resourceName, "spec.0.ttl_seconds_after_finished", "5"), + ), + }, + }, + }) +} + func testAccCheckJobV1Waited(minDuration time.Duration) func(*terraform.State) error { // NOTE this works because this function is called when setting up the test // and the function it returns is called after the resource has been created @@ -575,3 +617,28 @@ resource "kubernetes_job_v1" "test" { } `, name, imageName) } + +func testAccKubernetesJobV1Config_customizeDiff_ttlFive(name, imageName string) string { + return fmt.Sprintf(` +resource "kubernetes_job_v1" "test" { + metadata { + name = "%s" + } + spec { + ttl_seconds_after_finished = 5 + template { + metadata {} + spec { + container { + name = "wait-test" + image = "%s" + command = ["sleep", "60"] + } + restart_policy = "Never" + } + } + } + wait_for_completion = false +} +`, name, imageName) +}