Repository: bacalhau-project/bacalhau (language: python)

Issue:
bacalhau serve --max-timeout is broken
The `--max-timeout` flags value is being ignored.

The flag is bound to the following config filed: https://github.com/bacalhau-project/bacalhau/blob/main/cmd/util/flags/configflags/capacity.go#L57
But isn't used when creating the compute config: https://github.com/bacalhau-project/bacalhau/blob/main/cmd/cli/serve/util.go#L32

Source: https://bacalhauproject.slack.com/archives/C052BAZJVEU/p1695933433654849


Discussion / hints:
Now is probably a good time to also point out that we have many different "Timeout" fields in the config:
https://github.com/bacalhau-project/bacalhau/blob/main/pkg/config/types/compute.go#L23
https://github.com/bacalhau-project/bacalhau/blob/main/pkg/config/types/compute.go#L37
https://github.com/bacalhau-project/bacalhau/blob/main/pkg/config/types/requester.go#L39

That are disjoint from the defaults we use in scheduling a job: https://github.com/bacalhau-project/bacalhau/blob/main/cmd/util/flags/cliflags/spec.go#L28

Source: https://bacalhauproject.slack.com/archives/C052BAZJVEU/p1695933433654849

Fix patch:
diff --git a/cmd/cli/devstack/devstack.go b/cmd/cli/devstack/devstack.go
index 6493557bbb..08ab61205b 100644
--- a/cmd/cli/devstack/devstack.go
+++ b/cmd/cli/devstack/devstack.go
@@ -9,12 +9,13 @@ import (
 
 	"k8s.io/kubectl/pkg/util/i18n"
 
+	"github.com/samber/lo"
+
 	"github.com/bacalhau-project/bacalhau/cmd/util/flags/configflags"
 	"github.com/bacalhau-project/bacalhau/pkg/config"
 	"github.com/bacalhau-project/bacalhau/pkg/config/types"
 	"github.com/bacalhau-project/bacalhau/pkg/node"
 	"github.com/bacalhau-project/bacalhau/pkg/setup"
-	"github.com/samber/lo"
 
 	"github.com/bacalhau-project/bacalhau/cmd/cli/serve"
 	"github.com/bacalhau-project/bacalhau/cmd/util"
@@ -69,6 +70,7 @@ func NewCmd() *cobra.Command {
 		"job-selection":    configflags.JobSelectionFlags,
 		"disable-features": configflags.DisabledFeatureFlags,
 		"capacity":         configflags.CapacityFlags,
+		"job-timeouts":     configflags.ComputeTimeoutFlags,
 	}
 
 	devstackCmd := &cobra.Command{
diff --git a/cmd/cli/serve/serve.go b/cmd/cli/serve/serve.go
index 8e73d86963..b6b8a9b48d 100644
--- a/cmd/cli/serve/serve.go
+++ b/cmd/cli/serve/serve.go
@@ -92,6 +92,7 @@ func NewCmd() *cobra.Command {
 		"libp2p":           configflags.Libp2pFlags,
 		"ipfs":             configflags.IPFSFlags,
 		"capacity":         configflags.CapacityFlags,
+		"job-timeouts":     configflags.ComputeTimeoutFlags,
 		"job-selection":    configflags.JobSelectionFlags,
 		"disable-features": configflags.DisabledFeatureFlags,
 		"labels":           configflags.LabelFlags,
diff --git a/cmd/util/flags/configflags/capacity.go b/cmd/util/flags/configflags/capacity.go
index 5fc3580a62..f55224f2dd 100644
--- a/cmd/util/flags/configflags/capacity.go
+++ b/cmd/util/flags/configflags/capacity.go
@@ -3,12 +3,6 @@ package configflags
 import "github.com/bacalhau-project/bacalhau/pkg/config/types"
 
 var CapacityFlags = []Definition{
-	{
-		FlagName:     "job-execution-timeout-bypass-client-id",
-		ConfigPath:   types.NodeComputeJobTimeoutsJobExecutionTimeoutClientIDBypassList,
-		DefaultValue: Default.Node.Compute.JobTimeouts.JobExecutionTimeoutClientIDBypassList,
-		Description:  `List of IDs of clients that are allowed to bypass the job execution timeout check`,
-	},
 	{
 		FlagName:             "ignore-physical-resource-limits",
 		ConfigPath:           types.NodeComputeCapacityIgnorePhysicalResourceLimits,
@@ -52,10 +46,4 @@ var CapacityFlags = []Definition{
 		DefaultValue: Default.Node.Compute.Capacity.JobResourceLimits.GPU,
 		Description:  `Job GPU limit to run all jobs (e.g. 1, 2, or 8).`,
 	},
-	{
-		FlagName:     "max-timeout",
-		ConfigPath:   types.NodeComputeCapacityMaxJobExecutionTimeout,
-		DefaultValue: Default.Node.Compute.Capacity.MaxJobExecutionTimeout,
-		Description:  `Job time exeuciton limit for a single job (e.g. 30m, 1hr)`,
-	},
 }
diff --git a/cmd/util/flags/configflags/timeouts.go b/cmd/util/flags/configflags/timeouts.go
new file mode 100644
index 0000000000..2ecd4e0518
--- /dev/null
+++ b/cmd/util/flags/configflags/timeouts.go
@@ -0,0 +1,36 @@
+package configflags
+
+import "github.com/bacalhau-project/bacalhau/pkg/config/types"
+
+var ComputeTimeoutFlags = []Definition{
+	{
+		FlagName:     "job-execution-timeout-bypass-client-id",
+		ConfigPath:   types.NodeComputeJobTimeoutsJobExecutionTimeoutClientIDBypassList,
+		DefaultValue: Default.Node.Compute.JobTimeouts.JobExecutionTimeoutClientIDBypassList,
+		Description:  `List of IDs of clients that are allowed to bypass the job execution timeout check`,
+	},
+	{
+		FlagName:     "job-negotiation-timeout",
+		ConfigPath:   types.NodeComputeJobTimeoutsJobNegotiationTimeout,
+		DefaultValue: Default.Node.Compute.JobTimeouts.JobNegotiationTimeout,
+		Description:  `Timeout value to hold a bid for a job.`,
+	},
+	{
+		FlagName:     "min-job-execution-timeout",
+		ConfigPath:   types.NodeComputeJobTimeoutsMinJobExecutionTimeout,
+		DefaultValue: Default.Node.Compute.JobTimeouts.MinJobExecutionTimeout,
+		Description:  `The minimum execution timeout this compute node supports. Jobs with lower timeout requirements will not be bid on.`,
+	},
+	{
+		FlagName:     "max-job-execution-timeout",
+		ConfigPath:   types.NodeComputeJobTimeoutsMaxJobExecutionTimeout,
+		DefaultValue: Default.Node.Compute.JobTimeouts.MaxJobExecutionTimeout,
+		Description:  `The maximum execution timeout this compute node supports. Jobs with higher timeout requirements will not be bid on.`,
+	},
+	{
+		FlagName:     "default-job-execution-timeout",
+		ConfigPath:   types.NodeComputeJobTimeoutsJobExecutionTimeoutClientIDBypassList,
+		DefaultValue: Default.Node.Compute.JobTimeouts.DefaultJobExecutionTimeout,
+		Description:  `default value for the execution timeout this compute node will assign to jobs with no timeout requirement defined.`,
+	},
+}
diff --git a/ops/terraform/remote_files/scripts/start-bacalhau.sh b/ops/terraform/remote_files/scripts/start-bacalhau.sh
index 5fac0e14fb..64af63062b 100644
--- a/ops/terraform/remote_files/scripts/start-bacalhau.sh
+++ b/ops/terraform/remote_files/scripts/start-bacalhau.sh
@@ -64,7 +64,7 @@ bacalhau serve \
   --job-selection-data-locality anywhere \
   --job-selection-accept-networked \
   --job-selection-probe-exec "${BACALHAU_PROBE_EXEC}" \
-  --max-timeout '60m' \
+  --max-job-execution-timeout '60m' \
   --job-execution-timeout-bypass-client-id="${TRUSTED_CLIENT_IDS}" \
   --ipfs-swarm-addrs "" \
   --ipfs-connect /ip4/127.0.0.1/tcp/5001 \
diff --git a/pkg/config/types/compute.go b/pkg/config/types/compute.go
index d29b551cdd..6ac79c75a8 100644
--- a/pkg/config/types/compute.go
+++ b/pkg/config/types/compute.go
@@ -19,8 +19,6 @@ type CapacityConfig struct {
 	JobResourceLimits        model.ResourceUsageConfig `yaml:"JobResourceLimits"`
 	DefaultJobResourceLimits model.ResourceUsageConfig `yaml:"DefaultJobResourceLimits"`
 	QueueResourceLimits      model.ResourceUsageConfig `yaml:"QueueResourceLimits"`
-	// The maximum time a job can execute for.
-	MaxJobExecutionTimeout Duration `yaml:"MaxJobExecutionTimeout"`
 }
 
 type JobTimeoutConfig struct {
diff --git a/pkg/config/types/generated_constants.go b/pkg/config/types/generated_constants.go
index 4a1e3c5133..2ab0d7c036 100644
--- a/pkg/config/types/generated_constants.go
+++ b/pkg/config/types/generated_constants.go
@@ -51,7 +51,6 @@ const NodeComputeCapacityQueueResourceLimitsCPU = "Node.Compute.Capacity.QueueRe
 const NodeComputeCapacityQueueResourceLimitsMemory = "Node.Compute.Capacity.QueueResourceLimits.Memory"
 const NodeComputeCapacityQueueResourceLimitsDisk = "Node.Compute.Capacity.QueueResourceLimits.Disk"
 const NodeComputeCapacityQueueResourceLimitsGPU = "Node.Compute.Capacity.QueueResourceLimits.GPU"
-const NodeComputeCapacityMaxJobExecutionTimeout = "Node.Compute.Capacity.MaxJobExecutionTimeout"
 const NodeComputeExecutionStore = "Node.Compute.ExecutionStore"
 const NodeComputeExecutionStoreType = "Node.Compute.ExecutionStore.Type"
 const NodeComputeExecutionStorePath = "Node.Compute.ExecutionStore.Path"
diff --git a/pkg/config/types/generated_viper_defaults.go b/pkg/config/types/generated_viper_defaults.go
index 631893ffae..9af5c9c483 100644
--- a/pkg/config/types/generated_viper_defaults.go
+++ b/pkg/config/types/generated_viper_defaults.go
@@ -54,7 +54,6 @@ func SetDefaults(cfg BacalhauConfig) {
 	viper.SetDefault(NodeComputeCapacityQueueResourceLimitsMemory, cfg.Node.Compute.Capacity.QueueResourceLimits.Memory)
 	viper.SetDefault(NodeComputeCapacityQueueResourceLimitsDisk, cfg.Node.Compute.Capacity.QueueResourceLimits.Disk)
 	viper.SetDefault(NodeComputeCapacityQueueResourceLimitsGPU, cfg.Node.Compute.Capacity.QueueResourceLimits.GPU)
-	viper.SetDefault(NodeComputeCapacityMaxJobExecutionTimeout, cfg.Node.Compute.Capacity.MaxJobExecutionTimeout.String())
 	viper.SetDefault(NodeComputeExecutionStore, cfg.Node.Compute.ExecutionStore)
 	viper.SetDefault(NodeComputeExecutionStoreType, cfg.Node.Compute.ExecutionStore.Type.String())
 	viper.SetDefault(NodeComputeExecutionStorePath, cfg.Node.Compute.ExecutionStore.Path)
@@ -179,7 +178,6 @@ func Set(cfg BacalhauConfig) {
 	viper.Set(NodeComputeCapacityQueueResourceLimitsMemory, cfg.Node.Compute.Capacity.QueueResourceLimits.Memory)
 	viper.Set(NodeComputeCapacityQueueResourceLimitsDisk, cfg.Node.Compute.Capacity.QueueResourceLimits.Disk)
 	viper.Set(NodeComputeCapacityQueueResourceLimitsGPU, cfg.Node.Compute.Capacity.QueueResourceLimits.GPU)
-	viper.Set(NodeComputeCapacityMaxJobExecutionTimeout, cfg.Node.Compute.Capacity.MaxJobExecutionTimeout.String())
 	viper.Set(NodeComputeExecutionStore, cfg.Node.Compute.ExecutionStore)
 	viper.Set(NodeComputeExecutionStoreType, cfg.Node.Compute.ExecutionStore.Type.String())
 	viper.Set(NodeComputeExecutionStorePath, cfg.Node.Compute.ExecutionStore.Path)


Test patch:
diff --git a/cmd/cli/serve/timeout_test.go b/cmd/cli/serve/timeout_test.go
index 7f193c825c..20ebc63715 100644
--- a/cmd/cli/serve/timeout_test.go
+++ b/cmd/cli/serve/timeout_test.go
@@ -48,7 +48,7 @@ func (s *ServeSuite) TestNoTimeoutSetOrApplied() {
 		s.Run(name, func() {
 			args := []string{}
 			if tc.configuredMax != nil {
-				args = append(args, "--max-timeout", tc.configuredMax.String())
+				args = append(args, "--max-job-execution-timeout", tc.configuredMax.String())
 			}
 
 			port, err := s.serve(args...)