kueue: skip old platform param if new param exists

We came across a pipelinerun that was using the new parameter format to
parameterize the old template format via a matrix definition.  While
it's likely that the pipelinerun is doing extra configuration here that
it doesn't need to (it had `PLATFORM` defined in both matrix params and
task params), tekton doesn't reject it.  Therefore, we need to consider
the possibility that these pipelineruns may exist - tekton-kueue will
need to handle such pipelines without issues.

The problem in the pipelinerun that caused issues was that the old
platform parameter was populated with substitution data that cannot fit
inside a label.  The CEL expressions don't run against the data that has
been substituted in (that would require us to monitor TaskRuns instead),
so they would try to create labels with invalid characters, which would
be rejected by kubernetes.  Since the old platforms parameter will only
contain garbage like this, and this can happen only if the new platform
parameter is populated, only extract the old parameter if the new
parameter isn't exist.

See KONFLUX-9917 for further details.

Fixes: KONFLUX-9917
Signed-off-by: Andy Sadler <[email protected]>

Author: sadlerap

components/kueue/development/tekton-kueue/config.yaml

@@ -38,6 +38,10 @@ cel:
 
     # Set resource requests for multi platform pipelines which doesn't use the build-platforms parameter (old style)
     - |
+      !(
+        has(pipelineRun.spec.params) &&
+        pipelineRun.spec.params.exists(p, p.name == 'build-platforms')
+      ) &&
       has(pipelineRun.spec.pipelineSpec) &&
       has(pipelineRun.spec.pipelineSpec.tasks) &&
       pipelineRun.spec.pipelineSpec.tasks.size() > 0 ?
@@ -53,6 +57,10 @@ cel:
 
     # Request AWS IP for AWS-based platforms which doesn't use the build-platforms parameter (old style)
     - |
+      !(
+        has(pipelineRun.spec.params) &&
+        pipelineRun.spec.params.exists(p, p.name == 'build-platforms')
+      ) &&
       has(pipelineRun.spec.pipelineSpec) &&
       has(pipelineRun.spec.pipelineSpec.tasks) &&
       pipelineRun.spec.pipelineSpec.tasks.size() > 0 ?

components/kueue/staging/base/tekton-kueue/config.yaml

@@ -38,6 +38,10 @@ cel:
 
     # Set resource requests for multi platform pipelines which doesn't use the build-platforms parameter (old style)
     - |
+      !(
+        has(pipelineRun.spec.params) &&
+        pipelineRun.spec.params.exists(p, p.name == 'build-platforms')
+      ) &&
       has(pipelineRun.spec.pipelineSpec) &&
       has(pipelineRun.spec.pipelineSpec.tasks) &&
       pipelineRun.spec.pipelineSpec.tasks.size() > 0 ?
@@ -53,6 +57,10 @@ cel:
 
     # Request AWS IP for AWS-based platforms which doesn't use the build-platforms parameter (old style)
     - |
+      !(
+        has(pipelineRun.spec.params) &&
+        pipelineRun.spec.params.exists(p, p.name == 'build-platforms')
+      ) &&
       has(pipelineRun.spec.pipelineSpec) &&
       has(pipelineRun.spec.pipelineSpec.tasks) &&
       pipelineRun.spec.pipelineSpec.tasks.size() > 0 ?

hack/test-tekton-kueue-config.py

@@ -590,6 +590,65 @@ def check_prerequisites(should_print: bool = True) -> Dict[str, TestConfig]:
         }
     },
 
+    "prefer-new-parameters": {
+        "name": "Only use new platform parameters when both old and new parameters exist",
+        "pipelinerun": {
+            "apiVersion": "tekton.dev/v1",
+            "kind": "PipelineRun",
+            "metadata": {
+                "name": "prefer-new-platform-parameters",
+                "namespace": "default",
+                "labels": {
+                    "pipelinesascode.tekton.dev/event-type": "push",
+                }
+            },
+            "spec": {
+                "pipelineRef": {"name": "build-pipeline"},
+                "params": [
+                    {
+                        "name": "build-platforms",
+                        "value": ["linux/amd64", "linux/arm64", "linux/s390x"]
+                    },
+                    {
+                        "name": "other-param",
+                        "value": "test"
+                    }
+                ],
+                "pipelineSpec": {
+                    "tasks": [
+                        {
+                            "name": "build-task-amd64",
+                            "params": [{"name": "PLATFORM", "value": "linux/amd64"}],
+                            "taskRef": {"name": "build-task"}
+                        },
+                        {
+                            "name": "build-task-arm64",
+                            "params": [{"name": "PLATFORM", "value": "linux/arm64"}],
+                            "taskRef": {"name": "build-task"}
+                        },
+                        {
+                            "name": "other-task",
+                            "taskRef": {"name": "other-task"}
+                        }
+                    ]
+                },
+                "workspaces": [{"name": "shared-workspace", "emptyDir": {}}]
+            }
+        },
+        "expected": {
+            "annotations": {
+                "kueue.konflux-ci.dev/requests-linux-amd64": "1",
+                "kueue.konflux-ci.dev/requests-linux-s390x": "1",
+                "kueue.konflux-ci.dev/requests-linux-arm64": "1",
+                "kueue.konflux-ci.dev/requests-aws-ip": "2",
+            },
+            "labels": {
+                "kueue.x-k8s.io/queue-name": "pipelines-queue",
+                "kueue.x-k8s.io/priority-class": "konflux-post-merge-build",
+            }
+        }
+    },
+
 }
 
 # Configuration combinations that can be applied to any PipelineRun
@@ -660,6 +719,10 @@ def check_prerequisites(should_print: bool = True) -> Dict[str, TestConfig]:
         "pipelinerun_key": "mixed_platforms_excluded_included",
         "config_key": "development"
     },
+    "prefer_new_parameters_dev": {
+        "pipelinerun_key": "prefer-new-parameters",
+        "config_key": "development"
+    },
 
     # multiplatform_old edge cases
     "multiplatform_old_no_pipelineSpecTasks": {
@@ -688,6 +751,10 @@ def check_prerequisites(should_print: bool = True) -> Dict[str, TestConfig]:
         "pipelinerun_key": "mintmaker",
         "config_key": "staging"
     },
+    "prefer_new_parameters_staging": {
+        "pipelinerun_key": "prefer-new-parameters",
+        "config_key": "staging"
+    },
 
     # Test key PipelineRuns with production config
     "multiplatform_new_production": {