Validate ping mode flag, change ping-only to poll-only

Author: DrJosh9000

agent/agent_worker.go

@@ -20,6 +20,12 @@ import (
 	"github.com/buildkite/roko"
 )
 
+const (
+	PingModeAuto       = "auto" // empty string can be used from tests
+	PingModeStreamOnly = "stream-only"
+	PingModePollOnly   = "poll-only"
+)
+
 type AgentWorkerConfig struct {
 	// Whether to set debug in the job
 	Debug bool
@@ -323,18 +329,21 @@ func (a *AgentWorker) Start(ctx context.Context, idleMon *idleMonitor) (startErr
 
 	var loops []func()
 	switch a.agentConfiguration.PingMode {
-	case "", "auto":
+	case "", PingModeAuto: // note: "" can happen in some tests
 		loops = []func(){pingLoop, streamingLoop, debouncerLoop}
 		bat.Acquire(actorDebouncer)
 
-	case "ping-only":
+	case PingModePollOnly:
 		loops = []func(){pingLoop}
 		fromDebouncerCh = nil // prevent action loop listening to streaming side
 
-	case "stream-only":
+	case PingModeStreamOnly:
 		loops = []func(){streamingLoop, debouncerLoop}
 		fromPingLoopCh = nil // prevent action loop listening to ping side
 		bat.Acquire(actorDebouncer)
+
+	default:
+		return fmt.Errorf("unknown ping mode %q", a.agentConfiguration.PingMode)
 	}
 
 	// There's always an action handler.

clicommand/agent_start.go

@@ -60,9 +60,18 @@ Example:
 
     $ buildkite-agent start --token xxx`
 
+const pingModePingOnly = "ping-only"
+
 var (
 	verificationFailureBehaviors = []string{agent.VerificationBehaviourBlock, agent.VerificationBehaviourWarn}
 
+	pingModes = []string{
+		agent.PingModeAuto,
+		agent.PingModePollOnly,
+		pingModePingOnly, // canonicalises to agent.PingModePollOnly
+		agent.PingModeStreamOnly,
+	}
+
 	buildkiteSetEnvironmentVariables = []*regexp.Regexp{
 		regexp.MustCompile("^BUILDKITE$"),
 		regexp.MustCompile("^BUILDKITE_.*$"),
@@ -764,8 +773,8 @@ var AgentStartCommand = cli.Command{
 		// API + agent behaviour
 		cli.StringFlag{
 			Name:   "ping-mode",
-			Usage:  "Selects available protocols for dispatching work to this agent. One of ping-only (default), auto (prefer streaming, but fall back to polling when necessary) or stream-only.",
-			Value:  "ping-only",
+			Usage:  "Selects available protocols for dispatching work to this agent. One of auto (default, prefer streaming, but fall back to polling when necessary), poll-only, or stream-only.",
+			Value:  "auto",
 			EnvVar: "BUILDKITE_AGENT_PING_MODE",
 		},
 
@@ -852,6 +861,15 @@ var AgentStartCommand = cli.Command{
 			return fmt.Errorf("failed to unset config from environment: %w", err)
 		}
 
+		if !slices.Contains(pingModes, cfg.PingMode) {
+			return fmt.Errorf("invalid ping mode %q, must be one of %v", cfg.PingMode, pingModes)
+		}
+		// Calling it "ping-only" was a mistake, so canonicalise it to "poll-only"
+		// on the very remote chance someone is using that.
+		if cfg.PingMode == pingModePingOnly {
+			cfg.PingMode = agent.PingModePollOnly
+		}
+
 		if cfg.VerificationJWKSFile != "" {
 			if !slices.Contains(verificationFailureBehaviors, cfg.VerificationFailureBehavior) {
 				return fmt.Errorf(

internal/e2e/basic_test.go

@@ -31,11 +31,11 @@ func TestBasicE2E(t *testing.T) {
 	}
 }
 
-func TestBasicE2E_PingOnly(t *testing.T) {
+func TestBasicE2E_PollOnly(t *testing.T) {
 	ctx := t.Context()
 	tc := newTestCase(t, "basic_e2e.yaml")
 
-	tc.startAgent("--ping-mode=ping-only")
+	tc.startAgent("--ping-mode=poll-only")
 	build := tc.triggerBuild()
 
 	// It should take much less time than 1 minute to successfully run the job.