tektoncd
fix: merge podTemplates filed more specific, such as tolerations, nodeselector and so on.
Changes
fixes: #7127
Background:
In the original implementation, template replacement is simply replacing pipelineRun.spec.podTemplate with pipelineRun.spec.taskRunSpecs[].taskPodTemplate,the specific description can be seen here:Issue #6846.
To solve the problem #6846. In pr #6862, the function MergePodTemplateWithDefault is called to merge podtemplate, In this function, two parameters named tpl and defaultTpl are passed in. It only merges volumes and envs, the judgment logic of the remaining fields is if the same field is set on both templates, the value from tpl will overwrite the value from defaultTpl, which leads to the occurrence of problem #7127
Solution:
Here I created a new function named ExpandedMergePodTemplateWithDefault, which can merge fields such as NodeSelector, Tolerations ,ImagePullSecrets and so on. If there is a conflict when these fields are merged, we will use the value in the field corresponding to the incoming parameter with high priority (similar to the implementation of the original function, that is, the second default of the incoming parameter is higher priority). The reason why it is not modified on the original function is that the original function is referenced in multiple places. If it is modified directly, it may cause unknown problems.
In this pr, I replaced the merge function called in function GetTaskRunSpec with my newly defined function and wrote a test function for my newly defined function and added multiple test cases to the original TestPipelineRun_GetTaskRunSpec test function
/kind bug
Submitter Checklist
As the author of this PR, please check off the items in this checklist:
- [ ] Has Docs if any changes are user facing, including updates to minimum requirements e.g. Kubernetes version bumps
- [x] Has Tests included if any functionality added or changed
- [x] pre-commit Passed
- [x] Follows the commit message standard
- [x] Meets the Tekton contributor standards (including functionality, content, code)
- [x] Has a kind label. You can add one by adding a comment on this PR that contains
/kind <type>. Valid types are bug, cleanup, design, documentation, feature, flake, misc, question, tep - [x] Release notes block below has been updated with any user facing changes (API changes, bug fixes, changes requiring upgrade notices or deprecation warnings). See some examples of good release notes.
- [ ] Release notes contains the string "action required" if the change requires additional action from users switching to the new release
Release Notes
PodTemplate filed such as tolerations, nodeselector will be merged between pipelineRun.spec.podTemplate and pipelineRun.spec.taskRunSpecs[]
Hi @Ma-YuXin. Thanks for your PR.
I'm waiting for a tektoncd member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.
Once the patch is verified, the new status will be reflected by the ok-to-test label.
I understand the commands that are listed here.
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.
The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage-df to re-run this coverage report
| File | Old Coverage | New Coverage | Delta |
|---|---|---|---|
| pkg/apis/pipeline/pod/template.go | 75.3% | 80.7% | 5.4 |
The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report
| File | Old Coverage | New Coverage | Delta |
|---|---|---|---|
| pkg/apis/pipeline/pod/template.go | 75.3% | 80.7% | 5.4 |
The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report
| File | Old Coverage | New Coverage | Delta |
|---|---|---|---|
| pkg/apis/pipeline/pod/template.go | 75.3% | 80.7% | 5.4 |
The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage-df to re-run this coverage report
| File | Old Coverage | New Coverage | Delta |
|---|---|---|---|
| pkg/apis/pipeline/pod/template.go | 75.3% | 80.7% | 5.4 |
The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report
| File | Old Coverage | New Coverage | Delta |
|---|---|---|---|
| pkg/apis/pipeline/pod/template.go | 75.3% | 80.7% | 5.4 |
The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage-df to re-run this coverage report
| File | Old Coverage | New Coverage | Delta |
|---|---|---|---|
| pkg/apis/pipeline/pod/template.go | 75.3% | 80.7% | 5.4 |
The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report
| File | Old Coverage | New Coverage | Delta |
|---|---|---|---|
| pkg/apis/pipeline/pod/template.go | 75.3% | 80.7% | 5.4 |
The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage-df to re-run this coverage report
| File | Old Coverage | New Coverage | Delta |
|---|---|---|---|
| pkg/apis/pipeline/pod/template.go | 75.3% | 80.7% | 5.4 |
[APPROVALNOTIFIER] This PR is APPROVED
This pull-request has been approved by: chitrangpatel, vdemeester
The full list of commands accepted by this bot can be found here.
The pull request process is described here
- ~~pkg/apis/OWNERS~~ [chitrangpatel,vdemeester]
Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment
@afrittoli's commend still needs to be addressed. Lets move this to the next release unless it can be done now.
