fix: send maxRetries property when it is specified by the user in a c…

…loud run job manifest (GoogleContainerTools#9475)

integration/deploy_cloudrun_test.go

@@ -22,10 +22,14 @@ import (
 	"strings"
 	"testing"
 
+	"github.com/google/go-cmp/cmp"
+	"github.com/google/uuid"
+	"google.golang.org/api/option"
 	"google.golang.org/api/run/v1"
 
 	"github.com/GoogleContainerTools/skaffold/v2/integration/skaffold"
 	"github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/gcp"
+	"github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/schema/latest"
 	"github.com/GoogleContainerTools/skaffold/v2/testutil"
 )
 
@@ -69,6 +73,142 @@ func TestDeployCloudRunWithHooks(t *testing.T) {
 	})
 }
 
+func TestDeployJobWithMaxRetries(t *testing.T) {
+	MarkIntegrationTest(t, NeedsGcp)
+
+	tests := []struct {
+		descrition         string
+		jobManifest        string
+		skaffoldCfg        string
+		args               []string
+		expectedMaxRetries int64
+	}{
+		{
+			descrition:         "maxRetries set to specific value",
+			expectedMaxRetries: 2,
+			jobManifest: `
+apiVersion: run.googleapis.com/v1
+kind: Job
+metadata:
+  annotations:
+    run.googleapis.com/launch-stage: BETA
+  name: %v
+spec:
+  template:
+    spec:
+      template:
+        spec:
+          containers:
+            - image: docker.io/library/busybox:latest
+              name: job
+          maxRetries: 2`,
+			skaffoldCfg: `
+apiVersion: %v
+kind: Config
+metadata:
+   name: cloud-run-test
+manifests:
+  rawYaml:
+    - job.yaml
+deploy:
+  cloudrun:
+    projectid: %v
+    region: %v`,
+		},
+		{
+			descrition:         "maxRetries set to 0",
+			expectedMaxRetries: 0,
+			jobManifest: `
+apiVersion: run.googleapis.com/v1
+kind: Job
+metadata:
+  annotations:
+    run.googleapis.com/launch-stage: BETA
+  name: %v
+spec:
+  template:
+    spec:
+      template:
+        spec:
+          containers:
+            - image: docker.io/library/busybox:latest
+              name: job
+          maxRetries: 0`,
+			skaffoldCfg: `
+apiVersion: %v
+kind: Config
+metadata:
+   name: cloud-run-test
+manifests:
+  rawYaml:
+    - job.yaml
+deploy:
+  cloudrun:
+    projectid: %v
+    region: %v`,
+		},
+		{
+			descrition:         "maxRetries not specified - default 3",
+			expectedMaxRetries: 3,
+			jobManifest: `
+apiVersion: run.googleapis.com/v1
+kind: Job
+metadata:
+  annotations:
+    run.googleapis.com/launch-stage: BETA
+  name: %v
+spec:
+  template:
+    spec:
+      template:
+        spec:
+          containers:
+            - image: docker.io/library/busybox:latest
+              name: job`,
+			skaffoldCfg: `
+apiVersion: %v
+kind: Config
+metadata:
+   name: cloud-run-test
+manifests:
+  rawYaml:
+    - job.yaml
+deploy:
+  cloudrun:
+    projectid: %v
+    region: %v`,
+		},
+	}
+
+	for _, test := range tests {
+		testutil.Run(t, test.descrition, func(t *testutil.T) {
+			projectID := "k8s-skaffold"
+			region := "us-central1"
+			jobName := fmt.Sprintf("job-%v", uuid.New().String())
+			skaffoldCfg := fmt.Sprintf(test.skaffoldCfg, latest.Version, projectID, region)
+			jobManifest := fmt.Sprintf(test.jobManifest, jobName)
+
+			tmpDir := t.NewTempDir()
+			tmpDir.Write("skaffold.yaml", skaffoldCfg)
+			tmpDir.Write("job.yaml", jobManifest)
+
+			skaffold.Run().InDir(tmpDir.Root()).RunOrFail(t.T)
+			t.Cleanup(func() {
+				skaffold.Delete(test.args...).InDir(tmpDir.Root()).RunOrFail(t.T)
+			})
+
+			job, err := getJob(context.Background(), projectID, region, jobName)
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			if diff := cmp.Diff(job.Spec.Template.Spec.Template.Spec.MaxRetries, test.expectedMaxRetries); diff != "" {
+				t.Fatalf("Job MaxRetries differ (-got,+want):\n%s", diff)
+			}
+		})
+	}
+}
+
 // TODO: remove nolint when test is unskipped
 //
 //nolint:unused
@@ -82,6 +222,18 @@ func getRunService(ctx context.Context, project, region, service string) (*run.S
 	return call.Do()
 }
 
+func getJob(ctx context.Context, project, region, job string) (*run.Job, error) {
+	cOptions := []option.ClientOption{option.WithEndpoint(fmt.Sprintf("%s-run.googleapis.com", region))}
+	cOptions = append(gcp.ClientOptions(ctx), cOptions...)
+	crclient, err := run.NewService(ctx, cOptions...)
+	if err != nil {
+		return nil, err
+	}
+	jName := fmt.Sprintf("namespaces/%v/jobs/%v", project, job)
+	call := crclient.Namespaces.Jobs.Get(jName)
+	return call.Do()
+}
+
 // TODO: remove nolint when test is unskipped
 //
 //nolint:unused

pkg/skaffold/deploy/cloudrun/deploy.go

@@ -40,6 +40,7 @@ import (
 	"github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/kubernetes/manifest"
 	"github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/log"
 	"github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/output"
+	logger "github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/output/log"
 	"github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/schema/latest"
 	"github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/status"
 	"github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/sync"
@@ -294,6 +295,37 @@ func (d *Deployer) deployService(crclient *run.APIService, manifest []byte, out
 	return &resName, nil
 }
 
+func (d *Deployer) forceSendValueOfMaxRetries(job *run.Job, manifest []byte) {
+	maxRetriesPath := []string{"spec", "template", "spec", "template", "spec"}
+	node := make(map[string]interface{})
+
+	if err := k8syaml.Unmarshal(manifest, &node); err != nil {
+		logger.Entry(context.TODO()).Debugf("Error unmarshaling job into map, skipping maxRetries ForceSendFields logic: %v", err)
+		return
+	}
+
+	for _, field := range maxRetriesPath {
+		value := node[field]
+		child, ok := value.(map[string]interface{})
+		if !ok {
+			logger.Entry(context.TODO()).Debugf("Job maxRetries parent fields not found")
+			return
+		}
+		node = child
+	}
+
+	if _, exists := node["maxRetries"]; !exists {
+		logger.Entry(context.TODO()).Debugf("Job maxRetries property not found")
+		return
+	}
+
+	if job.Spec == nil || job.Spec.Template == nil || job.Spec.Template.Spec == nil || job.Spec.Template.Spec.Template == nil || job.Spec.Template.Spec.Template.Spec == nil {
+		logger.Entry(context.TODO()).Debugf("Job struct doesn't have the required values to force maxRetries sending")
+		return
+	}
+	job.Spec.Template.Spec.Template.Spec.ForceSendFields = append(job.Spec.Template.Spec.Template.Spec.ForceSendFields, "MaxRetries")
+}
+
 func (d *Deployer) deployJob(crclient *run.APIService, manifest []byte, out io.Writer) (*RunResourceName, error) {
 	job := &run.Job{}
 	if err := k8syaml.Unmarshal(manifest, job); err != nil {
@@ -302,6 +334,9 @@ func (d *Deployer) deployJob(crclient *run.APIService, manifest []byte, out io.W
 			ErrCode: proto.StatusCode_DEPLOY_READ_MANIFEST_ERR,
 		})
 	}
+
+	d.forceSendValueOfMaxRetries(job, manifest)
+
 	if d.Project != "" {
 		job.Metadata.Namespace = d.Project
 	} else if job.Metadata.Namespace == "" {