From 5003f7d1e5704e179629f1e5990378fadc1b1b4c Mon Sep 17 00:00:00 2001 From: Walid Baruni Date: Sun, 12 Jan 2025 14:34:55 +0200 Subject: [PATCH] remove unnecessary execution env vars (#4806) ## Summary by CodeRabbit - **Refactor** - Removed environment variable tracking for `NODE_ID`, `JOB_NAME`, and `JOB_NAMESPACE` in job execution. - Updated test cases to remove node-specific verifications and focus on partition-specific outputs. - Adjusted expected outputs in tests to reflect changes in environment variable priorities. --- pkg/compute/envvars.go | 3 --- pkg/compute/envvars_test.go | 16 +++------------- pkg/test/devstack/partitioning_test.go | 18 ++---------------- 3 files changed, 5 insertions(+), 32 deletions(-) diff --git a/pkg/compute/envvars.go b/pkg/compute/envvars.go index 2ab524faf6..f2051ee783 100644 --- a/pkg/compute/envvars.go +++ b/pkg/compute/envvars.go @@ -25,13 +25,10 @@ func GetExecutionEnvVars(execution *models.Execution) map[string]string { // Build system environment variables sysEnv := make(map[string]string) sysEnv[models.EnvVarPrefix+"EXECUTION_ID"] = execution.ID - sysEnv[models.EnvVarPrefix+"NODE_ID"] = envvar.Sanitize(execution.NodeID) // Add job-related environment variables if job exists if execution.Job != nil { sysEnv[models.EnvVarPrefix+"JOB_ID"] = execution.JobID - sysEnv[models.EnvVarPrefix+"JOB_NAME"] = envvar.Sanitize(execution.Job.Name) - sysEnv[models.EnvVarPrefix+"JOB_NAMESPACE"] = envvar.Sanitize(execution.Job.Namespace) sysEnv[models.EnvVarPrefix+"JOB_TYPE"] = execution.Job.Type // Add partition-related environment variables diff --git a/pkg/compute/envvars_test.go b/pkg/compute/envvars_test.go index 3ad514d4c8..2ec078c42c 100644 --- a/pkg/compute/envvars_test.go +++ b/pkg/compute/envvars_test.go @@ -29,7 +29,6 @@ func TestGetExecutionEnvVars(t *testing.T) { }, want: map[string]string{ "BACALHAU_EXECUTION_ID": "exec-1", - "BACALHAU_NODE_ID": "node-1", }, }, { @@ -53,10 +52,7 @@ func TestGetExecutionEnvVars(t *testing.T) { }, want: map[string]string{ "BACALHAU_EXECUTION_ID": "exec-1", - "BACALHAU_NODE_ID": "node-1", "BACALHAU_JOB_ID": "job-1", - "BACALHAU_JOB_NAME": "test-job", - "BACALHAU_JOB_NAMESPACE": "default", "BACALHAU_JOB_TYPE": "batch", "BACALHAU_PARTITION_INDEX": "0", "BACALHAU_PARTITION_COUNT": "3", @@ -77,9 +73,9 @@ func TestGetExecutionEnvVars(t *testing.T) { { Name: "task-1", Env: map[string]string{ - "MY_VAR": "my-value", - "BACALHAU_NODE_ID": "should-not-override", // Should not override system env - "OTHER_VAR": "other-value", + "MY_VAR": "my-value", + "BACALHAU_JOB_TYPE": "should-not-override", // Should not override system env + "OTHER_VAR": "other-value", }, }, }, @@ -88,10 +84,7 @@ func TestGetExecutionEnvVars(t *testing.T) { }, want: map[string]string{ "BACALHAU_EXECUTION_ID": "exec-1", - "BACALHAU_NODE_ID": "node-1", // System value takes precedence "BACALHAU_JOB_ID": "job-1", - "BACALHAU_JOB_NAME": "test-job", - "BACALHAU_JOB_NAMESPACE": "default", "BACALHAU_JOB_TYPE": "batch", "BACALHAU_PARTITION_INDEX": "0", "BACALHAU_PARTITION_COUNT": "3", @@ -120,10 +113,7 @@ func TestGetExecutionEnvVars(t *testing.T) { }, want: map[string]string{ "BACALHAU_EXECUTION_ID": "exec-1", - "BACALHAU_NODE_ID": "node-1", "BACALHAU_JOB_ID": "job-1", - "BACALHAU_JOB_NAME": "test_job_with_spaces", // Sanitized - "BACALHAU_JOB_NAMESPACE": "test_namespace", // Sanitized "BACALHAU_JOB_TYPE": "batch", "BACALHAU_PARTITION_INDEX": "0", "BACALHAU_PARTITION_COUNT": "1", diff --git a/pkg/test/devstack/partitioning_test.go b/pkg/test/devstack/partitioning_test.go index 28c6bef3c0..ccd5a07cca 100644 --- a/pkg/test/devstack/partitioning_test.go +++ b/pkg/test/devstack/partitioning_test.go @@ -62,10 +62,7 @@ func (s *PartitionSuite) TestSinglePartition() { []string{ "BACALHAU_EXECUTION_ID=", "BACALHAU_JOB_ID=", - "BACALHAU_JOB_NAME=" + s.T().Name(), - "BACALHAU_JOB_NAMESPACE=default", "BACALHAU_JOB_TYPE=batch", - "BACALHAU_NODE_ID=node-0", "BACALHAU_PARTITION_COUNT=1", "BACALHAU_PARTITION_INDEX=0", }, @@ -98,7 +95,7 @@ func (s *PartitionSuite) TestMultiplePartitions() { Name: s.T().Name(), Engine: dockmodels.NewDockerEngineBuilder("busybox:1.37.0"). WithEntrypoint("sh", "-c", - "echo Node=${BACALHAU_NODE_ID} Partition=${BACALHAU_PARTITION_INDEX} of ${BACALHAU_PARTITION_COUNT}", + "echo Partition=${BACALHAU_PARTITION_INDEX} of ${BACALHAU_PARTITION_COUNT}", ). MustBuild(), Publisher: publisher_local.NewSpecConfig(), @@ -116,16 +113,6 @@ func (s *PartitionSuite) TestMultiplePartitions() { }, -1, ), - // Verify they run on the nodes - scenario.FileContains( - "stdout", - []string{ - "Node=node-0", - "Node=node-1", - "Node=node-2", - }, - -1, - ), ), JobCheckers: []scenario.StateChecks{ scenario.WaitForSuccessfulCompletion(), @@ -155,9 +142,8 @@ func (s *PartitionSuite) TestPartitionRetry() { ExternalHooks: noop.ExecutorConfigExternalHooks{ JobHandler: func(ctx context.Context, execContext noop.ExecutionContext) (*models.RunCommandResult, error) { partition := execContext.Env["BACALHAU_PARTITION_INDEX"] - nodeID := execContext.Env["BACALHAU_NODE_ID"] - output := fmt.Sprintf("Running partition %s on node %s\n", partition, nodeID) + output := fmt.Sprintf("Running partition %s\n", partition) // Only increment attempt counter for partition 1 var currentAttempt int32