infraly / k8s-on-openstack

An opinionated way to deploy a Kubernetes cluster on top of an OpenStack cloud.
Apache License 2.0
112 stars 48 forks source link

NETWORK, should it not default to the value of NAME? #19

Closed madchap closed 6 years ago

madchap commented 6 years ago

Hi @ctrlaltdel :)

Thanks a lot for the job!

I tried to run it quickly before getting some zzzz. So before I forget, and maybe I am lucky not to say too many dumb things at this time...

I tried a minimal setup, where on top of the regular OS_* which get populated upon my cli login (v3), I have put:

10124  export IMAGE=image-centos-201802202308
10125  export KEY=FBI-KS
10126  export NAME=k8s-fbi
10127  export EXTERNAL_NETWORK=internet
10128  export FLOATING_IP_POOL=internet
10129  export FLOATING_IP_NETWORK_UUID=9a942d28-0000-1111-0000-cbae41ee626d

My deployment bombs at

Monday 02 April 2018  23:36:19 +0200 (0:00:00.862)       0:00:42.501 ********** 
fatal: [localhost]: FAILED! => {"changed": false, "msg": "Could not find network by net-name: k8s"}
    to retry, use: --limit @/home/fblaise/gitrepos/k8s-on-openstack/site.retry

Looking at the code (specifically at roles/openstack-master/tasks/main.yaml), the default for NETWORK is k8s, which indeed does not exist in my tenant. However, previous ansible steps do create a network for me, which seems to be derived from NAME.

(openstack) network list
+--------------------------------------+---------------+--------------------------------------+
| ID                                   | Name          | Subnets                              |
+--------------------------------------+---------------+--------------------------------------+
| 8e6b5f35-0000-1111-0000-046583047a01 | k8s-fbi       | d26549a1-0000-1111-0000-8ee3b30ebae6 |
| 9a942d28-0000-1111-0000-cbae41ee626d | internet      | aca7ee79-0000-1111-0000-fa936bfbbb68 |
+--------------------------------------+---------------+--------------------------------------+

Not digging much further, shouldn't the NETWORK variable get its value from the NAME variable in that case?

Thanks, see you.

zioproto commented 6 years ago

@ctrlaltdel is it possible you introduced the bug in commit d5a26122cf8077fe1dc39ee9e17ffd47e70581c7 ??

if NAME is not defined then the default is missing ?

Also, we can reference the value like "{{ name }}"

Can we change:

 - net-name: "{{ lookup('env', 'NETWORK') |  default(lookup('env', 'NAME'), true) }}"

with:

 - net-name: "{{ lookup('env', 'NETWORK') |  default(name, true) }}"
zioproto commented 6 years ago

@madchap if you can test the proposed change please give feedback, or even better propose a PR

ctrlaltdel commented 6 years ago

Yeah, my bad! I'm currently re-working how these variables are being set because it's quite messy for now.

All the defaults will be set in group_vars/all.yaml instead of being scattered all around.

zioproto commented 6 years ago

OK, If you want me to test a refactory branch let me know.

ctrlaltdel commented 6 years ago

@zioproto yep, it's available there: https://github.com/infraly/k8s-on-openstack/tree/refactor

@madchap Hopefully, it will be fixed next time you'll give it a try :)

zioproto commented 6 years ago

@ctrlaltdel are a workaround lets just revert commit d5a2612 and lets close this issue

ctrlaltdel commented 6 years ago

Hopefully now fixed on master, happy testing!