Skip to content

Commit 9bf608b

Browse files
renzodavid9renzodavid9
authored
feat: 2.13.1 cherry picks (#9479)
* 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)
1 parent 7f817f3 commit 9bf608b

File tree

5 files changed

+414
-28
lines changed

5 files changed

+414
-28
lines changed

‎integration/deploy_cloudrun_test.go

+152
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,14 @@ import (
2222
"strings"
2323
"testing"
2424

25+
"github.com/google/go-cmp/cmp"
26+
"github.com/google/uuid"
27+
"google.golang.org/api/option"
2528
"google.golang.org/api/run/v1"
2629

2730
"github.com/GoogleContainerTools/skaffold/v2/integration/skaffold"
2831
"github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/gcp"
32+
"github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/schema/latest"
2933
"github.com/GoogleContainerTools/skaffold/v2/testutil"
3034
)
3135

@@ -69,6 +73,142 @@ func TestDeployCloudRunWithHooks(t *testing.T) {
6973
})
7074
}
7175

76+
func TestDeployJobWithMaxRetries(t *testing.T) {
77+
MarkIntegrationTest(t, NeedsGcp)
78+
79+
tests := []struct {
80+
descrition string
81+
jobManifest string
82+
skaffoldCfg string
83+
args []string
84+
expectedMaxRetries int64
85+
}{
86+
{
87+
descrition: "maxRetries set to specific value",
88+
expectedMaxRetries: 2,
89+
jobManifest: `
90+
apiVersion: run.googleapis.com/v1
91+
kind: Job
92+
metadata:
93+
annotations:
94+
run.googleapis.com/launch-stage: BETA
95+
name: %v
96+
spec:
97+
template:
98+
spec:
99+
template:
100+
spec:
101+
containers:
102+
- image: docker.io/library/busybox:latest
103+
name: job
104+
maxRetries: 2`,
105+
skaffoldCfg: `
106+
apiVersion: %v
107+
kind: Config
108+
metadata:
109+
name: cloud-run-test
110+
manifests:
111+
rawYaml:
112+
- job.yaml
113+
deploy:
114+
cloudrun:
115+
projectid: %v
116+
region: %v`,
117+
},
118+
{
119+
descrition: "maxRetries set to 0",
120+
expectedMaxRetries: 0,
121+
jobManifest: `
122+
apiVersion: run.googleapis.com/v1
123+
kind: Job
124+
metadata:
125+
annotations:
126+
run.googleapis.com/launch-stage: BETA
127+
name: %v
128+
spec:
129+
template:
130+
spec:
131+
template:
132+
spec:
133+
containers:
134+
- image: docker.io/library/busybox:latest
135+
name: job
136+
maxRetries: 0`,
137+
skaffoldCfg: `
138+
apiVersion: %v
139+
kind: Config
140+
metadata:
141+
name: cloud-run-test
142+
manifests:
143+
rawYaml:
144+
- job.yaml
145+
deploy:
146+
cloudrun:
147+
projectid: %v
148+
region: %v`,
149+
},
150+
{
151+
descrition: "maxRetries not specified - default 3",
152+
expectedMaxRetries: 3,
153+
jobManifest: `
154+
apiVersion: run.googleapis.com/v1
155+
kind: Job
156+
metadata:
157+
annotations:
158+
run.googleapis.com/launch-stage: BETA
159+
name: %v
160+
spec:
161+
template:
162+
spec:
163+
template:
164+
spec:
165+
containers:
166+
- image: docker.io/library/busybox:latest
167+
name: job`,
168+
skaffoldCfg: `
169+
apiVersion: %v
170+
kind: Config
171+
metadata:
172+
name: cloud-run-test
173+
manifests:
174+
rawYaml:
175+
- job.yaml
176+
deploy:
177+
cloudrun:
178+
projectid: %v
179+
region: %v`,
180+
},
181+
}
182+
183+
for _, test := range tests {
184+
testutil.Run(t, test.descrition, func(t *testutil.T) {
185+
projectID := "k8s-skaffold"
186+
region := "us-central1"
187+
jobName := fmt.Sprintf("job-%v", uuid.New().String())
188+
skaffoldCfg := fmt.Sprintf(test.skaffoldCfg, latest.Version, projectID, region)
189+
jobManifest := fmt.Sprintf(test.jobManifest, jobName)
190+
191+
tmpDir := t.NewTempDir()
192+
tmpDir.Write("skaffold.yaml", skaffoldCfg)
193+
tmpDir.Write("job.yaml", jobManifest)
194+
195+
skaffold.Run().InDir(tmpDir.Root()).RunOrFail(t.T)
196+
t.Cleanup(func() {
197+
skaffold.Delete(test.args...).InDir(tmpDir.Root()).RunOrFail(t.T)
198+
})
199+
200+
job, err := getJob(context.Background(), projectID, region, jobName)
201+
if err != nil {
202+
t.Fatal(err)
203+
}
204+
205+
if diff := cmp.Diff(job.Spec.Template.Spec.Template.Spec.MaxRetries, test.expectedMaxRetries); diff != "" {
206+
t.Fatalf("Job MaxRetries differ (-got,+want):\n%s", diff)
207+
}
208+
})
209+
}
210+
}
211+
72212
// TODO: remove nolint when test is unskipped
73213
//
74214
//nolint:unused
@@ -82,6 +222,18 @@ func getRunService(ctx context.Context, project, region, service string) (*run.S
82222
return call.Do()
83223
}
84224

225+
func getJob(ctx context.Context, project, region, job string) (*run.Job, error) {
226+
cOptions := []option.ClientOption{option.WithEndpoint(fmt.Sprintf("%s-run.googleapis.com", region))}
227+
cOptions = append(gcp.ClientOptions(ctx), cOptions...)
228+
crclient, err := run.NewService(ctx, cOptions...)
229+
if err != nil {
230+
return nil, err
231+
}
232+
jName := fmt.Sprintf("namespaces/%v/jobs/%v", project, job)
233+
call := crclient.Namespaces.Jobs.Get(jName)
234+
return call.Do()
235+
}
236+
85237
// TODO: remove nolint when test is unskipped
86238
//
87239
//nolint:unused

‎pkg/skaffold/deploy/cloudrun/deploy.go

+35
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ import (
4040
"github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/kubernetes/manifest"
4141
"github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/log"
4242
"github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/output"
43+
logger "github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/output/log"
4344
"github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/schema/latest"
4445
"github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/status"
4546
"github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/sync"
@@ -294,6 +295,37 @@ func (d *Deployer) deployService(crclient *run.APIService, manifest []byte, out
294295
return &resName, nil
295296
}
296297

298+
func (d *Deployer) forceSendValueOfMaxRetries(job *run.Job, manifest []byte) {
299+
maxRetriesPath := []string{"spec", "template", "spec", "template", "spec"}
300+
node := make(map[string]interface{})
301+
302+
if err := k8syaml.Unmarshal(manifest, &node); err != nil {
303+
logger.Entry(context.TODO()).Debugf("Error unmarshaling job into map, skipping maxRetries ForceSendFields logic: %v", err)
304+
return
305+
}
306+
307+
for _, field := range maxRetriesPath {
308+
value := node[field]
309+
child, ok := value.(map[string]interface{})
310+
if !ok {
311+
logger.Entry(context.TODO()).Debugf("Job maxRetries parent fields not found")
312+
return
313+
}
314+
node = child
315+
}
316+
317+
if _, exists := node["maxRetries"]; !exists {
318+
logger.Entry(context.TODO()).Debugf("Job maxRetries property not found")
319+
return
320+
}
321+
322+
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 {
323+
logger.Entry(context.TODO()).Debugf("Job struct doesn't have the required values to force maxRetries sending")
324+
return
325+
}
326+
job.Spec.Template.Spec.Template.Spec.ForceSendFields = append(job.Spec.Template.Spec.Template.Spec.ForceSendFields, "MaxRetries")
327+
}
328+
297329
func (d *Deployer) deployJob(crclient *run.APIService, manifest []byte, out io.Writer) (*RunResourceName, error) {
298330
job := &run.Job{}
299331
if err := k8syaml.Unmarshal(manifest, job); err != nil {
@@ -302,6 +334,9 @@ func (d *Deployer) deployJob(crclient *run.APIService, manifest []byte, out io.W
302334
ErrCode: proto.StatusCode_DEPLOY_READ_MANIFEST_ERR,
303335
})
304336
}
337+
338+
d.forceSendValueOfMaxRetries(job, manifest)
339+
305340
if d.Project != "" {
306341
job.Metadata.Namespace = d.Project
307342
} else if job.Metadata.Namespace == "" {

0 commit comments

Comments
 (0)