From 1cbdad29a04397e208a04090581ea8f6f61c419f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Tich=C3=A1k?= Date: Fri, 17 Jul 2026 16:45:35 +0200 Subject: [PATCH] [control-operator] Remove OCC_CONTROL_PORT as requirement from TaskTemplate The reason for the removal is that ECS can assign OCC_CONTROL_PORT only for mesos tasks, thus the removal here. The variable is now set inside the TaskTemplates. Ability to overwrite EnvVars from TaskReference CRDs was also added --- .../task-templates/readout-tasktemplate.yaml | 3 ++ .../stfbuilder-senderoutput-tasktemplate.yaml | 3 ++ .../stfsender-tasktemplate.yaml | 3 ++ .../controller/environment_controller.go | 46 ++++++++++++------- 4 files changed, 38 insertions(+), 17 deletions(-) diff --git a/control-operator/ecs-manifests/task-templates/readout-tasktemplate.yaml b/control-operator/ecs-manifests/task-templates/readout-tasktemplate.yaml index 467470ce..a1010cef 100644 --- a/control-operator/ecs-manifests/task-templates/readout-tasktemplate.yaml +++ b/control-operator/ecs-manifests/task-templates/readout-tasktemplate.yaml @@ -16,6 +16,9 @@ spec: - name: readout image: gitlab-registry.cern.ch/aliceo2group/dockerfiles/alma9-flp-node/readout:1 command: ["/opt/o2/bin/o2-readout-exe"] + env: + - name: OCC_CONTROL_PORT + value: "47101" securityContext: privileged: true volumeMounts: diff --git a/control-operator/ecs-manifests/task-templates/stfbuilder-senderoutput-tasktemplate.yaml b/control-operator/ecs-manifests/task-templates/stfbuilder-senderoutput-tasktemplate.yaml index 50e3b757..f3a886b0 100644 --- a/control-operator/ecs-manifests/task-templates/stfbuilder-senderoutput-tasktemplate.yaml +++ b/control-operator/ecs-manifests/task-templates/stfbuilder-senderoutput-tasktemplate.yaml @@ -19,6 +19,9 @@ spec: - name: stfbuilder image: gitlab-registry.cern.ch/aliceo2group/dockerfiles/alma9-flp-node/dd:1 command: [/opt/o2/bin/StfBuilder] + env: + - name: OCC_CONTROL_PORT + value: "47102" securityContext: privileged: true volumeMounts: diff --git a/control-operator/ecs-manifests/task-templates/stfsender-tasktemplate.yaml b/control-operator/ecs-manifests/task-templates/stfsender-tasktemplate.yaml index 082b7309..874baedd 100644 --- a/control-operator/ecs-manifests/task-templates/stfsender-tasktemplate.yaml +++ b/control-operator/ecs-manifests/task-templates/stfsender-tasktemplate.yaml @@ -26,6 +26,9 @@ spec: - name: stfsender image: gitlab-registry.cern.ch/aliceo2group/dockerfiles/alma9-flp-node/dd:1 command: ["numactl"] + env: + - name: OCC_CONTROL_PORT + value: "47103" securityContext: privileged: true volumeMounts: diff --git a/control-operator/internal/controller/environment_controller.go b/control-operator/internal/controller/environment_controller.go index d1159bcd..a644d887 100644 --- a/control-operator/internal/controller/environment_controller.go +++ b/control-operator/internal/controller/environment_controller.go @@ -83,27 +83,11 @@ func (r *EnvironmentReconciler) runTasksFromReferenceOnNode(ctx context.Context, task.Spec.Pod = *template.Spec.Pod.DeepCopy() task.Spec.Control = *template.Spec.Control.DeepCopy() - // TODO: regarding error handling. Is it correct to stop while handling one task? this will fail the whole deployment, - // which might not be desirable outcome especially for non-critical tasks - if foundIdx := slices.IndexFunc(taskReference.Env, func(envVar v1.EnvVar) bool { return envVar.Name == "OCC_CONTROL_PORT" }); foundIdx == -1 { - log.Error(fmt.Errorf("didn't find OCC_CONTROL_PORT in env"), "failed to fill in env vars from template") - return &reconcile.Result{}, nil - } else { - port, err := strconv.Atoi(taskReference.Env[foundIdx].Value) - if err != nil { - log.Error(fmt.Errorf("found OCC_CONTROL_PORT isn't convertible to number "), "failed to fill in env vars from template") - return &reconcile.Result{}, nil - } - task.Spec.Control.Port = port - } - if task.Spec.Arguments == nil { task.Spec.Arguments = make(map[string]string) } - // TODO: check for containers! - task.Spec.Pod.Containers[0].Env = append(task.Spec.Pod.Containers[0].Env, taskReference.Env...) - task.Spec.Pod.Containers[0].Args = append(task.Spec.Pod.Containers[0].Args, taskReference.ArgsCLI...) + handleEnvVars(task, taskReference) maps.Copy(task.Spec.Arguments, taskReference.ArgsTransition) task.Spec.Pod.NodeName = nodename @@ -127,6 +111,34 @@ func (r *EnvironmentReconciler) runTasksFromReferenceOnNode(ctx context.Context, return nil, nil } +func handleEnvVars(task *aliecsv1alpha1.Task, taskReference aliecsv1alpha1.TaskReference) { + // TODO: check for existence of containers + // TODO: adapt for Pods with multiple containers + // reference to existing Env Vars to overwrite them if found in taskReference + existing := make(map[string]int, len(task.Spec.Pod.Containers[0].Env)) + for i, e := range task.Spec.Pod.Containers[0].Env { + existing[e.Name] = i + } + for _, e := range taskReference.Env { + if i, ok := existing[e.Name]; ok { + task.Spec.Pod.Containers[0].Env[i] = e + } else { + task.Spec.Pod.Containers[0].Env = append(task.Spec.Pod.Containers[0].Env, e) + } + } + task.Spec.Pod.Containers[0].Args = append(task.Spec.Pod.Containers[0].Args, taskReference.ArgsCLI...) + + // TODO: should the OCC_CONTROL_PORT handled in task_controller.go and set only Ports and types in this controller? + for _, envVar := range task.Spec.Pod.Containers[0].Env { + if envVar.Name == "OCC_CONTROL_PORT" { + if port, err := strconv.Atoi(envVar.Value); err == nil { + task.Spec.Control.Port = port + } + break + } + } +} + func (r *EnvironmentReconciler) runTaskFromDefinitionOnNode(ctx context.Context, taskDefs []aliecsv1alpha1.TaskDefinition, nodename string, req ctrl.Request, environment *aliecsv1alpha1.Environment, log logr.Logger, ) (*ctrl.Result, error) {