Skip to content

fix: allow unseting GracePeriod when default value is provided #4015

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

Merged
mergify merged 1 commit into from
Sep 23, 2022
Merged

fix: allow unseting GracePeriod when default value is provided #4015

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 2 additions & 2 deletions internal/pkg/deploy/cloudformation/stack/backend_svc_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,7 @@ Outputs:
},
HTTPHealthCheck: template.HTTPHealthCheckOpts{
HealthCheckPath: manifest.DefaultHealthCheckPath,
GracePeriod: aws.Int64(manifest.DefaultHealthCheckGracePeriod),
GracePeriod: manifest.DefaultHealthCheckGracePeriod,
},
DeregistrationDelay: aws.Int64(60), // defaults to 60
CustomResources: map[string]template.S3ObjectLocation{
Expand Down Expand Up @@ -447,7 +447,7 @@ Outputs:
UnhealthyThreshold: aws.Int64(63),
Timeout: aws.Int64(62),
Interval: aws.Int64(61),
GracePeriod: aws.Int64(60),
GracePeriod: 60,
},
HostedZoneAliases: make(template.AliasesForHostedZone),
DeregistrationDelay: aws.Int64(59),
Expand Down
2 changes: 1 addition & 1 deletion internal/pkg/deploy/cloudformation/stack/lb_web_svc_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@ Outputs:
WorkloadType: manifest.LoadBalancedWebServiceType,
HTTPHealthCheck: template.HTTPHealthCheckOpts{
HealthCheckPath: "/",
GracePeriod: aws.Int64(60),
GracePeriod: 60,
},
HostedZoneAliases: template.AliasesForHostedZone{
"mockHostedZone": []string{"mockAlias"},
Expand Down
4 changes: 2 additions & 2 deletions internal/pkg/deploy/cloudformation/stack/transformers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -321,7 +321,7 @@ func convertHTTPHealthCheck(hc *manifest.HealthCheckArgsOrString) template.HTTPH
HealthCheckPath: manifest.DefaultHealthCheckPath,
HealthyThreshold: hc.HealthCheckArgs.HealthyThreshold,
UnhealthyThreshold: hc.HealthCheckArgs.UnhealthyThreshold,
GracePeriod: aws.Int64(manifest.DefaultHealthCheckGracePeriod),
GracePeriod: manifest.DefaultHealthCheckGracePeriod,
}
if hc.HealthCheckArgs.Path != nil {
opts.HealthCheckPath = *hc.HealthCheckArgs.Path
Expand All @@ -341,7 +341,7 @@ func convertHTTPHealthCheck(hc *manifest.HealthCheckArgsOrString) template.HTTPH
opts.Timeout = aws.Int64(int64(hc.HealthCheckArgs.Timeout.Seconds()))
}
if hc.HealthCheckArgs.GracePeriod != nil {
opts.GracePeriod = aws.Int64(int64(hc.HealthCheckArgs.GracePeriod.Seconds()))
opts.GracePeriod = int64(hc.HealthCheckArgs.GracePeriod.Seconds())
}
return opts
}
Expand Down
16 changes: 8 additions & 8 deletions internal/pkg/deploy/cloudformation/stack/transformers_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -660,7 +660,7 @@ func Test_convertHTTPHealthCheck(t *testing.T) {

wantedOpts: template.HTTPHealthCheckOpts{
HealthCheckPath: "/",
GracePeriod: aws.Int64(60),
GracePeriod: 60,
},
},
"just HealthyThreshold": {
Expand All @@ -675,7 +675,7 @@ func Test_convertHTTPHealthCheck(t *testing.T) {
wantedOpts: template.HTTPHealthCheckOpts{
HealthCheckPath: "/",
HealthyThreshold: aws.Int64(5),
GracePeriod: aws.Int64(60),
GracePeriod: 60,
},
},
"just UnhealthyThreshold": {
Expand All @@ -690,7 +690,7 @@ func Test_convertHTTPHealthCheck(t *testing.T) {
wantedOpts: template.HTTPHealthCheckOpts{
HealthCheckPath: "/",
UnhealthyThreshold: aws.Int64(5),
GracePeriod: aws.Int64(60),
GracePeriod: 60,
},
},
"just Interval": {
Expand All @@ -705,7 +705,7 @@ func Test_convertHTTPHealthCheck(t *testing.T) {
wantedOpts: template.HTTPHealthCheckOpts{
HealthCheckPath: "/",
Interval: aws.Int64(15),
GracePeriod: aws.Int64(60),
GracePeriod: 60,
},
},
"just Timeout": {
Expand All @@ -720,7 +720,7 @@ func Test_convertHTTPHealthCheck(t *testing.T) {
wantedOpts: template.HTTPHealthCheckOpts{
HealthCheckPath: "/",
Timeout: aws.Int64(15),
GracePeriod: aws.Int64(60),
GracePeriod: 60,
},
},
"just SuccessCodes": {
Expand All @@ -735,7 +735,7 @@ func Test_convertHTTPHealthCheck(t *testing.T) {
wantedOpts: template.HTTPHealthCheckOpts{
HealthCheckPath: "/",
SuccessCodes: "200,301",
GracePeriod: aws.Int64(60),
GracePeriod: 60,
},
},
"just Port": {
Expand All @@ -745,7 +745,7 @@ func Test_convertHTTPHealthCheck(t *testing.T) {
wantedOpts: template.HTTPHealthCheckOpts{
HealthCheckPath: "/",
Port: "8000",
GracePeriod: aws.Int64(60),
GracePeriod: 60,
},
},
"all values changed in manifest": {
Expand All @@ -766,7 +766,7 @@ func Test_convertHTTPHealthCheck(t *testing.T) {
UnhealthyThreshold: aws.Int64(3),
Interval: aws.Int64(60),
Timeout: aws.Int64(60),
GracePeriod: aws.Int64(15),
GracePeriod: 15,
},
},
}
Expand Down
2 changes: 2 additions & 0 deletions internal/pkg/template/templates/workloads/services/backend/cf.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,9 @@ Resources:
{{- "\n"}}{{ include "service-base-properties" . | indent 6 }}
ServiceRegistries: !If [ExposePort, [{RegistryArn: !GetAtt DiscoveryService.Arn, Port: !Ref ContainerPort}], !Ref "AWS::NoValue"]
{{- if .ALBEnabled}}
{{- if .HTTPHealthCheck.GracePeriod }}
HealthCheckGracePeriodSeconds: {{.HTTPHealthCheck.GracePeriod}}
{{- end }}
LoadBalancers:
- ContainerName: !Ref TargetContainer
ContainerPort: !Ref TargetPort
Expand Down
2 changes: 2 additions & 0 deletions internal/pkg/template/templates/workloads/services/lb-web/cf.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,9 @@ Resources:
Properties:
{{include "service-base-properties" . | indent 6}}
# This may need to be adjusted if the container takes a while to start up
{{- if .HTTPHealthCheck.GracePeriod }}
HealthCheckGracePeriodSeconds: {{.HTTPHealthCheck.GracePeriod}}
{{- end }}
LoadBalancers:
{{- if .ALBEnabled}}
- ContainerName: !Ref TargetContainer
Expand Down
7 changes: 5 additions & 2 deletions internal/pkg/template/workload.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,15 +216,18 @@ func (tg HTTPTargetContainer) IsHTTPS() bool {

// HTTPHealthCheckOpts holds configuration that's needed for HTTP Health Check.
type HTTPHealthCheckOpts struct {
HealthCheckPath string
// Fields with defaults always set.
HealthCheckPath string
GracePeriod int64

// Optional.
Port string
SuccessCodes string
HealthyThreshold *int64
UnhealthyThreshold *int64
Interval *int64
Timeout *int64
DeregistrationDelay *int64
GracePeriod *int64
}

// A Secret represents an SSM or SecretsManager secret that can be rendered in CloudFormation.
Expand Down