Add --use-workspace-defaults
- tkn task start always asks for the configuration of the workspaces, even if you have:
default-task-run-workspace-binding: |
emptyDir: {}
In the config-defaults.
If there is another way to achieve this, please let me know.
Add a new flag `--use-workspace-default` to use the configured default workspace for the task run
The committers are authorized under a signed CLA.
- :white_check_mark: Aaron France (0de0504a6e5a7bbdb46db9e63c1458ff594ae96b)
Hi @AeroNotix. 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.
/assign @danielhelfand
if you set UseParamDefaults as false, it should use the workspace defaults? If this doesn't happen like this it would be a bug isnt it ?
(also if we go forward on this please consider adding a test)
/ok-to-test
The following is the coverage report on the affected files.
Say /test pull-tekton-cli-go-coverage to re-run this coverage report
| File | Old Coverage | New Coverage | Delta |
|---|---|---|---|
| pkg/cmd/task/start.go | 80.9% | 81.0% | 0.1 |
@chmouel no, using just --use-param-defaults=false without what I added, still prompts for the workspace configuration.
/retest
@danielhelfand can I get some help moving this along, please? What do I need to do/answer/change/discover/debug etc.
The CI failure looks to be a documentantion related thing.. what do I need to fix?
The committers are authorized under a signed CLA.
- :white_check_mark: Aaron France (0de0504a6e5a7bbdb46db9e63c1458ff594ae96b, be37447e32a089068c135db111d62dcda361c75b)
The following is the coverage report on the affected files.
Say /test pull-tekton-cli-go-coverage to re-run this coverage report
| File | Old Coverage | New Coverage | Delta |
|---|---|---|---|
| pkg/cmd/task/start.go | 81.0% | 81.1% | 0.1 |
@pradeepitm12 @vinamra28 @chmouel I think this we can close as #1465 is merged
@pradeepitm12 @vinamra28 @chmouel I think this we can close as #1465 is merged
@piyush-garg I think we can still consider this as this is solving a different problem. There is a field in configmap as config-defaults where we can set the value of key default-task-run-workspace-binding. If I am not wrong then this PR is to handle this scenario :sweat_smile:
#1465 is just to handle the binding of optional workspaces. This PR is to handle all the workspaces. It's similar to --use-params-default flag
@pradeepitm12 @vinamra28 @chmouel I think this we can close as #1465 is merged
@piyush-garg I think we can still consider this as this is solving a different problem. There is a field in configmap as
config-defaultswhere we can set the value of keydefault-task-run-workspace-binding. If I am not wrong then this PR is to handle this scenario sweat_smile
Aah ok, this is for skipping all and #1465 is for skipping optional
/retest
@AeroNotix can you please rebase with upstream/main and try to repush ? Also could you please squash the commits?
You can squash the commits when you merge, by doing a squash commit. I'll rebase tho.
You can squash the commits when you merge, by doing a squash commit. I'll rebase tho.
@AeroNotix there is a bot in place which performs the merging so won't be able to do that. Squashing of the commits needs to be done from your end 😅
Interesting so you use a bot to do some human's work but leave trivially automatable work to the humans. Quite an interesting workflow from a product which is all about automation :)
I'll squash ;) 👍🏻
The following is the coverage report on the affected files.
Say /test pull-tekton-cli-go-coverage to re-run this coverage report
| File | Old Coverage | New Coverage | Delta |
|---|---|---|---|
| pkg/cmd/task/start.go | 81.0% | 81.1% | 0.1 |
@vinamra28 do I need to do something with that failure?
@vinamra28 do I need to do something with that failure?
@AeroNotix could you please run make generated. It will generate some man pages that you need to commit and push. Also can you please do the same for ClusterTask and Pipeline as well?
Also can you please do the same for ClusterTask and Pipeline as well?
What do you mean? Add the same flag?
The following is the coverage report on the affected files.
Say /test pull-tekton-cli-go-coverage to re-run this coverage report
| File | Old Coverage | New Coverage | Delta |
|---|---|---|---|
| pkg/cmd/task/start.go | 81.0% | 81.1% | 0.1 |
Added the same flags for cluster/pipeline.
The following is the coverage report on the affected files.
Say /test pull-tekton-cli-go-coverage to re-run this coverage report
| File | Old Coverage | New Coverage | Delta |
|---|---|---|---|
| pkg/cmd/clustertask/start.go | 80.2% | 80.3% | 0.1 |
| pkg/cmd/pipeline/start.go | 66.3% | 66.4% | 0.1 |
| pkg/cmd/task/start.go | 81.0% | 81.1% | 0.1 |
/retest
@vinamra28 good to merge now?
Added a release-note block, it should be good to merge now /cc @tektoncd/cli-maintainers
Can we please add some tests for this?