Skip to content

Commit

Permalink
Merge pull request kubernetes#28821 from colemickens/azure-cloudprovi…
Browse files Browse the repository at this point in the history 
…der-pr

Automatic merge from submit-queue

Add an Azure CloudProvider Implementation

This PR adds `Azure` as a cloudprovider provider for Kubernetes. It specifically adds support for native pod networking (via Azure User Defined Routes) and L4 Load Balancing (via Azure Load Balancers).

I did have to add `clusterName` as a parameter to the `LoadBalancers` methods. This is because Azure only allows one "LoadBalancer" object per set of backend machines. This means a single "LoadBalancer" object must be shared across the cluster. The "LoadBalancer" is named via the `cluster-name` parameter passed to `kube-controller-manager` so as to enable multiple clusters per resource group if the user desires such a configuration.

There are few things that I'm a bit unsure about:

1. The implementation of the `Instances` interface. It's not extensively documented, it's not really clear what the different functions are used for, and my questions on the ML didn't get an answer.

2. Counter to the comments on the `LoadBalancers` Interface, I modify the `api.Service` object in `EnsureLoadBalancerDeleted`, but not with the intention of affecting Kube's view of the Service. I simply do it so that I can remove the `Port`s on the `Service` object and then re-use my reconciliation logic that can handle removing stale/deleted Ports. 

3. The logging is a bit verbose. I'm looking for guidance on the appropriate log level to use for the chattier bits.

Due to the (current) lack of Instance Metadata Service and lack of Virtual Machine Identity in Azure, the user is required to do a few things to opt-in to this provider. These things are called-out as they are in contrast to AWS/GCE:

1. The user must provision an Azure Active Directory ServicePrincipal with `Contributor` level access to the resource group that the cluster is deployed in. This creation process is documented [by Hashicorp](https://www.packer.io/docs/builders/azure-setup.html) or [on the MSDN Blog](https://blogs.msdn.microsoft.com/arsen/2016/05/11/how-to-create-and-test-azure-service-principal-using-azure-cli/).

2. The user must place a JSON file somewhere on each Node that conforms to the `AzureConfig` struct defined in `azure.go`. (This is automatically done in the Azure flavor of [Kubernetes-Anywhere](https://github.com/kubernetes/kubernetes-anywhere).)

3. The user must specify `--cloud-config=/path/to/azure.json` as an option to `kube-apiserver` and `kube-controller-manager` similarly to how the user would need to pass `--cloud-provider=azure`.

I've been running approximately this code for a month and a half. I only encountered one bug which has since been fixed and covered by a unit test. I've just deployed a new cluster (and a Type=LoadBalancer nginx Service) using this code (via `kubernetes-anywhere`) and have posted [the `kube-controller-manager` logs](https://gist.github.com/colemickens/1bf6a26e7ef9484a72a30b1fcf9fc3cb) for anyone who is interested in seeing the logs of the logic.

If you're interested in this PR, you can use the instructions in my [`azure-kubernetes-demo` repository](https://github.com/colemickens/azure-kubernetes-demo) to deploy a cluster with minimal effort via [`kubernetes-anywhere`](https://github.com/kubernetes/kubernetes-anywhere). (There is currently [a pending PR in `kubernetes-anywhere` that is needed](kubernetes-retired/kubernetes-anywhere#172) in conjuncture with this PR). I also have a pre-built `hyperkube` image: `docker.io/colemickens/hyperkube-amd64:v1.4.0-alpha.0-azure`, which will be kept in sync with the branch this PR stems from.

I'm hoping this can land in the Kubernetes 1.4 timeframe.

CC (potential code reviewers from Azure): @ahmetalpbalkan @brendandixon @paulmey

CC (other interested Azure folk): @brendandburns @johngossman @anandramakrishna @jmspring @jimzim

CC (others who've expressed interest): @codefx9 @edevil @thockin @rootfs
  • Loading branch information
k8s-merge-robot authored Jul 27, 2016
2 parents a3ac594 + 8db4969 commit c12770f
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 27 deletions.
50 changes: 25 additions & 25 deletions pkg/cloudprovider/providers/openstack/openstack_loadbalancer.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,7 @@ func waitLoadbalancerDeleted(client *gophercloud.ServiceClient, loadbalancerID s
}
}

func (lbaas *LbaasV2) GetLoadBalancer(service *api.Service) (*api.LoadBalancerStatus, bool, error) {
func (lbaas *LbaasV2) GetLoadBalancer(clusterName string, service *api.Service) (*api.LoadBalancerStatus, bool, error) {
loadBalancerName := cloudprovider.GetLoadBalancerName(service)
loadbalancer, err := getLoadbalancerByName(lbaas.network, loadBalancerName)
if err == ErrNotFound {
Expand All @@ -281,8 +281,8 @@ func (lbaas *LbaasV2) GetLoadBalancer(service *api.Service) (*api.LoadBalancerSt
// a list of regions (from config) and query/create loadbalancers in
// each region.

func (lbaas *LbaasV2) EnsureLoadBalancer(apiService *api.Service, hosts []string) (*api.LoadBalancerStatus, error) {
glog.V(4).Infof("EnsureLoadBalancer(%v, %v, %v, %v, %v, %v)", apiService.Namespace, apiService.Name, apiService.Spec.LoadBalancerIP, apiService.Spec.Ports, hosts, apiService.Annotations)
func (lbaas *LbaasV2) EnsureLoadBalancer(clusterName string, apiService *api.Service, hosts []string) (*api.LoadBalancerStatus, error) {
glog.V(4).Infof("EnsureLoadBalancer(%v, %v, %v, %v, %v, %v, %v)", clusterName, apiService.Namespace, apiService.Name, apiService.Spec.LoadBalancerIP, apiService.Spec.Ports, hosts, apiService.Annotations)

ports := apiService.Spec.Ports
if len(ports) > 1 {
Expand Down Expand Up @@ -318,15 +318,15 @@ func (lbaas *LbaasV2) EnsureLoadBalancer(apiService *api.Service, hosts []string
}

glog.V(2).Infof("Checking if openstack load balancer already exists: %s", cloudprovider.GetLoadBalancerName(apiService))
_, exists, err := lbaas.GetLoadBalancer(apiService)
_, exists, err := lbaas.GetLoadBalancer(clusterName, apiService)
if err != nil {
return nil, fmt.Errorf("error checking if openstack load balancer already exists: %v", err)
}

// TODO: Implement a more efficient update strategy for common changes than delete & create
// In particular, if we implement hosts update, we can get rid of UpdateHosts
if exists {
err := lbaas.EnsureLoadBalancerDeleted(apiService)
err := lbaas.EnsureLoadBalancerDeleted(clusterName, apiService)
if err != nil {
return nil, fmt.Errorf("error deleting existing openstack load balancer: %v", err)
}
Expand All @@ -352,7 +352,7 @@ func (lbaas *LbaasV2) EnsureLoadBalancer(apiService *api.Service, hosts []string
loadbalancer, err := loadbalancers.Create(lbaas.network, createOpts).Extract()
if err != nil {
// cleanup what was created so far
_ = lbaas.EnsureLoadBalancerDeleted(apiService)
_ = lbaas.EnsureLoadBalancerDeleted(clusterName, apiService)
return nil, err
}

Expand All @@ -366,7 +366,7 @@ func (lbaas *LbaasV2) EnsureLoadBalancer(apiService *api.Service, hosts []string
}).Extract()
if err != nil {
// cleanup what was created so far
_ = lbaas.EnsureLoadBalancerDeleted(apiService)
_ = lbaas.EnsureLoadBalancerDeleted(clusterName, apiService)
return nil, err
}

Expand All @@ -381,7 +381,7 @@ func (lbaas *LbaasV2) EnsureLoadBalancer(apiService *api.Service, hosts []string
}).Extract()
if err != nil {
// cleanup what was created so far
_ = lbaas.EnsureLoadBalancerDeleted(apiService)
_ = lbaas.EnsureLoadBalancerDeleted(clusterName, apiService)
return nil, err
}

Expand All @@ -400,7 +400,7 @@ func (lbaas *LbaasV2) EnsureLoadBalancer(apiService *api.Service, hosts []string
}).Extract()
if err != nil {
// cleanup what was created so far
_ = lbaas.EnsureLoadBalancerDeleted(apiService)
_ = lbaas.EnsureLoadBalancerDeleted(clusterName, apiService)
return nil, err
}
}
Expand All @@ -417,7 +417,7 @@ func (lbaas *LbaasV2) EnsureLoadBalancer(apiService *api.Service, hosts []string
}).Extract()
if err != nil {
// cleanup what was created so far
_ = lbaas.EnsureLoadBalancerDeleted(apiService)
_ = lbaas.EnsureLoadBalancerDeleted(clusterName, apiService)
return nil, err
}
}
Expand All @@ -430,7 +430,7 @@ func (lbaas *LbaasV2) EnsureLoadBalancer(apiService *api.Service, hosts []string
portID, err := getPortIDByIP(lbaas.network, loadbalancer.VipAddress)
if err != nil {
// cleanup what was created so far
_ = lbaas.EnsureLoadBalancerDeleted(apiService)
_ = lbaas.EnsureLoadBalancerDeleted(clusterName, apiService)
return nil, err
}

Expand All @@ -441,7 +441,7 @@ func (lbaas *LbaasV2) EnsureLoadBalancer(apiService *api.Service, hosts []string
floatIP, err := floatingips.Create(lbaas.network, floatIPOpts).Extract()
if err != nil {
// cleanup what was created so far
_ = lbaas.EnsureLoadBalancerDeleted(apiService)
_ = lbaas.EnsureLoadBalancerDeleted(clusterName, apiService)
return nil, err
}

Expand All @@ -451,9 +451,9 @@ func (lbaas *LbaasV2) EnsureLoadBalancer(apiService *api.Service, hosts []string
return status, nil
}

func (lbaas *LbaasV2) UpdateLoadBalancer(service *api.Service, hosts []string) error {
func (lbaas *LbaasV2) UpdateLoadBalancer(clusterName string, service *api.Service, hosts []string) error {
loadBalancerName := cloudprovider.GetLoadBalancerName(service)
glog.V(4).Infof("UpdateLoadBalancer(%v, %v)", loadBalancerName, hosts)
glog.V(4).Infof("UpdateLoadBalancer(%v, %v, %v)", clusterName, loadBalancerName, hosts)

ports := service.Spec.Ports
if len(ports) > 1 {
Expand Down Expand Up @@ -538,9 +538,9 @@ func (lbaas *LbaasV2) UpdateLoadBalancer(service *api.Service, hosts []string) e
return nil
}

func (lbaas *LbaasV2) EnsureLoadBalancerDeleted(service *api.Service) error {
func (lbaas *LbaasV2) EnsureLoadBalancerDeleted(clusterName string, service *api.Service) error {
loadBalancerName := cloudprovider.GetLoadBalancerName(service)
glog.V(4).Infof("EnsureLoadBalancerDeleted(%v)", loadBalancerName)
glog.V(4).Infof("EnsureLoadBalancerDeleted(%v, %v)", clusterName, loadBalancerName)

loadbalancer, err := getLoadbalancerByName(lbaas.network, loadBalancerName)
if err != nil && err != ErrNotFound {
Expand Down Expand Up @@ -690,7 +690,7 @@ func (lbaas *LbaasV2) EnsureLoadBalancerDeleted(service *api.Service) error {
return nil
}

func (lb *LbaasV1) GetLoadBalancer(service *api.Service) (*api.LoadBalancerStatus, bool, error) {
func (lb *LbaasV1) GetLoadBalancer(clusterName string, service *api.Service) (*api.LoadBalancerStatus, bool, error) {
loadBalancerName := cloudprovider.GetLoadBalancerName(service)
vip, err := getVipByName(lb.network, loadBalancerName)
if err == ErrNotFound {
Expand All @@ -711,8 +711,8 @@ func (lb *LbaasV1) GetLoadBalancer(service *api.Service) (*api.LoadBalancerStatu
// a list of regions (from config) and query/create loadbalancers in
// each region.

func (lb *LbaasV1) EnsureLoadBalancer(apiService *api.Service, hosts []string) (*api.LoadBalancerStatus, error) {
glog.V(4).Infof("EnsureLoadBalancer(%v, %v, %v, %v, %v, %v)", apiService.Namespace, apiService.Name, apiService.Spec.LoadBalancerIP, apiService.Spec.Ports, hosts, apiService.Annotations)
func (lb *LbaasV1) EnsureLoadBalancer(clusterName string, apiService *api.Service, hosts []string) (*api.LoadBalancerStatus, error) {
glog.V(4).Infof("EnsureLoadBalancer(%v, %v, %v, %v, %v, %v, %v)", clusterName, apiService.Namespace, apiService.Name, apiService.Spec.LoadBalancerIP, apiService.Spec.Ports, hosts, apiService.Annotations)

ports := apiService.Spec.Ports
if len(ports) > 1 {
Expand Down Expand Up @@ -748,15 +748,15 @@ func (lb *LbaasV1) EnsureLoadBalancer(apiService *api.Service, hosts []string) (
}

glog.V(2).Infof("Checking if openstack load balancer already exists: %s", cloudprovider.GetLoadBalancerName(apiService))
_, exists, err := lb.GetLoadBalancer(apiService)
_, exists, err := lb.GetLoadBalancer(clusterName, apiService)
if err != nil {
return nil, fmt.Errorf("error checking if openstack load balancer already exists: %v", err)
}

// TODO: Implement a more efficient update strategy for common changes than delete & create
// In particular, if we implement hosts update, we can get rid of UpdateHosts
if exists {
err := lb.EnsureLoadBalancerDeleted(apiService)
err := lb.EnsureLoadBalancerDeleted(clusterName, apiService)
if err != nil {
return nil, fmt.Errorf("error deleting existing openstack load balancer: %v", err)
}
Expand Down Expand Up @@ -860,9 +860,9 @@ func (lb *LbaasV1) EnsureLoadBalancer(apiService *api.Service, hosts []string) (

}

func (lb *LbaasV1) UpdateLoadBalancer(service *api.Service, hosts []string) error {
func (lb *LbaasV1) UpdateLoadBalancer(clusterName string, service *api.Service, hosts []string) error {
loadBalancerName := cloudprovider.GetLoadBalancerName(service)
glog.V(4).Infof("UpdateLoadBalancer(%v, %v)", loadBalancerName, hosts)
glog.V(4).Infof("UpdateLoadBalancer(%v, %v, %v)", clusterName, loadBalancerName, hosts)

vip, err := getVipByName(lb.network, loadBalancerName)
if err != nil {
Expand Down Expand Up @@ -922,9 +922,9 @@ func (lb *LbaasV1) UpdateLoadBalancer(service *api.Service, hosts []string) erro
return nil
}

func (lb *LbaasV1) EnsureLoadBalancerDeleted(service *api.Service) error {
func (lb *LbaasV1) EnsureLoadBalancerDeleted(clusterName string, service *api.Service) error {
loadBalancerName := cloudprovider.GetLoadBalancerName(service)
glog.V(4).Infof("EnsureLoadBalancerDeleted(%v)", loadBalancerName)
glog.V(4).Infof("EnsureLoadBalancerDeleted(%v, %v)", clusterName, loadBalancerName)

vip, err := getVipByName(lb.network, loadBalancerName)
if err != nil && err != ErrNotFound {
Expand Down
5 changes: 3 additions & 2 deletions pkg/cloudprovider/providers/openstack/openstack_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import (
const volumeAvailableStatus = "available"
const volumeInUseStatus = "in-use"
const volumeCreateTimeoutSeconds = 30
const testClusterName = "testCluster"

func WaitForVolumeStatus(t *testing.T, os *OpenStack, volumeName string, status string, timeoutSeconds int) {
timeout := timeoutSeconds
Expand Down Expand Up @@ -216,7 +217,7 @@ func TestLoadBalancer(t *testing.T) {
t.Fatalf("LoadBalancer() returned false - perhaps your stack doesn't support Neutron?")
}

_, exists, err := lb.GetLoadBalancer(&api.Service{ObjectMeta: api.ObjectMeta{Name: "noexist"}})
_, exists, err := lb.GetLoadBalancer(testClusterName, &api.Service{ObjectMeta: api.ObjectMeta{Name: "noexist"}})
if err != nil {
t.Fatalf("GetLoadBalancer(\"noexist\") returned error: %s", err)
}
Expand All @@ -242,7 +243,7 @@ func TestLoadBalancerV2(t *testing.T) {
t.Fatalf("LoadBalancer() returned false - perhaps your stack doesn't support Neutron?")
}

_, exists, err := lbaas.GetLoadBalancer(&api.Service{ObjectMeta: api.ObjectMeta{Name: "noexist"}})
_, exists, err := lbaas.GetLoadBalancer(testClusterName, &api.Service{ObjectMeta: api.ObjectMeta{Name: "noexist"}})
if err != nil {
t.Fatalf("GetLoadBalancer(\"noexist\") returned error: %s", err)
}
Expand Down

0 comments on commit c12770f

Please sign in to comment.