Closed zhangtbj closed 4 years ago
Hi @zhangtbj. Thanks for your PR.
I'm waiting for a knative 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.
/ok-to-test
All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.
We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent.
in this pull request.
Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla
label to yes
(if enabled on your project).
ℹ️ Googlers: Go here for more info.
/retest
Hi @maximilien and @rhuss ,
Thanks for your kind review and sorry I took this plugin for a long time.
I have fixed all issues which you mentioned in the PR and:
kn-hello
example plugin structurePlease help review again. Thanks!
Below is an example output, enjoy:
There are 2 service(s) in source default namespace:
Name Current Revision Ready
echo echo-dlmxf-2 true
|- Revision echo-dlmxf-2 ( Generation: 2 , Ready: true )
|- Revision echo-vqhwd-1 ( Generation: 1 , Ready: true )
Name Current Revision Ready
helloworld helloworld-srwvr-1 true
|- Revision helloworld-srwvr-1 ( Generation: 1 , Ready: true )
[Before migration in destination cluster]
There are 0 service(s) in destination default namespace:
Now migrate all Knative service resources:
From the source default namespace of cluster /Users/jordan/.bluemix/plugins/container-service/clusters/kubecon2019/kube-config-dal10-kubecon2019.yml
To the destination default namespace of cluster /Users/jordan/.bluemix/plugins/container-service/clusters/source_to_image/kube-config-dal10-source_to_image.yml
Namespace default already exists in destination cluster
Migrated service echo Successfully
Replace revision echo-dlmxf-2 to generation 2 successfully
Migrated revision echo-vqhwd-1 successfully
Migrated service helloworld Successfully
Replace revision helloworld-srwvr-1 to generation 1 successfully
[After migration in destination cluster]
There are 2 service(s) in destination default namespace:
Name Current Revision Ready
echo echo-dlmxf-2 false
|- Revision echo-dlmxf-2 ( Generation: 2 , Ready: true )
|- Revision echo-vqhwd-1 ( Generation: 1 , Ready: true )
Name Current Revision Ready
helloworld false
|- Revision helloworld-srwvr-1 ( Generation: 1 , Ready: false )
Migrate without --delete option, skip deleting Knative resource in source cluster
Thanks!
Cool, I hope I will get to a review this week (but we have RH summit time, so a lot of distractions ;-)
Sure thing. Take your time, Roland :)
Any progress on this. Ready for review?
Yes Max, it is already ready for review and fixed above issues
Sorry that I didn't make it until today (and now I'm off until the end of the week for PTO). Please feel free to review it (@maximilien @dsimansk @navidshaikh @daisy-ycguo ) but from a first look I think its good to merge and we can possibly refine it later, too.
Yes, please and it is better if we can agree and merge then keep improving it.
Thanks!
ok, lets merge it then now before I'm out ;-)
/lgtm
Hi @rhuss ,
I just found we have a main README.md in the root folder to show all plugins. Then I did two changes in this PR:
Add kn-migration
in the list of README: https://github.com/zhangtbj/client-contrib/blob/kn-migrate/README.md
Add OWNERs file in the plugins folder to as other plugins: https://github.com/zhangtbj/client-contrib/blob/kn-migrate/plugins/migration/OWNERS
Other codes are the same as before.
Thanks!
+1
/approve /lgtm
BTW, can this PR be merged with do-not-merge/invalid-owners-file
lable?
I asked the admin to add you to the GitHub Knative org. Only then we can add you to the OWNERS file.
ok, got it.
Thank you! :)
/verify-owners
/retest
/retest
Hi @rhuss ,
The pull-knative-* test still fail...., can you please help take a look again.
Thanks!
/retest
as soon as #35 is merged, then the test should be ok again. You can give #35 a quick review and a /lgtm
if it looks good to me, to get the PR merged.
Hi @rhuss ,
Yes, I reviewed and looks to me.
I added /lgtm
Thanks!
/retest
@zhangtbj: The following test failed, say /retest
to rerun all failed tests:
Test name | Commit | Details | Rerun command |
---|---|---|---|
pull-knative-client-contrib-go-coverage | b16585975a8b3c467d007e2f3a0318418986a8dc | link | /test pull-knative-client-contrib-go-coverage |
Full PR test history. Your PR dashboard.
@zhangtbj the location of the test infra scripts has changed, therefor this error:
source /home/prow/go/src/knative.dev/client-contrib/plugins/migration/test/../../../test-infra/scripts/presubmit-tests.sh
/home/prow/go/src/knative.dev/client-contrib/plugins/migration/test/presubmit-tests.sh: line 29: /home/prow/go/src/knative.dev/client-contrib/plugins/migration/test/../../../test-infra/scripts/presubmit-tests.sh: No such file or directory
++ main --build-tests
Could you please adjust the path to scripts/test-infra/presubmit-tests.sh
? (the directory name levels have changed)
Hi @rhuss ,
I have fixed the script, the test passed! :)
Thanks !
/approve /lgtm
[APPROVALNOTIFIER] This PR is APPROVED
This pull-request has been approved by: rhuss, zhangtbj
The full list of commands accepted by this bot can be found here.
The pull request process is described here
Hi All,
This kn-migration plugin is designed to help admin migrate Knative services from source cluster to destination cluster
The introduction is in the plugin README