Address comments

- Use context for timeout
- do not override exec probe
- simplify the logic for errors when multiple probes are mentioned

Signed-off-by: Shash Reddy <shashwathireddy@gmail.com>

cmd/queue/main.go

@@ -247,14 +247,9 @@ func probeQueueHealthPath(port int, timeoutSeconds int) error {
 		},
 		Timeout: timeoutDuration,
 	}
-
-	stopCh := make(chan struct{})
-	timeCh := time.After(timeoutDuration)
-
-	go func() {
-		<-timeCh
-		close(stopCh)
-	}()
+	ctx, cancel := context.WithTimeout(context.Background(), timeoutDuration)
+	defer cancel()
+	stopCh := ctx.Done()
 
 	var lastErr error
 	timeoutErr := wait.PollImmediateUntil(aggressivePollInterval, func() (bool, error) {

pkg/apis/serving/k8s_validation.go

@@ -467,30 +467,25 @@ func validateProbe(p *corev1.Probe) *apis.FieldError {
 	h := p.Handler
 	errs = errs.Also(apis.CheckDisallowedFields(h, *HandlerMask(&h)))
 
-	numHandlers := 0
+	var handlers []string
 
 	if h.HTTPGet != nil {
-		numHandlers++
+		handlers = append(handlers, "httpGet")
 		errs = errs.Also(apis.CheckDisallowedFields(*h.HTTPGet, *HTTPGetActionMask(h.HTTPGet))).ViaField("httpGet")
 	}
 	if h.TCPSocket != nil {
-		if numHandlers > 0 {
-			errs = errs.Also(apis.ErrDisallowedFields("tcpSocket"))
-		} else {
-			numHandlers++
-			errs = errs.Also(apis.CheckDisallowedFields(*h.TCPSocket, *TCPSocketActionMask(h.TCPSocket))).ViaField("tcpSocket")
-		}
+		handlers = append(handlers, "tcpSocket")
+		errs = errs.Also(apis.CheckDisallowedFields(*h.TCPSocket, *TCPSocketActionMask(h.TCPSocket))).ViaField("tcpSocket")
 	}
 	if h.Exec != nil {
-		if numHandlers > 0 {
-			errs = errs.Also(apis.ErrDisallowedFields("exec"))
-		} else {
-			numHandlers++
-			errs = errs.Also(apis.CheckDisallowedFields(*h.Exec, *ExecActionMask(h.Exec))).ViaField("exec")
-		}
+		handlers = append(handlers, "exec")
+		errs = errs.Also(apis.CheckDisallowedFields(*h.Exec, *ExecActionMask(h.Exec))).ViaField("exec")
 	}
-	if numHandlers == 0 {
+
+	if len(handlers) == 0 {
 		errs = errs.Also(apis.ErrMissingField("handler"))
+	} else if len(handlers) > 1 {
+		errs = errs.Also(apis.ErrMultipleOneOf(handlers...))
 	}
 	return errs
 }

pkg/apis/serving/k8s_validation_test.go

@@ -576,8 +576,7 @@ func TestContainerValidation(t *testing.T) {
 				},
 			},
 		},
-		want: apis.ErrDisallowedFields("readinessProbe.exec").Also(
-			apis.ErrDisallowedFields("readinessProbe.tcpSocket")),
+		want: apis.ErrMultipleOneOf("readinessProbe.exec", "readinessProbe.tcpSocket", "readinessProbe.httpGet"),
 	}, {
 		name: "invalid readiness http probe (has port)",
 		c: corev1.Container{

pkg/reconciler/revision/resources/deploy_test.go

@@ -580,7 +580,7 @@ func TestMakePodSpec(t *testing.T) {
 					withExecReadinessProbe([]string{"echo", "hello"})),
 				queueContainer(
 					withEnvVar("CONTAINER_CONCURRENCY", "0"),
-					withEnvVar("SERVING_READINESS_PROBE", `{"tcpSocket":{"port":8080,"host":"127.0.0.1"},"successThreshold":1}`),
+					withEnvVar("SERVING_READINESS_PROBE", "{}"),
 				),
 			}),
 	}, {

pkg/reconciler/revision/resources/queue.go

@@ -336,12 +336,7 @@ func applyReadinessProbeDefaults(p *corev1.Probe, port int32) {
 		p.TCPSocket.Host = localAddress
 		p.TCPSocket.Port = intstr.FromInt(int(port))
 	case p.Exec != nil:
-		// Use default TCP connect probe to ensure data path is open from queue-proxy to
-		// user-container. User-defined ExecProbe will still be run on user-container.
-		p.TCPSocket = &corev1.TCPSocketAction{}
-		p.SuccessThreshold = 1
-		p.TCPSocket.Host = localAddress
-		p.TCPSocket.Port = intstr.FromInt(int(port))
+		//User-defined ExecProbe will still be run on user-container.
 		p.Exec = nil
 	}