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.

Sign up for GitHub

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

Jump to bottom

Add namespace to cluster resource. #1255

Merged

tekton-robot merged 3 commits into tektoncd:master from cappyzawa:add-namespace-to-cluster

Sep 5, 2019

+42 −14

Contributor

cappyzawa commented Aug 30, 2019

Signed-off-by: cappyzawa [email protected]

Changes

I expect this to reduce args passed to kubectl apply.

Submitter Checklist

These are the criteria that every PR should meet, please check them off as you
review them:

Includes tests (if functionality changed/added)
Includes docs (if user facing)
Commit messages follow commit message best practices

See the contribution guide for more details.

Double check this list of stuff that's easy to miss:

If you are adding a new binary/image to the cmd dir, please update
the release Task and TaskRun to build and release this image

Reviewer Notes

If API changes
are included, additive changes
must be approved by at least two OWNERS
and backwards incompatible changes
must be approved by more than 50% of the OWNERS,
and they must first be added
in a backwards compatible way.

Release Notes

Enable to use `namespace` in ClusterResource params.

tekton-robot requested review from dibyom and vdemeester

August 30, 2019 18:05

googlebot added the cla: yes label

tekton-robot added size/M needs-ok-to-test labels

Collaborator

tekton-robot commented Aug 30, 2019

Hi @cappyzawa. 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.


          add namespace to cluster resource.

a10b3c4

I expect this to reduce args passed to `kubectl apply`.

Signed-off-by: cappyzawa <[email protected]>

cappyzawa force-pushed the add-namespace-to-cluster branch from f700c21 to a10b3c4 Compare

September 1, 2019 08:48

Contributor Author

cappyzawa commented Sep 4, 2019

/assign @sbwsg

tekton-robot assigned ghost

ghost reviewed

View reviewed changes

docs/resources.md Outdated

@@ @@ -356,6 +356,7 @@ The Cluster resource has the following parameters: @@
               -   `url` (required): Host url of the master node
               -   `username` (required): the user with access to the cluster
               -   `password`: to be used for clusters with basic auth
+              -   `namespace`: The namespace to be given to the target namespace

ghost Sep 4, 2019

Suggest changing this to The namespace to target in the cluster

Contributor Author

cappyzawa Sep 4, 2019

Thanks for your suggestion!

cmd/kubeconfigwriter/main.go Show resolved Hide resolved


          fix based on tektoncd#1255 (comment)

1745d82

Signed-off-by: cappyzawa <[email protected]>

ghost commented Sep 4, 2019

/ok-to-test

tekton-robot added ok-to-test and removed needs-ok-to-test labels

ghost commented Sep 4, 2019

/lgtm

tekton-robot added the lgtm label

Collaborator

tekton-robot commented Sep 4, 2019

The following is the coverage report on pkg/.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/apis/pipeline/v1alpha1/cluster_resource.go	71.1%	71.8%	0.7

Contributor Author

cappyzawa commented Sep 4, 2019

/test tekton-pipeline-unit-tests

tekton-robot removed the lgtm label

Collaborator

tekton-robot commented Sep 4, 2019

The following is the coverage report on pkg/.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/apis/pipeline/v1alpha1/cluster_resource.go	71.1%	71.8%	0.7

cappyzawa force-pushed the add-namespace-to-cluster branch from 22c21d1 to 716daf5 Compare

September 4, 2019 18:22

Collaborator

tekton-robot commented Sep 4, 2019

The following is the coverage report on pkg/.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/apis/pipeline/v1alpha1/cluster_resource.go	71.1%	71.8%	0.7

tekton-robot added size/L and removed size/M labels

cappyzawa force-pushed the add-namespace-to-cluster branch from fe92e4d to 46c8dba Compare

September 4, 2019 18:42

Collaborator

tekton-robot commented Sep 4, 2019

The following is the coverage report on pkg/.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/apis/pipeline/v1alpha1/cluster_resource.go	71.1%	71.8%	0.7

Collaborator

tekton-robot commented Sep 4, 2019

The following is the coverage report on pkg/.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/apis/pipeline/v1alpha1/cluster_resource.go	71.1%	71.8%	0.7

cappyzawa force-pushed the add-namespace-to-cluster branch from 46c8dba to fd71870 Compare

September 4, 2019 18:50

tekton-robot removed the size/L label

tekton-robot added the size/M label

Collaborator

tekton-robot commented Sep 4, 2019

The following is the coverage report on pkg/.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/apis/pipeline/v1alpha1/cluster_resource.go	71.1%	71.8%	0.7


          add namespace to GetDownloadSteps

eceadc4

Signed-off-by: cappyzawa <[email protected]>

add namespace as cluser3's parameter to input_resource_test

Signed-off-by: cappyzawa <[email protected]>

cappyzawa force-pushed the add-namespace-to-cluster branch from fd71870 to eceadc4 Compare

September 4, 2019 18:54

Collaborator

tekton-robot commented Sep 4, 2019

The following is the coverage report on pkg/.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/apis/pipeline/v1alpha1/cluster_resource.go	71.1%	71.8%	0.7

Contributor Author

cappyzawa commented Sep 4, 2019

@sbwsg I also added namaspace param to some test cases.
Could you review again if you have time, please:pray:

ghost commented Sep 4, 2019

/lgtm

tekton-robot added the lgtm label

dibyom approved these changes

View reviewed changes

Member

dibyom left a comment

/approve

Thanks @cappyzawa

Collaborator

tekton-robot commented Sep 5, 2019

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cappyzawa, dibyom

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [dibyom]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

tekton-robot added the approved label

tekton-robot merged commit 0d175f2 into tektoncd:master

tekton-robot pushed a commit that referenced this pull request


          fix based on #1255 (comment)

840d10a

Signed-off-by: cappyzawa <[email protected]>

cappyzawa deleted the add-namespace-to-cluster branch

September 22, 2019 16:44

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved cla: yes lgtm ok-to-test size/M