From 20de04d6c304122dc470c8de67172d5a4ccf1956 Mon Sep 17 00:00:00 2001 From: Shiming Zhang Date: Wed, 7 Jul 2021 10:44:10 +0800 Subject: [PATCH 1/5] Update API documents --- staging/src/k8s.io/api/core/v1/types.go | 1 + 1 file changed, 1 insertion(+) diff --git a/staging/src/k8s.io/api/core/v1/types.go b/staging/src/k8s.io/api/core/v1/types.go index 3f6a6211b215..b251e5f341c6 100644 --- a/staging/src/k8s.io/api/core/v1/types.go +++ b/staging/src/k8s.io/api/core/v1/types.go @@ -2133,6 +2133,7 @@ type Probe struct { // Value must be non-negative integer. The value zero indicates stop immediately via // the kill signal (no opportunity to shut down). // This is an alpha field and requires enabling ProbeTerminationGracePeriod feature gate. + // Minimum value is 1. spec.terminationGracePeriodSeconds is used if unset. // +optional TerminationGracePeriodSeconds *int64 `json:"terminationGracePeriodSeconds,omitempty" protobuf:"varint,7,opt,name=terminationGracePeriodSeconds"` } From 1ff5ae2cb5f1e7351c9c7161bacc11087e561c42 Mon Sep 17 00:00:00 2001 From: Shiming Zhang Date: Wed, 7 Jul 2021 10:44:25 +0800 Subject: [PATCH 2/5] Regenerate --- api/openapi-spec/swagger.json | 2 +- staging/src/k8s.io/api/core/v1/generated.proto | 1 + staging/src/k8s.io/api/core/v1/types_swagger_doc_generated.go | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/api/openapi-spec/swagger.json b/api/openapi-spec/swagger.json index a255566ecbb9..774d23a6727a 100644 --- a/api/openapi-spec/swagger.json +++ b/api/openapi-spec/swagger.json @@ -8189,7 +8189,7 @@ "description": "TCPSocket specifies an action involving a TCP port. TCP hooks not yet supported" }, "terminationGracePeriodSeconds": { - "description": "Optional duration in seconds the pod needs to terminate gracefully upon probe failure. The grace period is the duration in seconds after the processes running in the pod are sent a termination signal and the time when the processes are forcibly halted with a kill signal. Set this value longer than the expected cleanup time for your process. If this value is nil, the pod's terminationGracePeriodSeconds will be used. Otherwise, this value overrides the value provided by the pod spec. Value must be non-negative integer. The value zero indicates stop immediately via the kill signal (no opportunity to shut down). This is an alpha field and requires enabling ProbeTerminationGracePeriod feature gate.", + "description": "Optional duration in seconds the pod needs to terminate gracefully upon probe failure. The grace period is the duration in seconds after the processes running in the pod are sent a termination signal and the time when the processes are forcibly halted with a kill signal. Set this value longer than the expected cleanup time for your process. If this value is nil, the pod's terminationGracePeriodSeconds will be used. Otherwise, this value overrides the value provided by the pod spec. Value must be non-negative integer. The value zero indicates stop immediately via the kill signal (no opportunity to shut down). This is an alpha field and requires enabling ProbeTerminationGracePeriod feature gate. Minimum value is 1. spec.terminationGracePeriodSeconds is used if unset.", "format": "int64", "type": "integer" }, diff --git a/staging/src/k8s.io/api/core/v1/generated.proto b/staging/src/k8s.io/api/core/v1/generated.proto index c2f66dd2196c..53884f20f860 100644 --- a/staging/src/k8s.io/api/core/v1/generated.proto +++ b/staging/src/k8s.io/api/core/v1/generated.proto @@ -3890,6 +3890,7 @@ message Probe { // Value must be non-negative integer. The value zero indicates stop immediately via // the kill signal (no opportunity to shut down). // This is an alpha field and requires enabling ProbeTerminationGracePeriod feature gate. + // Minimum value is 1. spec.terminationGracePeriodSeconds is used if unset. // +optional optional int64 terminationGracePeriodSeconds = 7; } diff --git a/staging/src/k8s.io/api/core/v1/types_swagger_doc_generated.go b/staging/src/k8s.io/api/core/v1/types_swagger_doc_generated.go index f29297985f8d..1e0dac6a3960 100644 --- a/staging/src/k8s.io/api/core/v1/types_swagger_doc_generated.go +++ b/staging/src/k8s.io/api/core/v1/types_swagger_doc_generated.go @@ -1774,7 +1774,7 @@ var map_Probe = map[string]string{ "periodSeconds": "How often (in seconds) to perform the probe. Default to 10 seconds. Minimum value is 1.", "successThreshold": "Minimum consecutive successes for the probe to be considered successful after having failed. Defaults to 1. Must be 1 for liveness and startup. Minimum value is 1.", "failureThreshold": "Minimum consecutive failures for the probe to be considered failed after having succeeded. Defaults to 3. Minimum value is 1.", - "terminationGracePeriodSeconds": "Optional duration in seconds the pod needs to terminate gracefully upon probe failure. The grace period is the duration in seconds after the processes running in the pod are sent a termination signal and the time when the processes are forcibly halted with a kill signal. Set this value longer than the expected cleanup time for your process. If this value is nil, the pod's terminationGracePeriodSeconds will be used. Otherwise, this value overrides the value provided by the pod spec. Value must be non-negative integer. The value zero indicates stop immediately via the kill signal (no opportunity to shut down). This is an alpha field and requires enabling ProbeTerminationGracePeriod feature gate.", + "terminationGracePeriodSeconds": "Optional duration in seconds the pod needs to terminate gracefully upon probe failure. The grace period is the duration in seconds after the processes running in the pod are sent a termination signal and the time when the processes are forcibly halted with a kill signal. Set this value longer than the expected cleanup time for your process. If this value is nil, the pod's terminationGracePeriodSeconds will be used. Otherwise, this value overrides the value provided by the pod spec. Value must be non-negative integer. The value zero indicates stop immediately via the kill signal (no opportunity to shut down). This is an alpha field and requires enabling ProbeTerminationGracePeriod feature gate. Minimum value is 1. spec.terminationGracePeriodSeconds is used if unset.", } func (Probe) SwaggerDoc() map[string]string { From e378600c90dbeb335cfbead90aaa5abc170ed869 Mon Sep 17 00:00:00 2001 From: Shiming Zhang Date: Wed, 7 Jul 2021 10:47:31 +0800 Subject: [PATCH 3/5] Add validation for Prober TerminationGracePeriodSeconds --- pkg/apis/core/validation/validation.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pkg/apis/core/validation/validation.go b/pkg/apis/core/validation/validation.go index 2b9955156e8c..285e47e90d43 100644 --- a/pkg/apis/core/validation/validation.go +++ b/pkg/apis/core/validation/validation.go @@ -2637,6 +2637,9 @@ func validateProbe(probe *core.Probe, fldPath *field.Path) field.ErrorList { allErrs = append(allErrs, ValidateNonnegativeField(int64(probe.PeriodSeconds), fldPath.Child("periodSeconds"))...) allErrs = append(allErrs, ValidateNonnegativeField(int64(probe.SuccessThreshold), fldPath.Child("successThreshold"))...) allErrs = append(allErrs, ValidateNonnegativeField(int64(probe.FailureThreshold), fldPath.Child("failureThreshold"))...) + if probe.TerminationGracePeriodSeconds != nil && *probe.TerminationGracePeriodSeconds <= 0 { + allErrs = append(allErrs, field.Invalid(fldPath.Child("terminationGracePeriodSeconds"), *probe.TerminationGracePeriodSeconds, "must be greater than 0")) + } return allErrs } From d8fe255f41368b6726a0b8c1e405a225f9a2a5dd Mon Sep 17 00:00:00 2001 From: Shiming Zhang Date: Wed, 7 Jul 2021 11:30:16 +0800 Subject: [PATCH 4/5] Add test for validateProbe --- pkg/apis/core/validation/validation_test.go | 125 ++++++++++++++++++++ 1 file changed, 125 insertions(+) diff --git a/pkg/apis/core/validation/validation_test.go b/pkg/apis/core/validation/validation_test.go index 99cb717afdeb..c4b0c58b419b 100644 --- a/pkg/apis/core/validation/validation_test.go +++ b/pkg/apis/core/validation/validation_test.go @@ -6043,6 +6043,131 @@ func TestValidateProbe(t *testing.T) { } } +func Test_validateProbe(t *testing.T) { + fldPath := field.NewPath("test") + type args struct { + probe *core.Probe + fldPath *field.Path + } + tests := []struct { + name string + args args + want field.ErrorList + }{ + { + args: args{ + probe: &core.Probe{}, + fldPath: fldPath, + }, + want: field.ErrorList{field.Required(fldPath, "must specify a handler type")}, + }, + { + args: args{ + probe: &core.Probe{ + Handler: core.Handler{Exec: &core.ExecAction{Command: []string{"echo"}}}, + }, + fldPath: fldPath, + }, + want: field.ErrorList{}, + }, + { + args: args{ + probe: &core.Probe{ + Handler: core.Handler{Exec: &core.ExecAction{Command: []string{"echo"}}}, + InitialDelaySeconds: -1, + }, + fldPath: fldPath, + }, + want: field.ErrorList{field.Invalid(fldPath.Child("initialDelaySeconds"), -1, "must be greater than or equal to 0")}, + }, + { + args: args{ + probe: &core.Probe{ + Handler: core.Handler{Exec: &core.ExecAction{Command: []string{"echo"}}}, + TimeoutSeconds: -1, + }, + fldPath: fldPath, + }, + want: field.ErrorList{field.Invalid(fldPath.Child("timeoutSeconds"), -1, "must be greater than or equal to 0")}, + }, + { + args: args{ + probe: &core.Probe{ + Handler: core.Handler{Exec: &core.ExecAction{Command: []string{"echo"}}}, + PeriodSeconds: -1, + }, + fldPath: fldPath, + }, + want: field.ErrorList{field.Invalid(fldPath.Child("periodSeconds"), -1, "must be greater than or equal to 0")}, + }, + { + args: args{ + probe: &core.Probe{ + Handler: core.Handler{Exec: &core.ExecAction{Command: []string{"echo"}}}, + SuccessThreshold: -1, + }, + fldPath: fldPath, + }, + want: field.ErrorList{field.Invalid(fldPath.Child("successThreshold"), -1, "must be greater than or equal to 0")}, + }, + { + args: args{ + probe: &core.Probe{ + Handler: core.Handler{Exec: &core.ExecAction{Command: []string{"echo"}}}, + FailureThreshold: -1, + }, + fldPath: fldPath, + }, + want: field.ErrorList{field.Invalid(fldPath.Child("failureThreshold"), -1, "must be greater than or equal to 0")}, + }, + { + args: args{ + probe: &core.Probe{ + Handler: core.Handler{Exec: &core.ExecAction{Command: []string{"echo"}}}, + TerminationGracePeriodSeconds: utilpointer.Int64(-1), + }, + fldPath: fldPath, + }, + want: field.ErrorList{field.Invalid(fldPath.Child("terminationGracePeriodSeconds"), -1, "must be greater than 0")}, + }, + { + args: args{ + probe: &core.Probe{ + Handler: core.Handler{Exec: &core.ExecAction{Command: []string{"echo"}}}, + TerminationGracePeriodSeconds: utilpointer.Int64(0), + }, + fldPath: fldPath, + }, + want: field.ErrorList{field.Invalid(fldPath.Child("terminationGracePeriodSeconds"), 0, "must be greater than 0")}, + }, + { + args: args{ + probe: &core.Probe{ + Handler: core.Handler{Exec: &core.ExecAction{Command: []string{"echo"}}}, + TerminationGracePeriodSeconds: utilpointer.Int64(1), + }, + fldPath: fldPath, + }, + want: field.ErrorList{}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := validateProbe(tt.args.probe, tt.args.fldPath) + if len(got) != len(tt.want) { + t.Errorf("validateProbe() = %v, want %v", got, tt.want) + return + } + for i := range got { + if got[i].Type != tt.want[i].Type || + got[i].Field != tt.want[i].Field { + t.Errorf("validateProbe()[%d] = %v, want %v", i, got[i], tt.want[i]) + } + } + }) + } +} + func TestValidateHandler(t *testing.T) { successCases := []core.Handler{ {Exec: &core.ExecAction{Command: []string{"echo"}}}, From 513bd93f76ddd4b35b305ee1bf40302c3f0a531c Mon Sep 17 00:00:00 2001 From: Shiming Zhang Date: Wed, 7 Jul 2021 09:49:19 +0800 Subject: [PATCH 5/5] update test for feature gate --- pkg/apis/core/validation/validation_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkg/apis/core/validation/validation_test.go b/pkg/apis/core/validation/validation_test.go index c4b0c58b419b..592724994a56 100644 --- a/pkg/apis/core/validation/validation_test.go +++ b/pkg/apis/core/validation/validation_test.go @@ -6044,6 +6044,8 @@ func TestValidateProbe(t *testing.T) { } func Test_validateProbe(t *testing.T) { + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.ProbeTerminationGracePeriod, true)() + fldPath := field.NewPath("test") type args struct { probe *core.Probe