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.

Sign up for GitHub

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

DecorationConfig: add pointers to Durations. #12414

Merged
merged 4 commits into from
May 3, 2019
Merged

DecorationConfig: add pointers to Durations. #12414

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a72eaf3
DecorationConfig: add pointer to Duration to apply the omitempty
droslean Apr 30, 2019
9a58b5e
fix tests for the changed DecorationConfig
droslean Apr 30, 2019
f333f20
update prowjobs v1 generated code
droslean Apr 30, 2019
3645be7
update remaining entries
droslean May 3, 2019
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
15 changes: 11 additions & 4 deletions prow/apis/prowjobs/v1/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,11 +198,11 @@ func (d *Duration) MarshalJSON() ([]byte, error) {
type DecorationConfig struct {
// Timeout is how long the pod utilities will wait
// before aborting a job with SIGINT.
Timeout Duration `json:"timeout,omitempty"`
Timeout *Duration `json:"timeout,omitempty"`
// GracePeriod is how long the pod utilities will wait
// after sending SIGINT to send SIGKILL when aborting
// a job. Only applicable if decorating the PodSpec.
GracePeriod Duration `json:"grace_period,omitempty"`
GracePeriod *Duration `json:"grace_period,omitempty"`

// UtilityImages holds pull specs for utility container
// images used to decorate a PodSpec.
Expand Down Expand Up @@ -246,10 +246,10 @@ func (d *DecorationConfig) ApplyDefault(def *DecorationConfig) *DecorationConfig
merged.UtilityImages = merged.UtilityImages.ApplyDefault(def.UtilityImages)
merged.GCSConfiguration = merged.GCSConfiguration.ApplyDefault(def.GCSConfiguration)

if merged.Timeout.Duration == 0 {
if merged.Timeout == nil {
merged.Timeout = def.Timeout
}
if merged.GracePeriod.Duration == 0 {
if merged.GracePeriod == nil {
merged.GracePeriod = def.GracePeriod
}
if merged.GCSCredentialsSecret == "" {
Expand Down Expand Up @@ -305,6 +305,13 @@ func (d *DecorationConfig) Validate() error {
return nil
}

func (d *Duration) Get() time.Duration {
if d == nil {
return 0
}
return d.Duration
}

// UtilityImages holds pull specs for the utility images
// to be used for a job
type UtilityImages struct {
Expand Down
8 changes: 4 additions & 4 deletions prow/apis/prowjobs/v1/types_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ func TestDecorationDefaulting(t *testing.T) {
{
name: "timeout provided",
provided: &DecorationConfig{
Timeout: Duration{Duration: 10 * time.Minute},
Timeout: &Duration{Duration: 10 * time.Minute},
},
expected: func(orig, def *DecorationConfig) *DecorationConfig {
def.Timeout = orig.Timeout
Expand All @@ -52,7 +52,7 @@ func TestDecorationDefaulting(t *testing.T) {
{
name: "grace period provided",
provided: &DecorationConfig{
GracePeriod: Duration{Duration: 10 * time.Hour},
GracePeriod: &Duration{Duration: 10 * time.Hour},
},
expected: func(orig, def *DecorationConfig) *DecorationConfig {
def.GracePeriod = orig.GracePeriod
Expand Down Expand Up @@ -163,8 +163,8 @@ func TestDecorationDefaulting(t *testing.T) {
tc := testCase
t.Run(tc.name, func(t *testing.T) {
defaults := &DecorationConfig{
Timeout: Duration{Duration: 1 * time.Minute},
GracePeriod: Duration{Duration: 10 * time.Second},
Timeout: &Duration{Duration: 1 * time.Minute},
GracePeriod: &Duration{Duration: 10 * time.Second},
UtilityImages: &UtilityImages{
CloneRefs: "clonerefs",
InitUpload: "initupload",
Expand Down
12 changes: 10 additions & 2 deletions prow/apis/prowjobs/v1/zz_generated.deepcopy.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions prow/cmd/build/controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -686,7 +686,7 @@ func decorateSteps(steps []coreapi.Container, dc prowjobv1.DecorationConfig, too
previousMarker = entries[i-1].MarkerFile
}
// TODO(fejta): consider refactoring entrypoint to accept --expire=time.Now.Add(dc.Timeout) so we timeout each step correctly (assuming a good clock)
opt, err := decorate.InjectEntrypoint(&steps[i], dc.Timeout.Duration, dc.GracePeriod.Duration, steps[i].Name, previousMarker, alwaysPass, logMount, toolsMount)
opt, err := decorate.InjectEntrypoint(&steps[i], dc.Timeout.Get(), dc.GracePeriod.Get(), steps[i].Name, previousMarker, alwaysPass, logMount, toolsMount)
if err != nil {
return nil, fmt.Errorf("inject entrypoint into %s: %v", steps[i].Name, err)
}
Expand Down Expand Up @@ -723,7 +723,7 @@ func determineTimeout(spec *buildv1alpha1.BuildSpec, dc *prowjobv1.DecorationCon
switch {
case spec.Timeout != nil:
return spec.Timeout.Duration
case dc != nil && dc.Timeout.Duration > 0:
case dc != nil && dc.Timeout.Get() > 0:
return dc.Timeout.Duration
default:
return defaultTimeout
Expand Down
17 changes: 12 additions & 5 deletions prow/cmd/build/controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1469,6 +1469,8 @@ func TestMakeBuild(t *testing.T) {
pj.Spec.ExtraRefs = []prowjobv1.Refs{{Org: "bonus"}}
pj.Spec.DecorationConfig = &prowjobv1.DecorationConfig{
UtilityImages: &prowjobv1.UtilityImages{},
Timeout: &prowjobv1.Duration{Duration: 0},
GracePeriod: &prowjobv1.Duration{Duration: 0},
}
return pj
},
Expand All @@ -1479,6 +1481,8 @@ func TestMakeBuild(t *testing.T) {
pj.Spec.ExtraRefs = []prowjobv1.Refs{{Org: "bonus"}}
pj.Spec.DecorationConfig = &prowjobv1.DecorationConfig{
UtilityImages: &prowjobv1.UtilityImages{},
Timeout: &prowjobv1.Duration{Duration: 0},
GracePeriod: &prowjobv1.Duration{Duration: 0},
}
pj.Spec.BuildSpec.Source = &buildv1alpha1.SourceSpec{}
return pj
Expand Down Expand Up @@ -1542,8 +1546,8 @@ func TestMakeBuild(t *testing.T) {

func TestDecorateSteps(t *testing.T) {
var dc prowjobv1.DecorationConfig
dc.Timeout = prowjobv1.Duration{Duration: 10 * time.Minute}
dc.GracePeriod = prowjobv1.Duration{Duration: 5 * time.Minute}
dc.Timeout = &prowjobv1.Duration{Duration: 10 * time.Minute}
dc.GracePeriod = &prowjobv1.Duration{Duration: 5 * time.Minute}
_, tm := tools()
tm.Name += "not-static"
tm.MountPath += "fancy"
Expand All @@ -1570,15 +1574,15 @@ func TestDecorateSteps(t *testing.T) {
},
}
expected[1].Name = "step-1"
o1, err := decorate.InjectEntrypoint(&expected[0], dc.Timeout.Duration, dc.GracePeriod.Duration, expected[0].Name, "", true, logMount, tm)
o1, err := decorate.InjectEntrypoint(&expected[0], dc.Timeout.Get(), dc.GracePeriod.Get(), expected[0].Name, "", true, logMount, tm)
if err != nil {
t.Fatalf("inject expected 0: %v", err)
}
o2, err := decorate.InjectEntrypoint(&expected[1], dc.Timeout.Duration, dc.GracePeriod.Duration, expected[1].Name, o1.MarkerFile, true, logMount, tm)
o2, err := decorate.InjectEntrypoint(&expected[1], dc.Timeout.Get(), dc.GracePeriod.Get(), expected[1].Name, o1.MarkerFile, true, logMount, tm)
if err != nil {
t.Fatalf("inject expected 1: %v", err)
}
o3, err := decorate.InjectEntrypoint(&expected[2], dc.Timeout.Duration, dc.GracePeriod.Duration, expected[2].Name, o2.MarkerFile, true, logMount, tm)
o3, err := decorate.InjectEntrypoint(&expected[2], dc.Timeout.Get(), dc.GracePeriod.Get(), expected[2].Name, o2.MarkerFile, true, logMount, tm)
if err != nil {
t.Fatalf("inject expected 2: %v", err)
}
Expand Down Expand Up @@ -1739,6 +1743,9 @@ func TestInjectTimeout(t *testing.T) {

dc := prowjobv1.DecorationConfig{
UtilityImages: &prowjobv1.UtilityImages{},
Timeout: &prowjobv1.Duration{
Duration: defaultTimeout,
},
}
if tc.decoratedTimeout != nil {
dc.Timeout.Duration = *tc.decoratedTimeout
Expand Down
8 changes: 4 additions & 4 deletions prow/config/config_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -323,8 +323,8 @@ periodics:
- "test"
- "./..."`,
expected: &prowapi.DecorationConfig{
Timeout: prowapi.Duration{Duration: 2 * time.Hour},
GracePeriod: prowapi.Duration{Duration: 15 * time.Second},
Timeout: &prowapi.Duration{Duration: 2 * time.Hour},
GracePeriod: &prowapi.Duration{Duration: 15 * time.Second},
UtilityImages: &prowapi.UtilityImages{
CloneRefs: "clonerefs:default",
InitUpload: "initupload:default",
Expand Down Expand Up @@ -382,8 +382,8 @@ periodics:
- "test"
- "./..."`,
expected: &prowapi.DecorationConfig{
Timeout: prowapi.Duration{Duration: 1 * time.Nanosecond},
GracePeriod: prowapi.Duration{Duration: 1 * time.Nanosecond},
Timeout: &prowapi.Duration{Duration: 1 * time.Nanosecond},
GracePeriod: &prowapi.Duration{Duration: 1 * time.Nanosecond},
UtilityImages: &prowapi.UtilityImages{
CloneRefs: "clonerefs:explicit",
InitUpload: "initupload:explicit",
Expand Down
2 changes: 1 addition & 1 deletion prow/pod-utils/decorate/podspec.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -535,7 +535,7 @@ func decorate(spec *coreapi.PodSpec, pj *prowapi.ProwJob, rawEnv map[string]stri
previous = ""
exitZero = false
)
wrapperOptions, err := InjectEntrypoint(&spec.Containers[0], pj.Spec.DecorationConfig.Timeout.Duration, pj.Spec.DecorationConfig.GracePeriod.Duration, prefix, previous, exitZero, logMount, toolsMount)
wrapperOptions, err := InjectEntrypoint(&spec.Containers[0], pj.Spec.DecorationConfig.Timeout.Get(), pj.Spec.DecorationConfig.GracePeriod.Get(), prefix, previous, exitZero, logMount, toolsMount)
if err != nil {
return fmt.Errorf("wrap container: %v", err)
}
Expand Down
24 changes: 12 additions & 12 deletions prow/pod-utils/decorate/podspec_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -456,8 +456,8 @@ func TestProwJobToPod(t *testing.T) {
Type: prowapi.PresubmitJob,
Job: "job-name",
DecorationConfig: &prowapi.DecorationConfig{
Timeout: prowapi.Duration{Duration: 120 * time.Minute},
GracePeriod: prowapi.Duration{Duration: 10 * time.Second},
Timeout: &prowapi.Duration{Duration: 120 * time.Minute},
GracePeriod: &prowapi.Duration{Duration: 10 * time.Second},
UtilityImages: &prowapi.UtilityImages{
CloneRefs: "clonerefs:tag",
InitUpload: "initupload:tag",
Expand Down Expand Up @@ -677,8 +677,8 @@ func TestProwJobToPod(t *testing.T) {
Type: prowapi.PresubmitJob,
Job: "job-name",
DecorationConfig: &prowapi.DecorationConfig{
Timeout: prowapi.Duration{Duration: 120 * time.Minute},
GracePeriod: prowapi.Duration{Duration: 10 * time.Second},
Timeout: &prowapi.Duration{Duration: 120 * time.Minute},
GracePeriod: &prowapi.Duration{Duration: 10 * time.Second},
UtilityImages: &prowapi.UtilityImages{
CloneRefs: "clonerefs:tag",
InitUpload: "initupload:tag",
Expand Down Expand Up @@ -898,8 +898,8 @@ func TestProwJobToPod(t *testing.T) {
Type: prowapi.PresubmitJob,
Job: "job-name",
DecorationConfig: &prowapi.DecorationConfig{
Timeout: prowapi.Duration{Duration: 120 * time.Minute},
GracePeriod: prowapi.Duration{Duration: 10 * time.Second},
Timeout: &prowapi.Duration{Duration: 120 * time.Minute},
GracePeriod: &prowapi.Duration{Duration: 10 * time.Second},
UtilityImages: &prowapi.UtilityImages{
CloneRefs: "clonerefs:tag",
InitUpload: "initupload:tag",
Expand Down Expand Up @@ -1145,8 +1145,8 @@ func TestProwJobToPod(t *testing.T) {
Type: prowapi.PresubmitJob,
Job: "job-name",
DecorationConfig: &prowapi.DecorationConfig{
Timeout: prowapi.Duration{Duration: 120 * time.Minute},
GracePeriod: prowapi.Duration{Duration: 10 * time.Second},
Timeout: &prowapi.Duration{Duration: 120 * time.Minute},
GracePeriod: &prowapi.Duration{Duration: 10 * time.Second},
UtilityImages: &prowapi.UtilityImages{
CloneRefs: "clonerefs:tag",
InitUpload: "initupload:tag",
Expand Down Expand Up @@ -1391,8 +1391,8 @@ func TestProwJobToPod(t *testing.T) {
Type: prowapi.PeriodicJob,
Job: "job-name",
DecorationConfig: &prowapi.DecorationConfig{
Timeout: prowapi.Duration{Duration: 120 * time.Minute},
GracePeriod: prowapi.Duration{Duration: 10 * time.Second},
Timeout: &prowapi.Duration{Duration: 120 * time.Minute},
GracePeriod: &prowapi.Duration{Duration: 10 * time.Second},
UtilityImages: &prowapi.UtilityImages{
CloneRefs: "clonerefs:tag",
InitUpload: "initupload:tag",
Expand Down Expand Up @@ -1554,8 +1554,8 @@ func TestProwJobToPod(t *testing.T) {
Type: prowapi.PresubmitJob,
Job: "job-name",
DecorationConfig: &prowapi.DecorationConfig{
Timeout: prowapi.Duration{Duration: 120 * time.Minute},
GracePeriod: prowapi.Duration{Duration: 10 * time.Second},
Timeout: &prowapi.Duration{Duration: 120 * time.Minute},
GracePeriod: &prowapi.Duration{Duration: 10 * time.Second},
UtilityImages: &prowapi.UtilityImages{
CloneRefs: "clonerefs:tag",
InitUpload: "initupload:tag",
Expand Down