LBaaS v2 Support for Openstack Cloud Provider Plugin #25987

dagnello · 2016-05-20T20:50:20Z

Resolves #19774.

This work is based on Gophercloud support for LBaaS v2 currently in review (this will have to merge first):
rackspace/gophercloud#575

These changes includes the addition of a new loadbalancer configuration option: LBVersion. If this configuration attribute is missing or anything other than "v2", lbaas v1 implementation will be used.

k8s-bot · 2016-05-20T20:50:39Z

Can one of the admins verify that this patch is reasonable to test? If so, please reply "ok to test".
(Note: "add to whitelist" is no longer supported. Please update configurations in kubernetes/test-infra/jenkins/job-configs/kubernetes-jenkins-pull instead.)

This message may repeat a few times in short succession due to jenkinsci/ghprb-plugin#292. Sorry.

Otherwise, if this message is too spammy, please complain to ixdy.

k8s-bot · 2016-05-20T20:51:15Z

Can one of the admins verify that this patch is reasonable to test? If so, please reply "ok to test".
(Note: "add to whitelist" is no longer supported. Please update configurations in kubernetes/test-infra/jenkins/job-configs/kubernetes-jenkins-pull instead.)

This message may repeat a few times in short succession due to jenkinsci/ghprb-plugin#292. Sorry.

Otherwise, if this message is too spammy, please complain to ixdy.

k8s-bot · 2016-05-20T20:52:17Z

Can one of the admins verify that this patch is reasonable to test? If so, please reply "ok to test".
(Note: "add to whitelist" is no longer supported. Please update configurations in kubernetes/test-infra/jenkins/job-configs/kubernetes-jenkins-pull instead.)

This message may repeat a few times in short succession due to jenkinsci/ghprb-plugin#292. Sorry.

Otherwise, if this message is too spammy, please complain to ixdy.

anguslees · 2016-05-24T05:10:12Z

pkg/cloudprovider/providers/openstack/openstack_loadbalancer.go

+		return nil, err
+	}
+
+	waitLoadbalancerActiveProvisioningStatus(lbaas.network, loadbalancer.ID)


Are all these waitLoadbalancerActiveProvisioningStatus calls required before proceeding to each next step? If so, that's a shame :(

yeah, unfortunately anytime a change is made to a component, the respective load balancer changes state to "... pending". During this time, any changes made will return with failure bc the load balancer state is not ACTIVE

Does the underlying library have a equivalent of --wait that does this under the covers?

There is https://godoc.org/github.com/rackspace/gophercloud#WaitFor, but it doesn't do anything more magic than repeatedly polling until a provided predicate returns true (so very similar to waitLoadbalancerActiveProvisioningStatus)

anguslees · 2016-05-24T05:21:42Z

lgtm, nice work.

My only high-level comment is that it'd be nice to automatically determine v1 vs v2 by querying available neutron extensions (use v2 if available, fallback to v1) - rather than force deployers to have to specify.

dagnello · 2016-05-24T07:53:03Z

Thanks @anguslees. Ok, that would be nice, let me look into that. If it's small enough I can include it as part of this PR, otherwise I can address in a separate PR.

mikedanese · 2016-05-31T18:31:37Z

pkg/cloudprovider/providers/openstack/openstack.go

@@ -1,5 +1,5 @@
 /*
-Copyright 2014 The Kubernetes Authors All rights reserved.
+Copyright 2016 The Kubernetes Authors All rights reserved.


I don't think we want to change this.

mikedanese · 2016-05-31T18:33:13Z

@k8s-bot ok to test

mikedanese · 2016-05-31T18:33:37Z

I'm happy to merge once nits are addressed assuming CI passes, sorry for the delay @dagnello

googlebot · 2016-06-01T23:44:04Z

We found a Contributor License Agreement for you (the sender of this pull request) and all commit authors, but as best as we can tell these commits were authored by someone else. If that's the case, please add them to this pull request and have them confirm that they're okay with these commits being contributed to Google. If we're mistaken and you did author these commits, just reply here to confirm.

sputnik13 · 2016-06-01T23:53:00Z

@googlebot We did author this pull request

yeah this doesn't seem to do anything, how do we get rid of that cla:no, I signed both corporate and individual CLAs... I have all the CLAs

mikedanese · 2016-06-02T03:31:38Z

@sputnik13 we can human verify. It's not a problem.

sputnik13 · 2016-06-02T03:46:41Z

@mikedanese that sounds good, thanks. I just realized we didn't address your comment about changing the copyright date. Should it be 2014-2016 instead of just 2016, or do you want it to stay 2014? It's been standard practice on our team to update copyright notices to include the year that it was last updated. I think that's why @dagnello updated that, but we'd be fine with following the standard practice of the k8s community.

googlebot · 2016-06-08T00:40:57Z

CLAs look good, thanks!

dagnello · 2016-06-08T01:13:10Z

@lavalamp @mikedanese updated

feiskyer · 2016-06-08T02:26:10Z

LGTM.

dagnello · 2016-06-08T16:54:36Z

@lavalamp @mikedanese any other updates required?

mikedanese · 2016-06-08T17:01:31Z

LGTM still

k8s-bot · 2016-06-09T01:57:29Z

GCE e2e build/test passed for commit bb1ae12.

mikedanese · 2016-06-09T02:58:11Z

@k8s-bot node e2e test this please, issue: #IGNORE

dagnello · 2016-06-09T17:29:55Z

@mikedanese build test failed with error: failed: No test report files were found. Configuration error?
is this error related to my changes?

mikedanese · 2016-06-09T17:32:06Z

this is almost certainly a flake

@k8s-bot node e2e test this please, issue: #IGNORE

k8s-github-robot · 2016-06-10T01:32:35Z

Automatic merge from submit-queue

jianhuiz · 2016-06-21T01:06:45Z

pkg/cloudprovider/providers/openstack/openstack_loadbalancer.go

+		waitLoadbalancerActiveProvisioningStatus(lbaas.network, loadbalancer.ID)
+
+		_, err = v2_pools.CreateAssociateMember(lbaas.network, pool.ID, v2_pools.MemberCreateOpts{
+			Name:         name,


there is no name for pool members according to https://wiki.openstack.org/wiki/Neutron/LBaaS/API_2.0#Add_a_New_Member_to_a_Pool, I got the error "create failed (client error): Unrecognized attribute(s) 'name'" when creating a service.

I had to remove this line to be able to successfully create a service with lbv2 on openstack liberty

Perhaps member's name is supported in Mitaka and beyond. Can you verify this?

This works for latest/Mitaka but will remove it so this will work for liberty as well.

tybritten · 2016-07-09T13:25:13Z

Do you think the plugin should ask for a default security group id? Once the LB is created for the service, a security group has to be applied to the LB VIP port manually via neutron.

tybritten · 2016-07-25T18:44:11Z

Currently in LBaaS in openstack (v1 and v2), it put the LBaaS port in the default security group only. Most environments have that set not to allow any traffic. There's a bug open to ask for a security group when creating an LB, but it hasn't gone anywhere. This means if you use this plugin with kubernetes, it with create the LB and assign a floating ip, but no one will be able to access it. the best fix is allow the config file to specify a security group id- the plugin would get the port_id of the LB after its created and then use update-port (http://gophercloud.io/docs/networking/#update-port) to add the security group to the port. if no security group was supplied in the config file it would behave as it does now.

dagnello · 2016-07-28T17:10:42Z

@vmtyler that looks like a good extension of the current implementation. Can you raise an issue in Kubernetes for this?

tybritten · 2016-07-28T17:12:23Z

yes, will do now.

#29745

@dagnello

Automatic merge from submit-queue Openstack heat network add lbaas subnet and floating network configuration support lbaas v2 add environment variable for fixed network ~~fix lb creation failed because of no 'name' for pool members according to lbaas v2 api~~ #27810 #25987 @dagnello @lavalamp @mikedanese [![Analytics](https://kubernetes-site.appspot.com/UA-36037335-10/GitHub/.github/PULL_REQUEST_TEMPLATE.md?pixel)]()

Automatic merge from submit-queue LBaaS v2 Support for Openstack Cloud Provider Plugin Resolves kubernetes#19774. This work is based on Gophercloud support for LBaaS v2 currently in review (this will have to merge first): rackspace/gophercloud#575 These changes includes the addition of a new loadbalancer configuration option: **LBVersion**. If this configuration attribute is missing or anything other than "v2", lbaas v1 implementation will be used.

googlebot added the cla: yes label May 20, 2016

k8s-github-robot assigned mikedanese May 20, 2016

k8s-github-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. release-note-label-needed labels May 20, 2016

anguslees reviewed May 24, 2016
View reviewed changes

dagnello force-pushed the openstack-lbaas-v2 branch from 0c1a9a9 to cc915fa Compare May 24, 2016 22:57

k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 28, 2016

mikedanese reviewed May 31, 2016
View reviewed changes

mikedanese added the ok-to-merge label May 31, 2016

sputnik13 force-pushed the openstack-lbaas-v2 branch from cc915fa to 14025be Compare June 1, 2016 23:41

googlebot added cla: no and removed cla: yes labels Jun 1, 2016

k8s-github-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 1, 2016

sputnik13 force-pushed the openstack-lbaas-v2 branch from acf9d82 to ddf3b46 Compare June 1, 2016 23:50

sputnik13 force-pushed the openstack-lbaas-v2 branch from ddf3b46 to bc5c3ac Compare June 2, 2016 00:36

sputnik13 force-pushed the openstack-lbaas-v2 branch from f1c3e35 to 3348b3b Compare June 3, 2016 04:23

k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 3, 2016

googlebot added the cla: yes label Jun 8, 2016

mikedanese added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 8, 2016

k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 9, 2016

dagnello and others added 2 commits June 8, 2016 18:16

Adding support for lbaas-v2 in OpenStack Cloud Provider

eb32bf5

Vendor/Godep updates

bb1ae12

dagnello force-pushed the openstack-lbaas-v2 branch from fc256c4 to bb1ae12 Compare June 9, 2016 01:17

k8s-github-robot removed lgtm "Looks good to me", indicates that a PR is ready to be merged. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Jun 9, 2016

mikedanese added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 9, 2016

lavalamp added the retest-not-required label Jun 9, 2016

k8s-github-robot merged commit cbde2ec into kubernetes:master Jun 10, 2016

jianhuiz reviewed Jun 21, 2016
View reviewed changes

jianhuiz mentioned this pull request Jun 21, 2016

Openstack heat network #27783

Merged

tybritten mentioned this pull request Jul 25, 2016

LBaaS doesn't return vip_port_id gophercloud/gophercloud#9

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

LBaaS v2 Support for Openstack Cloud Provider Plugin #25987

LBaaS v2 Support for Openstack Cloud Provider Plugin #25987

dagnello commented May 20, 2016

k8s-bot commented May 20, 2016

k8s-bot commented May 20, 2016

k8s-bot commented May 20, 2016

anguslees May 24, 2016

dagnello May 24, 2016

harlowja May 31, 2016

anguslees Jun 1, 2016

anguslees commented May 24, 2016

dagnello commented May 24, 2016

mikedanese May 31, 2016

mikedanese commented May 31, 2016

mikedanese commented May 31, 2016 •

edited

Loading

googlebot commented Jun 1, 2016

sputnik13 commented Jun 1, 2016 •

edited

Loading

mikedanese commented Jun 2, 2016

sputnik13 commented Jun 2, 2016 •

edited

Loading

googlebot commented Jun 8, 2016

dagnello commented Jun 8, 2016

feiskyer commented Jun 8, 2016

dagnello commented Jun 8, 2016

mikedanese commented Jun 8, 2016

k8s-bot commented Jun 9, 2016

mikedanese commented Jun 9, 2016

dagnello commented Jun 9, 2016

mikedanese commented Jun 9, 2016

k8s-github-robot commented Jun 10, 2016

jianhuiz Jun 21, 2016

kokhang Jun 21, 2016

dagnello Jun 21, 2016

tybritten commented Jul 9, 2016

tybritten commented Jul 25, 2016

dagnello commented Jul 28, 2016

tybritten commented Jul 28, 2016 •

edited

Loading

LBaaS v2 Support for Openstack Cloud Provider Plugin #25987

LBaaS v2 Support for Openstack Cloud Provider Plugin #25987

Conversation

dagnello commented May 20, 2016

k8s-bot commented May 20, 2016

k8s-bot commented May 20, 2016

k8s-bot commented May 20, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

anguslees commented May 24, 2016

dagnello commented May 24, 2016

Choose a reason for hiding this comment

mikedanese commented May 31, 2016

mikedanese commented May 31, 2016 • edited Loading

googlebot commented Jun 1, 2016

sputnik13 commented Jun 1, 2016 • edited Loading

mikedanese commented Jun 2, 2016

sputnik13 commented Jun 2, 2016 • edited Loading

googlebot commented Jun 8, 2016

dagnello commented Jun 8, 2016

feiskyer commented Jun 8, 2016

dagnello commented Jun 8, 2016

mikedanese commented Jun 8, 2016

k8s-bot commented Jun 9, 2016

mikedanese commented Jun 9, 2016

dagnello commented Jun 9, 2016

mikedanese commented Jun 9, 2016

k8s-github-robot commented Jun 10, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tybritten commented Jul 9, 2016

tybritten commented Jul 25, 2016

dagnello commented Jul 28, 2016

tybritten commented Jul 28, 2016 • edited Loading

mikedanese commented May 31, 2016 •

edited

Loading

sputnik13 commented Jun 1, 2016 •

edited

Loading

sputnik13 commented Jun 2, 2016 •

edited

Loading

tybritten commented Jul 28, 2016 •

edited

Loading