Skip to content

Commit

Permalink
feat: 2.13.1 cherry picks (#9479)
Browse files Browse the repository at this point in the history
* feat: default to ADC when `gcloud` cred helper is configured in docker/config.json when using docker go library (#9469)

* fix: send maxRetries property when it is specified by the user in a cloud run job manifest (#9475)
  • Loading branch information
renzodavid9 authored Jul 24, 2024
1 parent 7f817f3 commit 9bf608b
Show file tree
Hide file tree
Showing 5 changed files with 414 additions and 28 deletions.
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

0 comments on commit 9bf608b

Please sign in to comment.