Openstack heat network #27783

jianhuiz · 2016-06-21T17:06:19Z

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

This change is

googlebot · 2016-06-21T17:06:21Z

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for the commit author(s). If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.

k8s-bot · 2016-06-21T17:06:37Z

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-06-21T17:07:00Z

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-06-21T17:07:23Z

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-06-21T17:08:18Z

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.

googlebot · 2016-06-21T17:09:49Z

CLAs look good, thanks!

k8s-bot · 2016-06-23T22:05:01Z

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.

eparis · 2016-07-21T23:09:48Z

@kubernetes/sig-openstack I'm pretty clueless how to review this. Can anyone suggest a person?

anguslees · 2016-07-22T03:22:31Z

lgtm.

@jianhuiz: Any reason you chose to default to lbaas v1 rather than v2? I mention it only because I thought part of the push for adding support for v2 was that many (most?) clouds supported v2 and not v1.

rootfs · 2016-07-22T12:16:45Z

cluster/openstack-heat/kubernetes-heat/fragments/configure-salt.yaml

@@ -32,6 +32,10 @@ write_files:
      password=$OS_PASSWORD
      region=$OS_REGION_NAME
      tenant-id=$OS_TENANT_ID
+      [LoadBalancer]
+      lb-version=$LBAAS_VERSION
+      subnet-id=$SUBNET_ID


Is it ok if SUBNET_ID and FLOATING_NETWORK_ID are not set?

Not in my environments which are OpenStack Juno and Liberty.

What is the current state of the proposal to remove LBaaS V1 from Neutron? I was under the impression that this could happen as early as Newton.

Yes, I don't think it's worth writing any new code using v1.

So for this change we have 2 choices:

Default to v1, as written.

Default to v2 above at line 38.

Defaulting to v2 here would effectively be a change in the heat scripts vs the regular k8s openstack plugin that defaults to v1 if nothing is specified. I agree 100% that at some point we need to start defaulting to v2, and that point is now / very soon. I kind of think the two pieces of k8s/openstack should follow the same logic, whatever that logic is, however - so I have a weak preference for that s/v1/v2/g default to be in another change.

I'm working on a PR to just query the available neutron extensions, and pick v2 vs v1 automatically - so we can hopefully drop all the explicit defaults once that is merged and never speak of this again. I don't have an opinion on whether we should delay this change until that other one lands - I'm fine with merging this change and then removing the explicit default (line 38) later if there's any urgency.

Edit: LBaaS v1/v2 autodetect PR is #29726

Ok, #29726 has merged. I think we should make sure we default lb-version to "" (empty string / unspecified) here, and let the autodetection code do its thing (still with the option for users to override if they need to).

jianhuiz · 2016-07-22T17:03:25Z

@anguslees I thought defaults to v1 is backward compatible and will not break the scripts people are currently using. I personally have both versions (Juno and Liberty) so it wouldn't matter which is default.
It would be ideal if the provider could determine the versions automatically.

anguslees · 2016-07-26T00:40:10Z

@jianhuiz: Agreed, we should auto-detect which lbaas versions the server advertises and remove this option (or at least leave it only for rare override use cases). I raised this on the lbaasv2 change, but the original submitter didn't have time to add it (nor have I) and I didn't want to block support for lbaasv2 over it. Patches always welcome :)

k8s-bot · 2016-08-02T19:55:41Z

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-08-04T18:28:32Z

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.

dagnello · 2016-08-15T21:24:14Z

cluster/openstack-heat/config-default.sh

@@ -35,6 +35,8 @@ MINION_FLAVOR=${MINION_FLAVOR:-m1.medium}

 EXTERNAL_NETWORK=${EXTERNAL_NETWORK:-public}

+LBAAS_VERSION=${LBAAS_VERSION:-v1}


I agree with @anguslees, we can default to "" and let the plugin auto-detect which loadbalancer to use.

jianhuiz · 2016-08-15T22:30:02Z

default LBAAS_VERSION to ""

dagnello · 2016-08-15T22:32:25Z

this LGTM

anguslees · 2016-08-17T02:27:35Z

cluster/openstack-heat/kubernetes-heat/kubecluster.yaml

+  lbaas_version:
+    type: string
+    description: version of OpenStack LBaaS service
+    default: v1


This "default" needs to be removed too.

right, forgot it. fixed now

…fixed network

k8s-bot · 2016-08-18T13:02:22Z

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-08-19T09:39:48Z

lgtm

k8s-bot · 2016-08-20T19:21:18Z

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-09-07T07:47:16Z

GCE e2e build/test passed for commit 9908a02.

k8s-github-robot · 2016-09-07T22:44:50Z

@k8s-bot test this [submit-queue is verifying that this PR is safe to merge]

k8s-bot · 2016-09-07T23:24:17Z

GCE e2e build/test passed for commit 9908a02.

k8s-github-robot · 2016-09-07T23:25:02Z

Automatic merge from submit-queue

googlebot added the cla: no label Jun 21, 2016

jianhuiz force-pushed the openstack-heat-network branch from a59d197 to c2ea1d1 Compare June 21, 2016 17:09

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

k8s-github-robot assigned eparis Jun 21, 2016

k8s-github-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. release-note-label-needed labels Jun 21, 2016

jianhuiz force-pushed the openstack-heat-network branch from c2ea1d1 to 9fc460a Compare June 21, 2016 23:44

jianhuiz force-pushed the openstack-heat-network branch from 9fc460a to 57da831 Compare July 5, 2016 17:59

rootfs reviewed Jul 22, 2016
View reviewed changes

k8s-github-robot added do-not-merge DEPRECATED. Indicates that a PR should not merge. Label can only be manually applied/removed. retest-not-required-docs-only and removed retest-not-required-docs-only labels Jul 28, 2016

apelisse removed the do-not-merge DEPRECATED. Indicates that a PR should not merge. Label can only be manually applied/removed. label Aug 11, 2016

dagnello reviewed Aug 15, 2016
View reviewed changes

jianhuiz force-pushed the openstack-heat-network branch from 57da831 to 551bd65 Compare August 15, 2016 22:29

anguslees reviewed Aug 17, 2016
View reviewed changes

lbaas subnet and floating network config; lbaas v2; env variable for …

9908a02

…fixed network

jianhuiz force-pushed the openstack-heat-network branch from 551bd65 to 9908a02 Compare August 17, 2016 09:28

mikedanese added lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note-none Denotes a PR that doesn't merit a release note. and removed release-note-label-needed labels Aug 30, 2016

k8s-github-robot merged commit 9cf0ec3 into kubernetes:master Sep 7, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Openstack heat network #27783

Openstack heat network #27783

jianhuiz commented Jun 21, 2016 •

edited by k8s-oncall

Loading

googlebot commented Jun 21, 2016

k8s-bot commented Jun 21, 2016

k8s-bot commented Jun 21, 2016

k8s-bot commented Jun 21, 2016

k8s-bot commented Jun 21, 2016

googlebot commented Jun 21, 2016

k8s-bot commented Jun 23, 2016

eparis commented Jul 21, 2016

anguslees commented Jul 22, 2016

rootfs Jul 22, 2016

jianhuiz Jul 22, 2016

xsgordon Jul 25, 2016

russellb Jul 26, 2016

anguslees Jul 27, 2016 •

edited

Loading

anguslees Aug 15, 2016

jianhuiz commented Jul 22, 2016

anguslees commented Jul 26, 2016

k8s-bot commented Aug 2, 2016

k8s-bot commented Aug 4, 2016

dagnello Aug 15, 2016

jianhuiz commented Aug 15, 2016

dagnello commented Aug 15, 2016

anguslees Aug 17, 2016

jianhuiz Aug 17, 2016 •

edited

Loading

k8s-bot commented Aug 18, 2016

anguslees commented Aug 19, 2016

k8s-bot commented Aug 20, 2016

k8s-bot commented Sep 7, 2016

k8s-github-robot commented Sep 7, 2016

k8s-bot commented Sep 7, 2016

k8s-github-robot commented Sep 7, 2016

		@@ -35,6 +35,8 @@ MINION_FLAVOR=${MINION_FLAVOR:-m1.medium}

		EXTERNAL_NETWORK=${EXTERNAL_NETWORK:-public}

		LBAAS_VERSION=${LBAAS_VERSION:-v1}

Openstack heat network #27783

Openstack heat network #27783

Conversation

jianhuiz commented Jun 21, 2016 • edited by k8s-oncall Loading

googlebot commented Jun 21, 2016

k8s-bot commented Jun 21, 2016

k8s-bot commented Jun 21, 2016

k8s-bot commented Jun 21, 2016

k8s-bot commented Jun 21, 2016

googlebot commented Jun 21, 2016

k8s-bot commented Jun 23, 2016

eparis commented Jul 21, 2016

anguslees commented Jul 22, 2016

rootfs Jul 22, 2016

Choose a reason for hiding this comment

jianhuiz Jul 22, 2016

Choose a reason for hiding this comment

xsgordon Jul 25, 2016

Choose a reason for hiding this comment

russellb Jul 26, 2016

Choose a reason for hiding this comment

anguslees Jul 27, 2016 • edited Loading

Choose a reason for hiding this comment

anguslees Aug 15, 2016

Choose a reason for hiding this comment

jianhuiz commented Jul 22, 2016

anguslees commented Jul 26, 2016

k8s-bot commented Aug 2, 2016

k8s-bot commented Aug 4, 2016

dagnello Aug 15, 2016

Choose a reason for hiding this comment

jianhuiz commented Aug 15, 2016

dagnello commented Aug 15, 2016

anguslees Aug 17, 2016

Choose a reason for hiding this comment

jianhuiz Aug 17, 2016 • edited Loading

Choose a reason for hiding this comment

k8s-bot commented Aug 18, 2016

anguslees commented Aug 19, 2016

k8s-bot commented Aug 20, 2016

k8s-bot commented Sep 7, 2016

k8s-github-robot commented Sep 7, 2016

k8s-bot commented Sep 7, 2016

k8s-github-robot commented Sep 7, 2016

jianhuiz commented Jun 21, 2016 •

edited by k8s-oncall

Loading

anguslees Jul 27, 2016 •

edited

Loading

jianhuiz Aug 17, 2016 •

edited

Loading