Skip to content

Commit 2df4983

Browse files
menahyouyeahmenahyouyeah
committed
Add support for configurable statusCheckDeadlineSeconds field for Cloud Run
1 parent ad1a074 commit 2df4983

File tree

4 files changed

+102
-34
lines changed

4 files changed

+102
-34
lines changed

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

+96-27
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import (
2424
"net/http/httptest"
2525
"os"
2626
"testing"
27+
"time"
2728

2829
"github.com/google/go-cmp/cmp"
2930
"google.golang.org/api/option"
@@ -46,21 +47,25 @@ const (
4647
configName = "default"
4748
)
4849

50+
var defaultStatusCheckDeadline = 10 * time.Minute
51+
4952
func TestDeployService(tOuter *testing.T) {
5053
tests := []struct {
51-
description string
52-
toDeploy *run.Service
53-
defaultProject string
54-
region string
55-
expectedPath string
56-
httpErr int
57-
errCode proto.StatusCode
54+
description string
55+
toDeploy *run.Service
56+
defaultProject string
57+
region string
58+
statusCheckDeadlineSec time.Duration
59+
expectedPath string
60+
httpErr int
61+
errCode proto.StatusCode
5862
}{
5963
{
60-
description: "test deploy",
61-
defaultProject: "testProject",
62-
region: "us-central1",
63-
expectedPath: "/v1/projects/testProject/locations/us-central1/services",
64+
description: "test deploy",
65+
defaultProject: "testProject",
66+
region: "us-central1",
67+
expectedPath: "/v1/projects/testProject/locations/us-central1/services",
68+
statusCheckDeadlineSec: defaultStatusCheckDeadline,
6469
toDeploy: &run.Service{
6570
ApiVersion: "serving.knative.dev/v1",
6671
Kind: "Service",
@@ -70,10 +75,25 @@ func TestDeployService(tOuter *testing.T) {
7075
},
7176
},
7277
{
73-
description: "test deploy with specified project",
74-
defaultProject: "testProject",
75-
region: "us-central1",
76-
expectedPath: "/v1/projects/testProject/locations/us-central1/services",
78+
description: "test deploy with status check deadline set to a non default value",
79+
defaultProject: "testProject",
80+
region: "us-central1",
81+
expectedPath: "/v1/projects/testProject/locations/us-central1/services",
82+
statusCheckDeadlineSec: 15 * time.Minute,
83+
toDeploy: &run.Service{
84+
ApiVersion: "serving.knative.dev/v1",
85+
Kind: "Service",
86+
Metadata: &run.ObjectMeta{
87+
Name: "test-service",
88+
},
89+
},
90+
},
91+
{
92+
description: "test deploy with specified project",
93+
defaultProject: "testProject",
94+
region: "us-central1",
95+
statusCheckDeadlineSec: defaultStatusCheckDeadline,
96+
expectedPath: "/v1/projects/testProject/locations/us-central1/services",
7797
toDeploy: &run.Service{
7898
ApiVersion: "serving.knative.dev/v1",
7999
Kind: "Service",
@@ -84,10 +104,11 @@ func TestDeployService(tOuter *testing.T) {
84104
},
85105
},
86106
{
87-
description: "test permission denied on deploy errors",
88-
defaultProject: "testProject",
89-
region: "us-central1",
90-
httpErr: http.StatusUnauthorized,
107+
description: "test permission denied on deploy errors",
108+
defaultProject: "testProject",
109+
region: "us-central1",
110+
statusCheckDeadlineSec: defaultStatusCheckDeadline,
111+
httpErr: http.StatusUnauthorized,
91112
toDeploy: &run.Service{
92113
ApiVersion: "serving.knative.dev/v1",
93114
Kind: "Service",
@@ -99,8 +120,9 @@ func TestDeployService(tOuter *testing.T) {
99120
errCode: proto.StatusCode_DEPLOY_CLOUD_RUN_GET_SERVICE_ERR,
100121
},
101122
{
102-
description: "test no project specified",
103-
region: "us-central1",
123+
description: "test no project specified",
124+
region: "us-central1",
125+
statusCheckDeadlineSec: defaultStatusCheckDeadline,
104126
toDeploy: &run.Service{
105127
ApiVersion: "serving.knative.dev/v1",
106128
Kind: "Service",
@@ -139,7 +161,14 @@ func TestDeployService(tOuter *testing.T) {
139161
w.Write(b)
140162
}))
141163

142-
deployer, _ := NewDeployer(&runcontext.RunContext{}, &label.DefaultLabeller{}, &latest.CloudRunDeploy{ProjectID: test.defaultProject, Region: test.region}, configName)
164+
deployer, _ := NewDeployer(
165+
&runcontext.RunContext{},
166+
&label.DefaultLabeller{},
167+
&latest.CloudRunDeploy{
168+
ProjectID: test.defaultProject,
169+
Region: test.region},
170+
configName,
171+
test.statusCheckDeadlineSec)
143172
deployer.clientOptions = append(deployer.clientOptions, option.WithEndpoint(ts.URL), option.WithoutAuthentication())
144173
deployer.useGcpOptions = false
145174
manifestList, _ := json.Marshal(test.toDeploy)
@@ -311,7 +340,15 @@ func TestDeployJob(tOuter *testing.T) {
311340
w.Write(b)
312341
}))
313342

314-
deployer, _ := NewDeployer(&runcontext.RunContext{}, &label.DefaultLabeller{}, &latest.CloudRunDeploy{ProjectID: test.defaultProject, Region: test.region}, configName)
343+
deployer, _ := NewDeployer(
344+
&runcontext.RunContext{},
345+
&label.DefaultLabeller{},
346+
&latest.CloudRunDeploy{
347+
ProjectID: test.defaultProject,
348+
Region: test.region,
349+
},
350+
configName,
351+
defaultStatusCheckDeadline)
315352
deployer.clientOptions = append(deployer.clientOptions, option.WithEndpoint(ts.URL), option.WithoutAuthentication())
316353
deployer.useGcpOptions = false
317354
manifestList, _ := k8syaml.Marshal(test.toDeploy)
@@ -469,7 +506,15 @@ func TestDeployRewrites(tOuter *testing.T) {
469506
}
470507
w.Write(b)
471508
}))
472-
deployer, _ := NewDeployer(&runcontext.RunContext{}, &label.DefaultLabeller{}, &latest.CloudRunDeploy{ProjectID: test.defaultProject, Region: test.region}, "")
509+
deployer, _ := NewDeployer(
510+
&runcontext.RunContext{},
511+
&label.DefaultLabeller{},
512+
&latest.CloudRunDeploy{
513+
ProjectID: test.defaultProject,
514+
Region: test.region,
515+
},
516+
"",
517+
defaultStatusCheckDeadline)
473518
deployer.clientOptions = append(deployer.clientOptions, option.WithEndpoint(ts.URL), option.WithoutAuthentication())
474519
deployer.useGcpOptions = false
475520
m, _ := json.Marshal(test.toDeploy)
@@ -555,7 +600,15 @@ func TestCleanupService(tOuter *testing.T) {
555600
w.Write(b)
556601
}))
557602
defer ts.Close()
558-
deployer, _ := NewDeployer(&runcontext.RunContext{}, &label.DefaultLabeller{}, &latest.CloudRunDeploy{ProjectID: test.defaultProject, Region: test.region}, configName)
603+
deployer, _ := NewDeployer(
604+
&runcontext.RunContext{},
605+
&label.DefaultLabeller{},
606+
&latest.CloudRunDeploy{
607+
ProjectID: test.defaultProject,
608+
Region: test.region,
609+
},
610+
configName,
611+
defaultStatusCheckDeadline)
559612
deployer.clientOptions = append(deployer.clientOptions, option.WithEndpoint(ts.URL), option.WithoutAuthentication())
560613
deployer.useGcpOptions = false
561614
manifestListByConfig := manifest.NewManifestListByConfig()
@@ -629,7 +682,15 @@ func TestCleanupJob(tOuter *testing.T) {
629682
w.Write(b)
630683
}))
631684
defer ts.Close()
632-
deployer, _ := NewDeployer(&runcontext.RunContext{}, &label.DefaultLabeller{}, &latest.CloudRunDeploy{ProjectID: test.defaultProject, Region: test.region}, configName)
685+
deployer, _ := NewDeployer(
686+
&runcontext.RunContext{},
687+
&label.DefaultLabeller{},
688+
&latest.CloudRunDeploy{
689+
ProjectID: test.defaultProject,
690+
Region: test.region,
691+
},
692+
configName,
693+
defaultStatusCheckDeadline)
633694
deployer.clientOptions = append(deployer.clientOptions, option.WithEndpoint(ts.URL), option.WithoutAuthentication())
634695
deployer.useGcpOptions = false
635696
manifestListByConfig := manifest.NewManifestListByConfig()
@@ -693,7 +754,15 @@ func TestCleanupMultipleResources(tOuter *testing.T) {
693754
w.Write(b)
694755
}))
695756
defer ts.Close()
696-
deployer, _ := NewDeployer(&runcontext.RunContext{}, &label.DefaultLabeller{}, &latest.CloudRunDeploy{ProjectID: test.defaultProject, Region: test.region}, configName)
757+
deployer, _ := NewDeployer(
758+
&runcontext.RunContext{},
759+
&label.DefaultLabeller{},
760+
&latest.CloudRunDeploy{
761+
ProjectID: test.defaultProject,
762+
Region: test.region,
763+
},
764+
configName,
765+
defaultStatusCheckDeadline)
697766
deployer.clientOptions = append(deployer.clientOptions, option.WithEndpoint(ts.URL), option.WithoutAuthentication())
698767
deployer.useGcpOptions = false
699768
manifestListByConfig := manifest.NewManifestListByConfig()

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

+2-2
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ func TestPrintSummaryStatus(t *testing.T) {
8484
status: Status{ae: test.ae},
8585
sub: &runServiceResource{path: test.resource.String()},
8686
}
87-
s := NewMonitor(labeller, []option.ClientOption{})
87+
s := NewMonitor(labeller, []option.ClientOption{}, defaultStatusCheckDeadline)
8888
out := new(bytes.Buffer)
8989
testEvent.InitializeState([]latest.Pipeline{{}})
9090
c := newCounter(10)
@@ -428,7 +428,7 @@ func TestMonitorPrintStatus(t *testing.T) {
428428
testutil.Run(t, test.description, func(t *testutil.T) {
429429
testEvent.InitializeState([]latest.Pipeline{{}})
430430

431-
monitor := NewMonitor(labeller, []option.ClientOption{})
431+
monitor := NewMonitor(labeller, []option.ClientOption{}, defaultStatusCheckDeadline)
432432
out := new(bytes.Buffer)
433433
done := monitor.printStatus(test.resources, out)
434434
if done != test.done {

‎pkg/skaffold/runner/deployer.go

+1-3
Original file line numberDiff line numberDiff line change
@@ -202,7 +202,6 @@ func GetDeployer(ctx context.Context, runCtx *runcontext.RunContext, labeller *l
202202
deployers = append(deployers, deployer)
203203
}
204204
if d.CloudRunDeploy != nil {
205-
// Need to pass it in here. d.StatusCheckDeadline?
206205
deployer, err := getCloudRunDeployer(dCtx.RunContext, labeller, runCtx.DeployConfigs(), configName)
207206
if err != nil {
208207
return nil, err
@@ -337,7 +336,6 @@ func getCloudRunDeployer(runCtx *runcontext.RunContext, labeller *label.DefaultL
337336
defaultProject = runCtx.Opts.CloudRunProject
338337
projectFlag = true
339338
}
340-
// Go through each and find max of statuscheckdeadlineseconds
341339
for _, d := range deployers {
342340
if d.CloudRunDeploy != nil {
343341
crDeploy := d.CloudRunDeploy
@@ -370,7 +368,7 @@ func getCloudRunDeployer(runCtx *runcontext.RunContext, labeller *label.DefaultL
370368
}
371369

372370
// maxStatusCheckDeadlineSeconds goes through each of the Deploy Configs and
373-
// finds the max.
371+
// finds the max. If none have the field set, it uses the default.
374372
func maxStatusCheckDeadlineSeconds(deployConfigs []latest.DeployConfig) time.Duration {
375373
c := 0
376374
// set the group status check deadline to maximum of any individually specified value

‎pkg/skaffold/runner/deployer_test.go

+3-2
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import (
2020
"context"
2121
"reflect"
2222
"testing"
23+
"time"
2324

2425
"github.com/google/go-cmp/cmp/cmpopts"
2526

@@ -128,7 +129,7 @@ func TestGetDeployer(tOuter *testing.T) {
128129
},
129130
expected: deploy.NewDeployerMux(
130131
[]deploy.Deployer{
131-
t.RequireNonNilResult(cloudrun.NewDeployer(&runcontext.RunContext{}, &label.DefaultLabeller{}, &latest.CloudRunDeploy{}, "default")).(deploy.Deployer)},
132+
t.RequireNonNilResult(cloudrun.NewDeployer(&runcontext.RunContext{}, &label.DefaultLabeller{}, &latest.CloudRunDeploy{}, "default", 10*time.Minute)).(deploy.Deployer)},
132133
false),
133134
},
134135
{
@@ -274,7 +275,7 @@ func TestGetDeployer(tOuter *testing.T) {
274275
},
275276
},
276277
},
277-
expected: t.RequireNonNilResult(cloudrun.NewDeployer(&runcontext.RunContext{}, &label.DefaultLabeller{}, &latest.CloudRunDeploy{ProjectID: "TestProject", Region: "us-central1"}, "default")).(deploy.Deployer),
278+
expected: t.RequireNonNilResult(cloudrun.NewDeployer(&runcontext.RunContext{}, &label.DefaultLabeller{}, &latest.CloudRunDeploy{ProjectID: "TestProject", Region: "us-central1"}, "default", 10*time.Minute)).(deploy.Deployer),
278279
deepCheckDeployer: true,
279280
},
280281
{

0 commit comments

Comments
 (0)