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

Implement DigitalOcean Droplet FI Task #3707

Merged
merged 1 commit into from
Oct 27, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions cmd/kops/create_cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -444,6 +444,28 @@ func RunCreateCluster(f *util.Factory, out io.Writer, c *CreateClusterOptions) e
}
zoneToSubnetMap[zoneName] = subnet
}
} else if api.CloudProviderID(cluster.Spec.CloudProvider) == api.CloudProviderDO {
if len(c.Zones) > 1 {
return fmt.Errorf("digitalocean cloud provider currently only supports 1 region, expect multi-region support when digitalocean support is in beta")
}

// For DO we just pass in the region for --zones
region := c.Zones[0]
subnet := model.FindSubnet(cluster, region)

// for DO, subnets are just regions
subnetName := region

if subnet == nil {
subnet = &api.ClusterSubnetSpec{
Name: subnetName,
// region and zone are the same for DO
Region: region,
Zone: region,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm... we should be able to cope without a Zone being set, but I'm guessing something breaks ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, the FindZonesForInstanceGroup function only inserts to list if Zone is set, and so this check fails. Any objections to add a region check there?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No objections, particularly if we can scope it to DO. The root issue there is that EBS volumes on AWS / PDs on GCE are single zone, and thus we need to figure out the zone. But it looks like DO volumes are regional (wow!) so it's fine to relax that check. I would want to see the existing checks for AWS & GCE continue to work (likely by a switch statement on the cloudproviderid).

}
cluster.Spec.Subnets = append(cluster.Spec.Subnets, *subnet)
}
zoneToSubnetMap[region] = subnet
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd be in favor of a region flag in future, but we can run with this...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed!

} else {
for _, zoneName := range allZones.List() {
// We create default subnets named the same as the zones
Expand Down
1 change: 1 addition & 0 deletions hack/.packages
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ k8s.io/kops/pkg/model
k8s.io/kops/pkg/model/awsmodel
k8s.io/kops/pkg/model/components
k8s.io/kops/pkg/model/defaults
k8s.io/kops/pkg/model/domodel
k8s.io/kops/pkg/model/gcemodel
k8s.io/kops/pkg/model/iam
k8s.io/kops/pkg/model/resources
Expand Down
9 changes: 6 additions & 3 deletions pkg/apis/kops/validation/legacy.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,9 +80,6 @@ func ValidateCluster(c *kops.Cluster, strict bool) *field.Error {
switch kops.CloudProviderID(c.Spec.CloudProvider) {
case kops.CloudProviderBareMetal:
requiresSubnets = false
if c.Spec.NetworkCIDR != "" {
return field.Invalid(fieldSpec.Child("NetworkCIDR"), c.Spec.NetworkCIDR, "NetworkCIDR should not be set on bare metal")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

}
requiresNetworkCIDR = false
if c.Spec.NetworkCIDR != "" {
return field.Invalid(fieldSpec.Child("NetworkCIDR"), c.Spec.NetworkCIDR, "NetworkCIDR should not be set on bare metal")
Expand All @@ -96,6 +93,12 @@ func ValidateCluster(c *kops.Cluster, strict bool) *field.Error {
requiresSubnetCIDR = false

case kops.CloudProviderDO:
requiresSubnets = false
requiresSubnetCIDR = false
requiresNetworkCIDR = false
if c.Spec.NetworkCIDR != "" {
return field.Invalid(fieldSpec.Child("NetworkCIDR"), c.Spec.NetworkCIDR, "NetworkCIDR should not be set on DigitalOcean")
}
case kops.CloudProviderAWS:
case kops.CloudProviderVSphere:

Expand Down
24 changes: 24 additions & 0 deletions pkg/model/domodel/context.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
/*
Copyright 2016 The Kubernetes Authors.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package domodel

import "k8s.io/kops/pkg/model"

// DigitalOcean Model Context
type DOModelContext struct {
*model.KopsModelContext
}
71 changes: 71 additions & 0 deletions pkg/model/domodel/droplets.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
/*
Copyright 2016 The Kubernetes Authors.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package domodel

import (
"strings"

"k8s.io/kops/pkg/model"
"k8s.io/kops/upup/pkg/fi"
"k8s.io/kops/upup/pkg/fi/cloudup/dotasks"
)

// DropletBuilder configures droplets for the cluster
type DropletBuilder struct {
*DOModelContext

BootstrapScript *model.BootstrapScript
Lifecycle *fi.Lifecycle
}

var _ fi.ModelBuilder = &DropletBuilder{}

func (d *DropletBuilder) Build(c *fi.ModelBuilderContext) error {
sshKeyName, err := d.SSHKeyName()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could tweak SSHKeyName to return something you don't have to parse if that would help...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Noted! Didn't want to creep too much into other packages but this seems like a good future optimization!

if err != nil {
return err
}

splitSSHKeyName := strings.Split(sshKeyName, "-")
sshKeyFingerPrint := splitSSHKeyName[len(splitSSHKeyName)-1]

// replace "." with "-" since DO API does not accept "."
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GCE has a similar issue, and has SafeClusterName - you might want to create a similar function for DO, depending on how often you end up using it!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ahh, that's useful noted!

clusterTag := "KubernetesCluster:" + strings.Replace(d.ClusterName(), ".", "-", -1)

for _, ig := range d.InstanceGroups {
name := d.AutoscalingGroupName(ig)

var droplet dotasks.Droplet
droplet.Name = fi.String(name)
// during alpha support we only allow 1 region
// validation for only 1 region is done at this point
droplet.Region = fi.String(d.Cluster.Spec.Subnets[0].Region)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI, single-region is actually pretty deep in the k8s design today

droplet.Size = fi.String(ig.Spec.MachineType)
droplet.Image = fi.String(ig.Spec.Image)
droplet.SSHKey = fi.String(sshKeyFingerPrint)
droplet.Tags = []string{clusterTag}

userData, err := d.BootstrapScript.ResourceNodeUp(ig, &d.Cluster.Spec)
if err != nil {
return err
}
droplet.UserData = userData

c.AddTask(&droplet)
}
return nil
}
4 changes: 4 additions & 0 deletions pkg/resources/digitalocean/cloud.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,10 @@ func (c *Cloud) Volumes() godo.StorageService {
return c.Client.Storage
}

func (c *Cloud) Droplets() godo.DropletsService {
return c.Client.Droplets
}

// FindVPCInfo is not implemented, it's only here to satisfy the fi.Cloud interface
func (c *Cloud) FindVPCInfo(id string) (*fi.VPCInfo, error) {
return nil, errors.New("not implemented")
Expand Down
64 changes: 10 additions & 54 deletions pkg/resources/digitalocean/dns/dns.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,6 @@ package dns

import (
"fmt"
"io/ioutil"
"net/http"

"github.com/digitalocean/godo"
"github.com/digitalocean/godo/context"
Expand Down Expand Up @@ -311,7 +309,7 @@ func (r *resourceRecordChangeset) Apply() error {

err := deleteRecord(r.client, r.zone.Name(), desiredRecord.ID)
if err != nil {
return err
return fmt.Errorf("failed to delete record: %v", err)
}
}

Expand All @@ -321,7 +319,7 @@ func (r *resourceRecordChangeset) Apply() error {
if len(r.upserts) > 0 {
records, err := getRecords(r.client, r.zone.Name())
if err != nil {
return err
return fmt.Errorf("failed to get records: %v", err)
}

for _, record := range r.upserts {
Expand All @@ -346,7 +344,7 @@ func (r *resourceRecordChangeset) Apply() error {
}
err := editRecord(r.client, r.zone.Name(), desiredRecord.ID, domainEditRequest)
if err != nil {
return err
return fmt.Errorf("failed to edit record: %v", err)
}
}

Expand Down Expand Up @@ -374,112 +372,70 @@ func (r *resourceRecordChangeset) ResourceRecordSets() dnsprovider.ResourceRecor
// listDomains returns a list of godo.Domain
func listDomains(c *godo.Client) ([]godo.Domain, error) {
// TODO (andrewsykim): pagination in ListOptions
domains, resp, err := c.Domains.List(context.TODO(), &godo.ListOptions{})
domains, _, err := c.Domains.List(context.TODO(), &godo.ListOptions{})
if err != nil {
return nil, fmt.Errorf("failed to list domains: %v", err)
}

if err = handleResponse(resp); err != nil {
return nil, err
}

return domains, err
}

// createDomain creates a domain provided godo.DomainCreateRequest
func createDomain(c *godo.Client, createRequest *godo.DomainCreateRequest) (*godo.Domain, error) {
domain, resp, err := c.Domains.Create(context.TODO(), createRequest)
domain, _, err := c.Domains.Create(context.TODO(), createRequest)
if err != nil {
return nil, err
}

if err = handleResponse(resp); err != nil {
return nil, err
}

return domain, nil
}

// deleteDomain deletes a domain given its name
func deleteDomain(c *godo.Client, name string) error {
resp, err := c.Domains.Delete(context.TODO(), name)
_, err := c.Domains.Delete(context.TODO(), name)
if err != nil {
return err
}

if err = handleResponse(resp); err != nil {
return err
}

return nil
}

// getRecords returns a list of godo.DomainRecord given a zone name
func getRecords(c *godo.Client, zoneName string) ([]godo.DomainRecord, error) {
records, resp, err := c.Domains.Records(context.TODO(), zoneName, &godo.ListOptions{})
records, _, err := c.Domains.Records(context.TODO(), zoneName, &godo.ListOptions{})
if err != nil {
return nil, err
}

if err = handleResponse(resp); err != nil {
return nil, err
}

return records, nil
}

// createRecord creates a record given an associated zone and a godo.DomainRecordEditRequest
func createRecord(c *godo.Client, zoneName string, createRequest *godo.DomainRecordEditRequest) error {
_, resp, err := c.Domains.CreateRecord(context.TODO(), zoneName, createRequest)
_, _, err := c.Domains.CreateRecord(context.TODO(), zoneName, createRequest)
if err != nil {
return fmt.Errorf("error applying changeset: %v", err)
}

if err = handleResponse(resp); err != nil {
return err
}

return nil
}

// editRecord edits a record given an associated ozone and a godo.DomainRecordEditRequest
func editRecord(c *godo.Client, zoneName string, recordID int, editRequest *godo.DomainRecordEditRequest) error {
_, resp, err := c.Domains.EditRecord(context.TODO(), zoneName, recordID, editRequest)
_, _, err := c.Domains.EditRecord(context.TODO(), zoneName, recordID, editRequest)
if err != nil {
return fmt.Errorf("error applying changeset: %v", err)
}

if err = handleResponse(resp); err != nil {
return err
}

return nil
}

// deleteRecord deletes a record given an associated zone and a record ID
func deleteRecord(c *godo.Client, zoneName string, recordID int) error {
resp, err := c.Domains.DeleteRecord(context.TODO(), zoneName, recordID)
_, err := c.Domains.DeleteRecord(context.TODO(), zoneName, recordID)
if err != nil {
return fmt.Errorf("error applying changeset: %v", err)
}

if err = handleResponse(resp); err != nil {
return err
}

return nil
}

func handleResponse(resp *godo.Response) error {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I take it that godo now handles this for you?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes! That was actually always the case, just only recently found out :)

if resp.StatusCode != http.StatusOK {
respData, err := ioutil.ReadAll(resp.Body)
if err != nil {
return fmt.Errorf("error reading response body: %v", err)
}

return fmt.Errorf("received non 200 status code: %d from api: %v",
resp.StatusCode, string(respData))
}

return nil
}
Loading