Skip to content
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

Code transfer of SWISS TXT cloudstack-cloud-controller-manager to the Apache project #1

Merged
merged 59 commits into from
Sep 20, 2019

Conversation

onitake
Copy link
Contributor

@onitake onitake commented Aug 2, 2019

This merges the history of https://github.com/swisstxt/cloudstack-cloud-controller-manager into the Apache foundation project https://github.com/apache/cloudstack-kubernetes-provider and transfers custody and ownership of the code to the Apache foundation.

The original code from the Kubernetes project as well as the modifications done by @swisstxt are covered by the Apache 2.0 license, so there is no change with respect to the license conditions.

Other things that may need to be transferred or recreated:

Please advise on how to proceed on these points.

@onitake
Copy link
Contributor Author

onitake commented Aug 2, 2019

Much of the imported code are vendored dependencies.
Since we already use Go modules, I can also remove those if you prefer.

Also note that the code wasn't updated to use modules from the latest k8s release - but I'd like to postpone this until after the code is merged.

@rohityadavcloud
Copy link
Member

rohityadavcloud commented Aug 6, 2019

@onitake LGTM, can you remove/clean the vendor directory? I'm not sure if gcp/azure etc dependencies are useful? I'm OK to keep the dependencies in vendor but not if they are not used. What's the standard practice?

@onitake
Copy link
Contributor Author

onitake commented Aug 6, 2019

A very good point!

These dependencies are not useful in any way, but they were pulled in by the dependency on the k8s cloud provider code used by the controller. At the point when we forked the code, they were required because the old kube-controller code still contained all the dependencies on the internal cloud providers.

I will try to update all k8s dependencies to 1.15 and see if this removes the cloud provider stuff.

@onitake
Copy link
Contributor Author

onitake commented Aug 13, 2019

@rhtyd Dependency cleanup was harder than expected due to the incompatible versioning scheme on the k8s.io packages. Go expects semantic versions, but the tags on these repositories are not consistent with that (kubernetes-1.x.y).

go mod will still scan all transitive dependencies of all referenced packages and record their checksums, but the vendor directory now contains only packages that are actually used.
We still need to do some testing, but I think we're on track now.

@joschi36 and I are also addressing swisstxt/cloudstack-cloud-controller-manager#9 right now and we will update the documentation and example deployment shortly.

@rohityadavcloud
Copy link
Member

Thanks @onitake keep me posted when you think this is ready.

@onitake
Copy link
Contributor Author

onitake commented Aug 15, 2019

@rhtyd I think we're ready to push now.
Issue no. 9 is still open, but I think we can move along and direct the OP to the new repo for further debugging.

I built a last beta release (0.0.3), fixed some build steps and replaced the remaining references to the old swisstxt repo with the new home. The README still has a bit of copyright text at the end, please advise if we should remove or replace it.

Will you give @joschi36 and myself commit access to the new repository, or should we handle further contributions via PRs from personal repos?

@onitake
Copy link
Contributor Author

onitake commented Sep 9, 2019

@rhtyd Any update?

@rohityadavcloud
Copy link
Member

@onitake hi, I'll get back to you by next week. Currently at a conference and then travelling till Tuesday next week.

@onitake
Copy link
Contributor Author

onitake commented Sep 13, 2019

Thanks @rhtyd !

In the meantime, we fixed another bug due to the API change introduced in apache/cloudstack#3066 - our cloudstack-go version was a bit outdated.

@rohityadavcloud
Copy link
Member

@onitake thanks, let me know when you think it's ready for merge. Can you also add documentation (if not already added to the README?) how to build/use/integrate the provider?

@onitake
Copy link
Contributor Author

onitake commented Sep 18, 2019

@rhtyd I believe we're ready to merge, unless you still have legal, formal or moral objections. In particular, please advise on the copyright and author references in the README.

The README already includes instructions on compilation and use. Please take a look and try it out for yourself.

One important question is also about future contributions: Will you consider giving @joschi36 and me direct commit rights or should we handle future contributions via PRs from private repos?

@rohityadavcloud
Copy link
Member

@onitake thanks, can you check and add the Apache license 2.0 to all files (wherever possible) and perhaps remove Copyright 2016 The Kubernetes Authors. now that the repository/file are maintained by CloudStack. Ignore files in vendor and other dependencies. I think we can rebase-merge once this is done, thanks.

@onitake
Copy link
Contributor Author

onitake commented Sep 19, 2019

All right, I followed https://www.apache.org/legal/src-headers.html as closely as possible.

The source files contain the required license header, and I moved all previous copyright text into the NOTICE file, as required.

@rohityadavcloud rohityadavcloud merged commit 5d9af86 into apache:master Sep 20, 2019
@onitake onitake deleted the transfer branch September 20, 2019 09:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants