Skip to content

Commit

Permalink
Support gRPC probe (#14134)
Browse files Browse the repository at this point in the history
* Add gRPC probe

* Modify unit test

* Modify unit test

* Set default to grpcprobe's service field

* Use knative pkg for ptr operation

* Use config's KubeMajor & KubeMinor instead of k8s native version pkg

* Wrap error in GRPCProbe

* Add comment to explain why use dialer_others.go

* Run update scripts

* Add probe test

* Add test in readiness/probe_test.go

* update deps

* Ignore readinessProbe when it's gRPC

* Fix test to use empemeral port

* Resolve govet lint error

* Use errors.Is to compare

* Also use ephemeral port for handler test

* drop unneccesary else block
  • Loading branch information
seongpyoHong authored Sep 21, 2023
1 parent 5b1eab3 commit d02702e
Show file tree
Hide file tree
Showing 17 changed files with 421 additions and 7 deletions.
26 changes: 26 additions & 0 deletions config/core/300-resources/configuration.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,19 @@ spec:
description: Minimum consecutive failures for the probe to be considered failed after having succeeded. Defaults to 3. Minimum value is 1.
type: integer
format: int32
grpc:
description: GRPC specifies an action involving a GRPC port. This is a beta field and requires enabling GRPCContainerProbe feature gate.
type: object
required:
- port
properties:
port:
description: Port number of the gRPC service. Number must be in the range 1 to 65535.
type: integer
format: int32
service:
description: "Service is the name of the service to place in the gRPC HealthCheckRequest (see https://github.com/grpc/grpc/blob/master/doc/health-checking.md). \n If this is not specified, the default behavior is defined by gRPC."
type: string
httpGet:
description: HTTPGet specifies the http request to perform.
type: object
Expand Down Expand Up @@ -355,6 +368,19 @@ spec:
description: Minimum consecutive failures for the probe to be considered failed after having succeeded. Defaults to 3. Minimum value is 1.
type: integer
format: int32
grpc:
description: GRPC specifies an action involving a GRPC port. This is a beta field and requires enabling GRPCContainerProbe feature gate.
type: object
required:
- port
properties:
port:
description: Port number of the gRPC service. Number must be in the range 1 to 65535.
type: integer
format: int32
service:
description: "Service is the name of the service to place in the gRPC HealthCheckRequest (see https://github.com/grpc/grpc/blob/master/doc/health-checking.md). \n If this is not specified, the default behavior is defined by gRPC."
type: string
httpGet:
description: HTTPGet specifies the http request to perform.
type: object
Expand Down
26 changes: 26 additions & 0 deletions config/core/300-resources/revision.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,19 @@ spec:
description: Minimum consecutive failures for the probe to be considered failed after having succeeded. Defaults to 3. Minimum value is 1.
type: integer
format: int32
grpc:
description: GRPC specifies an action involving a GRPC port. This is a beta field and requires enabling GRPCContainerProbe feature gate.
type: object
required:
- port
properties:
port:
description: Port number of the gRPC service. Number must be in the range 1 to 65535.
type: integer
format: int32
service:
description: "Service is the name of the service to place in the gRPC HealthCheckRequest (see https://github.com/grpc/grpc/blob/master/doc/health-checking.md). \n If this is not specified, the default behavior is defined by gRPC."
type: string
httpGet:
description: HTTPGet specifies the http request to perform.
type: object
Expand Down Expand Up @@ -334,6 +347,19 @@ spec:
description: Minimum consecutive failures for the probe to be considered failed after having succeeded. Defaults to 3. Minimum value is 1.
type: integer
format: int32
grpc:
description: GRPC specifies an action involving a GRPC port. This is a beta field and requires enabling GRPCContainerProbe feature gate.
type: object
required:
- port
properties:
port:
description: Port number of the gRPC service. Number must be in the range 1 to 65535.
type: integer
format: int32
service:
description: "Service is the name of the service to place in the gRPC HealthCheckRequest (see https://github.com/grpc/grpc/blob/master/doc/health-checking.md). \n If this is not specified, the default behavior is defined by gRPC."
type: string
httpGet:
description: HTTPGet specifies the http request to perform.
type: object
Expand Down
26 changes: 26 additions & 0 deletions config/core/300-resources/service.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,19 @@ spec:
description: Minimum consecutive failures for the probe to be considered failed after having succeeded. Defaults to 3. Minimum value is 1.
type: integer
format: int32
grpc:
description: GRPC specifies an action involving a GRPC port. This is a beta field and requires enabling GRPCContainerProbe feature gate.
type: object
required:
- port
properties:
port:
description: Port number of the gRPC service. Number must be in the range 1 to 65535.
type: integer
format: int32
service:
description: "Service is the name of the service to place in the gRPC HealthCheckRequest (see https://github.com/grpc/grpc/blob/master/doc/health-checking.md). \n If this is not specified, the default behavior is defined by gRPC."
type: string
httpGet:
description: HTTPGet specifies the http request to perform.
type: object
Expand Down Expand Up @@ -359,6 +372,19 @@ spec:
description: Minimum consecutive failures for the probe to be considered failed after having succeeded. Defaults to 3. Minimum value is 1.
type: integer
format: int32
grpc:
description: GRPC specifies an action involving a GRPC port. This is a beta field and requires enabling GRPCContainerProbe feature gate.
type: object
required:
- port
properties:
port:
description: Port number of the gRPC service. Number must be in the range 1 to 65535.
type: integer
format: int32
service:
description: "Service is the name of the service to place in the gRPC HealthCheckRequest (see https://github.com/grpc/grpc/blob/master/doc/health-checking.md). \n If this is not specified, the default behavior is defined by gRPC."
type: string
httpGet:
description: HTTPGet specifies the http request to perform.
type: object
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ require (
go.uber.org/zap v1.26.0
golang.org/x/net v0.15.0
golang.org/x/sync v0.3.0
golang.org/x/sys v0.12.0
golang.org/x/time v0.3.0
google.golang.org/api v0.141.0
google.golang.org/grpc v1.58.1
Expand Down Expand Up @@ -138,7 +139,6 @@ require (
golang.org/x/crypto v0.13.0 // indirect
golang.org/x/mod v0.12.0 // indirect
golang.org/x/oauth2 v0.12.0 // indirect
golang.org/x/sys v0.12.0 // indirect
golang.org/x/term v0.12.0 // indirect
golang.org/x/text v0.13.0 // indirect
golang.org/x/tools v0.13.0 // indirect
Expand Down
5 changes: 5 additions & 0 deletions hack/schemapatch-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,11 @@ k8s.io/api/core/v1.ProbeHandler:
- Exec
- HTTPGet
- TCPSocket
- GRPC
k8s.io/api/core/v1.GRPCAction:
fieldMask:
- Port
- Service
k8s.io/api/core/v1.ExecAction:
fieldMask:
- Command
Expand Down
17 changes: 17 additions & 0 deletions pkg/apis/serving/fieldmask.go
Original file line number Diff line number Diff line change
Expand Up @@ -374,6 +374,7 @@ func HandlerMask(in *corev1.ProbeHandler) *corev1.ProbeHandler {
out.Exec = in.Exec
out.HTTPGet = in.HTTPGet
out.TCPSocket = in.TCPSocket
out.GRPC = in.GRPC

return out

Expand Down Expand Up @@ -429,6 +430,22 @@ func TCPSocketActionMask(in *corev1.TCPSocketAction) *corev1.TCPSocketAction {
return out
}

// GRPCActionMask performs a _shallow_ copy of the Kubernetes GRPCAction object to a new
// Kubernetes GRPCAction object bringing over only the fields allowed in the Knative API. This
// does not validate the contents or the bounds of the provided fields.
func GRPCActionMask(in *corev1.GRPCAction) *corev1.GRPCAction {
if in == nil {
return nil
}
out := new(corev1.GRPCAction)

// Allowed fields
out.Port = in.Port
out.Service = in.Service

return out
}

// ContainerPortMask performs a _shallow_ copy of the Kubernetes ContainerPort object to a new
// Kubernetes ContainerPort object bringing over only the fields allowed in the Knative API. This
// does not validate the contents or the bounds of the provided fields.
Expand Down
28 changes: 28 additions & 0 deletions pkg/apis/serving/fieldmask_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -318,6 +318,7 @@ func TestHandlerMask(t *testing.T) {
Exec: &corev1.ExecAction{},
HTTPGet: &corev1.HTTPGetAction{},
TCPSocket: &corev1.TCPSocketAction{},
GRPC: &corev1.GRPCAction{},
}
in := want

Expand Down Expand Up @@ -421,6 +422,33 @@ func TestTCPSocketActionMask(t *testing.T) {
}
}

func TestGRPCActionMask(t *testing.T) {
want := &corev1.GRPCAction{
Port: 42,
Service: ptr.String("foo"),
}
in := &corev1.GRPCAction{
Port: 42,
Service: ptr.String("foo"),
}

got := GRPCActionMask(in)

if &want == &got {
t.Error("Input and output share addresses. Want different addresses")
}

if diff, err := kmp.SafeDiff(want, got); err != nil {
t.Error("Got error comparing output, err =", err)
} else if diff != "" {
t.Error("GRPCActionMask (-want, +got):", diff)
}

if got = GRPCActionMask(nil); got != nil {
t.Errorf("GRPCActionMask(nil) = %v, want: nil", got)
}
}

func TestContainerPortMask(t *testing.T) {
want := &corev1.ContainerPort{
ContainerPort: 42,
Expand Down
6 changes: 5 additions & 1 deletion pkg/apis/serving/k8s_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -832,9 +832,13 @@ func validateProbe(p *corev1.Probe, port corev1.ContainerPort) *apis.FieldError
handlers = append(handlers, "exec")
errs = errs.Also(apis.CheckDisallowedFields(*h.Exec, *ExecActionMask(h.Exec))).ViaField("exec")
}
if h.GRPC != nil {
handlers = append(handlers, "gRPC")
errs = errs.Also(apis.CheckDisallowedFields(*h.GRPC, *GRPCActionMask(h.GRPC))).ViaField("grpc")
}

if len(handlers) == 0 {
errs = errs.Also(apis.ErrMissingOneOf("httpGet", "tcpSocket", "exec"))
errs = errs.Also(apis.ErrMissingOneOf("httpGet", "tcpSocket", "exec", "grpc"))
} else if len(handlers) > 1 {
errs = errs.Also(apis.ErrMultipleOneOf(handlers...))
}
Expand Down
35 changes: 34 additions & 1 deletion pkg/apis/serving/k8s_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1654,7 +1654,7 @@ func TestContainerValidation(t *testing.T) {
ProbeHandler: corev1.ProbeHandler{},
},
},
want: apis.ErrMissingOneOf("livenessProbe.httpGet", "livenessProbe.tcpSocket", "livenessProbe.exec"),
want: apis.ErrMissingOneOf("livenessProbe.httpGet", "livenessProbe.tcpSocket", "livenessProbe.exec", "livenessProbe.grpc"),
}, {
name: "invalid with multiple handlers",
c: corev1.Container{
Expand Down Expand Up @@ -1888,6 +1888,39 @@ func TestContainerValidation(t *testing.T) {
},
want: apis.ErrDisallowedFields("lifecycle").Also(
apis.ErrMissingField("image")),
}, {
name: "valid grpc probe",
c: corev1.Container{
Image: "foo",
ReadinessProbe: &corev1.Probe{
PeriodSeconds: 1,
TimeoutSeconds: 1,
SuccessThreshold: 1,
FailureThreshold: 3,
ProbeHandler: corev1.ProbeHandler{
GRPC: &corev1.GRPCAction{
Port: 46,
},
},
},
},
}, {
name: "valid grpc probe with service",
c: corev1.Container{
Image: "foo",
ReadinessProbe: &corev1.Probe{
PeriodSeconds: 1,
TimeoutSeconds: 1,
SuccessThreshold: 1,
FailureThreshold: 3,
ProbeHandler: corev1.ProbeHandler{
GRPC: &corev1.GRPCAction{
Port: 46,
Service: ptr.String("foo"),
},
},
},
},
},
}
tests = append(tests, getCommonContainerValidationTestCases()...)
Expand Down
7 changes: 6 additions & 1 deletion pkg/apis/serving/v1/revision_defaults.go
Original file line number Diff line number Diff line change
Expand Up @@ -143,10 +143,15 @@ func (*RevisionSpec) applyProbes(container *corev1.Container) {
}
if container.ReadinessProbe.TCPSocket == nil &&
container.ReadinessProbe.HTTPGet == nil &&
container.ReadinessProbe.Exec == nil {
container.ReadinessProbe.Exec == nil &&
container.ReadinessProbe.GRPC == nil {
container.ReadinessProbe.TCPSocket = &corev1.TCPSocketAction{}
}

if container.ReadinessProbe.GRPC != nil && container.ReadinessProbe.GRPC.Service == nil {
container.ReadinessProbe.GRPC.Service = ptr.String("")
}

if container.ReadinessProbe.SuccessThreshold == 0 {
container.ReadinessProbe.SuccessThreshold = 1
}
Expand Down
Loading

0 comments on commit d02702e

Please sign in to comment.