-
Notifications
You must be signed in to change notification settings - Fork 71
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
move cloud-providers to top go module #1719
move cloud-providers to top go module #1719
Conversation
2ae8efb
to
8db87ac
Compare
07cf80e
to
530594d
Compare
cb06f0c
to
9a65e55
Compare
9a65e55
to
530594d
Compare
The pr are ready to review.
it seems that it use the workflow file from main branch, it should use the podvm build yaml from this PR. |
c3b4ca9
to
530594d
Compare
So we maybe need merge this pr first without test the |
@liudalibj - so the problem is that the workflow files comes from the |
@stevenhorsman base on this comment, I move some common scripts(check lint...) to the root hack folder, and also create new |
And verified that the |
40e691b
to
87647e0
Compare
|
cfb0874
to
973f9f0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I ran simple smoke tests using a CAA image built from this branch (launch + delete peerpods), did not observe any problems.
Reviewed the code as far as feasible: LGTM, great work!
Sorry if this comes across as bike shedding, but do you think some of the docs should remain in the root of the repo (under a new/old docs directory as they relate to the project as a whole and are more tricky to find after the move. I'm pariticularly thinking of the architecture and release process docs as they cover all the components, not just the CAA. I think the golang-fedora image build is also not specific to the CAA? https://github.com/liudalibj/cloud-api-adaptor/blob/toplevel-cloud-module/src/cloud-api-adaptor/Dockerfile (it might even just be used in one of the controllers, so be a candidate to move into their directories, or be renamed for clarity I guess? |
568258d
to
d99e8e8
Compare
Good points, I updated the pr:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a couple of minor things I spotted scanning through the commits. Obviously there are too many changes to be confident for human review, so I'll try and run through a few builds as well.
d99e8e8
to
056e480
Compare
Oh - I think this is broken in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this (once rebased) is good enough to merge. Thanks for all the hard-work on this DaLi.
Full disclaimer - I tried to build it all locally and run the e2e tests with libvirt, but hit a problem where the CAA couldn't connect to the APF, but I've been having a bunch of issues locally with kcli and libvirt in other tests and the e2e still passes and you've linked it working for you, so I don't want to hold this up further.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, fantastic! thank you @liudalibj
Create new go module, provider, to hold generic provider code. This can be used between the cloud-api-adaptor and the peerpod-ctrl. - Extract out cloudinit code for the cloud-api-adaptor pkg - Extract out various utility functions that should live outside of the cloud-api-adaptor package - Extract out the generic instance related types - Make the cloudTable reusable between cloud-api-adaptor and the peerpod-ctrl Signed-off-by: James Tumber <james.tumber@ibm.com> Co-authored-by: Da Li Liu <liudali@cn.ibm.com>
- create cloud-providers go mod Signed-off-by: Da Li Liu <liudali@cn.ibm.com>
- move all function source codes to src folder - keep depency go modules in the single repo Signed-off-by: Da Li Liu <liudali@cn.ibm.com>
- caa local build - peerpod-ctrl local build - csi-wrapper-localbuild - test codes local build Signed-off-by: Da Li Liu <liudali@cn.ibm.com>
- golang-fedora - test-azure-e2e - peerpod-ctrl and peerpodconfig-ctrl - csi_wrapper_images - lint and links - podvm - webhook - release - publish on push - test-images Signed-off-by: Da Li Liu <liudali@cn.ibm.com>
- add a new hack/release-helper.sh script to help generate tags command - update release document review: address review comments - keep build-golang-fedora on the root of repo - keep common docs architecture and Release-Process at the root docs Signed-off-by: Da Li Liu <liudali@cn.ibm.com>
056e480
to
28072dc
Compare
Remove cyclic dependency among cloud-api-adaptor and peerpod-ctrl.
Move cloud-providers to a new go mod and re-structure all the source codes as:
And updated all workflow yamls to reflect new directory structure.
part of #1122
Signed-off-by: Da Li Liu liudali@cn.ibm.com
Co-authored-by: James Tumber james.tumber@ibm.com