Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Configurable displayName for a Matrix TaskRun #7082

Closed
AverageMarcus opened this issue Aug 31, 2023 · 7 comments · Fixed by #7683
Closed

Configurable displayName for a Matrix TaskRun #7082

AverageMarcus opened this issue Aug 31, 2023 · 7 comments · Fixed by #7683
Labels
kind/feature Categorizes issue or PR as related to a new feature.

Comments

@AverageMarcus
Copy link
Contributor

Feature request

A new field, that supports variable substitution, that allows specifying a display name to use for the Task in TaskRuns created from a Matrix.

Use case

TaskRuns created from a Pipeline Matrix all have the same Task name which requires checking each manually to find a specific combination.
Screenshot 2023-08-31 at 9 26 51 am

I'd like to be able to configure the displayName property dynamically for each of the variations to make it easier to see which variation has passed and failed without needing to check all.
E.g.
Screenshot 2023-08-31 at 9 29 11 am

The field would need to be able to reference the variant parameters to be able to handle this, and optionally all the tasks params.

Possible approach:

- name: run-tests
  matrix:
    displayName: "Run Tests - $(params.VARIANT)"
    params:
      - name: VARIANT
        value: $(tasks.calculate-vars.results.variations)
  taskRef:
    name: run-tests-task

I would expect a matrix displayName to override any other displayName if one is provided.

@AverageMarcus AverageMarcus added the kind/feature Categorizes issue or PR as related to a new feature. label Aug 31, 2023
@dev-wealthpilot
Copy link

would be great if variable substitution works on all displayName fields, not just in the matrix runs.

@AverageMarcus
Copy link
Contributor Author

would be great if variable substitution works on all displayName fields, not just in the matrix runs.

+1

Maybe worth opening that as a specific feature request issue?

@pritidesai
Copy link
Member

Please check my response at #7273 (comment)

Since PR #7273 does not solve for matrix and this proposed solution is an API change, we will need a TEP for this. Are you interested/available to work on a proposal?

@AverageMarcus
Copy link
Contributor Author

I'd be happy to help out with a TEP but I'm unlikely to have much free time for it before the end of the year. I've never done one before, I'll take a look through the TEP process and try and get one started.

@pritidesai
Copy link
Member

From @AlanGreene:

Having displayName under matrix seems like the best option right now. We'd need to clearly document that pipelineTask.matrix.displayName takes precedence if pipelineTask.displayName is also set, and maybe produce a warning in this case?

As discussed on Slack, ideally the resolved displayName with param / results references replaced would be made available somewhere for all consuming clients (Dashboard, tkn, OpenShift console, etc.) so they don't have to duplicate the effort of resolving it themselves. This could potentially be in pipelineRun.status.childReferences or similar.


I was trying to remember what was the reason behind not implementing displayName as a label in taskRun and @jerop has listed the reasoning here:

tektoncd/community#593 (comment)

With the documented limitations, it will be best to introduce this in childReferences in addition to pipelineTaskName:

      "status": {
        "childReferences": [
          {
            "apiVersion": "tekton.dev/v1",
            "kind": "TaskRun",
            "name": "matrixed-pr-gp2mz-platforms-and-browsers-1",
            "pipelineTaskName": "platforms-and-browsers"
          },
          {
            "apiVersion": "tekton.dev/v1",
            "kind": "TaskRun",
            "name": "matrixed-pr-gp2mz-platforms-and-browsers-2",
            "pipelineTaskName": "platforms-and-browsers"
          },
          {
            "apiVersion": "tekton.dev/v1",
            "kind": "TaskRun",
            "name": "matrixed-pr-gp2mz-platforms-and-browsers-3",
            "pipelineTaskName": "platforms-and-browsers"
          },

@AlanGreene
Copy link
Member

@pritidesai Should we also make matrix.include[].name available in the same way? i.e. in status.childReferences.

This would avoid the need for clients to try matching the TaskRuns back to the definition (based on the param values used by the TaskRun?) to find the optional name. I don't see another way to achieve this with the current behaviour and output, happy to learn if there's something I've missed.

I don't see anything in the docs explaining the purpose or intention behind the name field, it's only present in the examples but never explained. It's not clear to me if it was intended to be a display name for this explicit combination, or if it's used elsewhere for another reason: https://tekton.dev/docs/pipelines/matrix/#explicit-combinations

@pritidesai
Copy link
Member

@AverageMarcus @dev-wealthpilot @AlanGreene please help review the proposal - tektoncd/community#1122 Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants