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

fix: send maxRetries property when it is specified by the user in a cloud run job manifest #9475

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
152 changes: 152 additions & 0 deletions integration/deploy_cloudrun_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
35 changes: 35 additions & 0 deletions pkg/skaffold/deploy/cloudrun/deploy.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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 {
Expand All @@ -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 == "" {
Expand Down
Loading
Loading