Skip to content
This repository has been archived by the owner on Oct 21, 2020. It is now read-only.

Add DigitalOcean provisioner #470

Merged
merged 4 commits into from
Dec 20, 2017
Merged

Add DigitalOcean provisioner #470

merged 4 commits into from
Dec 20, 2017

Conversation

klausenbusk
Copy link
Contributor

@klausenbusk klausenbusk commented Nov 17, 2017

This commit add a DigitalOcean provisioner and a flexvolume plugin
for DigitalOcean.


Finally, here we go :) The code has only been tested slightly on my k8s 1.8 cluster.

I'm not sure if I did the vendoring correctly (glide get github.com/digitalocean/godo + glide-vc --use-lock-file).. Edit: See commit message solved

/cc @andrewsykim

@k8s-ci-robot
Copy link
Contributor

@klausenbusk: GitHub didn't allow me to request PR reviews from the following users: andrewsykim.

Note that only kubernetes-incubator members can review this PR, and authors cannot review their own PRs.

In response to this:

This commit add a DigitalOcean provisioner and a flexvolume plugin
for DigitalOcean.

Finally, here we go :) The code has only been tested slightly on my k8s 1.8 cluster.

I'm not sure if I did the vendoring correctly (glide get github.com/digitalocean/godo + glide-vc --use-lock-file)..

/cc @andrewsykim

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.

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Nov 17, 2017
@andrewsykim
Copy link

cc @odacremolbap

@klausenbusk klausenbusk mentioned this pull request Nov 18, 2017
@klausenbusk
Copy link
Contributor Author

Ready for review, CI is green and the vendoring "issue" has been solved. The Go flexvolume plugin is maybe a big ugly, but it gets the job done..

@wongma7
Copy link
Contributor

wongma7 commented Nov 30, 2017

/lgtm

I will create quay.io/kubernetes_incubator/digitalocean-flexplugin and quay.io/kubernetes_incubator/digitalocean-provisioner and give you write permissions to them after we merge.

Thanks & sorry for the delay :)

@k8s-ci-robot k8s-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Nov 30, 2017
@klausenbusk
Copy link
Contributor Author

/lgtm

Great, I will rebase and remove the REVERT commit when I get home..

@klausenbusk
Copy link
Contributor Author

Great, I will rebase and remove the REVERT commit when I get home..

Done and ready for merge :)

@klausenbusk
Copy link
Contributor Author

@wongma7 are we waiting on anything? or maybe I'm just a bit impatient..

@klausenbusk
Copy link
Contributor Author

 - [ ] Prevent k8s from scheduling more than 5 disks to a single droplet 

Probably solvable when Topology Aware Volume Scheduling get implemented.

@msau42
Copy link
Contributor

msau42 commented Dec 11, 2017

I think for solving the max count scheduling issue, you're looking for a generalized version of kubernetes/kubernetes#53461

@klausenbusk
Copy link
Contributor Author

I think for solving the max count scheduling issue, you're looking for a generalized version of kubernetes/kubernetes#53461

Interesting (but mostly related to internal volume plugin, it seems (?)), I will play around with whatever gets implemented first. MatchUnboundPVCs seems to provide enough functionality to implement à Volume Count Predicate like the internal volume plugin.

kubernetes/kubernetes#24317 also contain a few ideas.

@msau42
Copy link
Contributor

msau42 commented Dec 11, 2017

The comments in the PR suggest an alternative using ConfigMaps to better support out of tree cloud providers.

@wongma7 wongma7 merged commit 68fd702 into kubernetes-retired:master Dec 20, 2017
@klausenbusk
Copy link
Contributor Author

Thanks @wongma7 Do you have time to create quay.io/kubernetes_incubator/digitalocean-provisioner and quay.io/kubernetes_incubator/digitalocean-flexplugin ? :)

@wongma7
Copy link
Contributor

wongma7 commented Dec 20, 2017

@klausenbusk yes, I have sent you an invite also

Still trying to sort out and document how we should do versioning/release/auto-push for the repo, atm it needs a git tag to trigger build&push but that's not ideal. will create a github issue later.

@klausenbusk
Copy link
Contributor Author

@klausenbusk yes, I have sent you an invite also

Great! I have just pushed both image as: quay.io/external_storage/digitalocean-provisioner:latest and quay.io/external_storage/digitalocean-provisioner:latest. The registry in the Makefile wasn't correct, so I have opened a PR to fix that: #520
BTW: It seems like I also forgot to add the provisioner to the Makefile, will do that later.

Still trying to sort out and document how we should do versioning/release/auto-push for the repo, atm it needs a git tag to trigger build&push but that's not ideal. will create a github issue later.

For now I'm just pushing as :latest, but it would be more ideal pushing with a version number, so backwards incompatible changes doesn't break a running cluster. Hmm...

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area/digitalocean cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm Indicates that a PR is ready to be merged. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants