enable stsstack to also support non-openstack clouds #227 #228
Conversation
zhhuabj
force-pushed
the
underlying_lxd
branch
from
4b1bf47 to
1579176
Compare
common/generate_bundle_base
Outdated
| vip_start=${MASTER_OPTS[VIP_ADDR_START]} | ||
| cidr="" | ||
| if [[ -z $vip_start ]] && [[ -e ~/novarc ]]; then |
There was a problem hiding this comment.
Think you can simply change this by changing:
if [[ -z $vip_start ]] && [[ -e ~/novarc ]]; then
to:
if [[ -z $vip_start ]] && [[ -e ~/novarc ]] && [[ $type = openstack ]]; then
Then you wouldn't check twice inside the if. And can also drop the cidr="" too.
There was a problem hiding this comment.
hi pon, I made the modifications as you suggested,
if [[ -z $vip_start ]] && [[ -e ~/novarc ]] && [[ $type = openstack ]]; then
and also added the following code to improve it. thanks.
elif [[ -z $vip_start ]]; then
echo "ERROR: No VIP start address provided with --vip-addr-start for the $type cloud."
exit 1
common/helpers
Outdated
| @@ -43,6 +43,13 @@ list_overlays () | |||
| } | sort -u | |||
| } | |||
|
|
|||
| get_cloud_type () | |||
| { | |||
| local cloud=`juju show-model| sed -rn 's/.+cloud:\s*(.+).*/\1/p'| uniq` | |||
There was a problem hiding this comment.
jq is better than parsing yaml with sed (it can also get unique values):
local cloud=$(juju show-model --format json |jq -r '[.[].cloud] | unique[]')
local type=$(juju show-cloud $cloud --format json | jq -r '[.[].type] | unique[]')
There was a problem hiding this comment.
Hi pon, yes, jq is better than parsing yaml with sed. Although I merely relocated two original lines without modifying the previous code (perhaps the optimized code should submit another pr), I have now modified to using jq instead of sed to align with your suggestion. thanks for the review.
There was a problem hiding this comment.
Thanks Joshua. Looks good now!
common/generate_bundle_base
Outdated
| if [[ $type = openstack ]]; then | ||
| # stsstack | ||
| cidr=$(source ~/novarc; openstack subnet show ${OS_USERNAME}_admin_subnet -c cidr -f value 2>/dev/null) | ||
| fi | ||
| if [[ -n $cidr ]]; then | ||
| vip_start=$(echo $cidr| sed -r 's/([0-9]+\.[0-9]+).+/\1/g').150.0 | ||
| fi |
There was a problem hiding this comment.
I think all the above code should be under the 'openstack' if since it is only used in that case, correct? Then you wouldn't need the cidr="" assignment either.
There was a problem hiding this comment.
hi brian, I have added the following code for non-openstack cloud to improve this scene. thanks for the review.
elif [[ -z $vip_start ]]; then
echo "ERROR: No VIP start address provided with --vip-addr-start for the $type cloud."
exit 1
zhhuabj
force-pushed
the
underlying_lxd
branch
from
1579176 to
3b4dfce
Compare
common/generate_bundle_base
Outdated
| @@ -26,6 +27,9 @@ if [[ -z $vip_start ]] && [[ -e ~/novarc ]]; then | |||
| net_end=$(awk -F'.' '/HostMax/{print $NF}' <<<$(ipcalc -b $cidr)) | |||
| vip_start=$(echo $cidr| sed -r 's/([0-9]+\.[0-9]+\.[0-9]+).+/\1/g').$((net_end - 19)) | |||
| fi | |||
| elif [[ -z $vip_start ]]; then | |||
There was a problem hiding this comment.
i think this is going to fail if using maas without vips which i presume is not the intention?
e.g. i did the following:
$ ./generate-bundle.sh --hyperconverged
parse error: Invalid numeric literal at line 1, column 11
parse error: Invalid numeric literal at line 1, column 11
parse error: Invalid numeric literal at line 1, column 11
parse error: Invalid numeric literal at line 1, column 11
ERROR: No VIP start address provided with --vip-addr-start for the cloud.
There was a problem hiding this comment.
ed, I remove that line now, thanks.
$ ./generate-bundle.sh --hyperconverged
ERROR: no default binding provided. Use --default-binding
In fact, stsstack can also work well on lxd cloud. So this patch is to enable stsstack to not only support underlying openstack cloud, but also support underlying lxd and maas cloud.
zhhuabj
force-pushed
the
underlying_lxd
branch
from
3b4dfce to
d408a5d
Compare
The PR canonical#228 [1] changed to use jq instead of sed to parse yaml when relocating two lines. However, yaml format is not supported in versions prior to juju 3.x. $ juju show-cloud lxd --format json ERROR invalid value "json" for option --format: unknown format "json" Therefore, we need to revert back to the previous two lines to support both juju 2.x and juju 3.x simultaneously. [1] canonical#228
The PR canonical#228 [1] changed to use jq instead of sed to parse yaml when relocating two lines. However, yaml format is not supported in versions prior to juju 3.x. $ juju show-cloud localhost --format json ERROR invalid value "json" for option --format: unknown format "json" Therefore, we need to revert back to the previous two lines to support both juju 2.x and juju 3.x simultaneously. [1] canonical#228
The PR canonical#228 [1] changed to use jq instead of sed to parse yaml when relocating two lines. However, yaml format is not supported in versions prior to juju 3.x. $ juju show-cloud localhost --format json ERROR invalid value "json" for option --format: unknown format "json" Therefore, we need to revert back to the previous two lines to support both juju 2.x and juju 3.x simultaneously. [1] canonical#228
The PR canonical#228 [1] changed to use jq instead of sed to parse yaml when relocating two lines. However, yaml format is not supported in versions prior to juju 3.x. $ juju show-cloud localhost --format json ERROR invalid value "json" for option --format: unknown format "json" Therefore, we need to revert back to the previous two lines to support both juju 2.x and juju 3.x simultaneously. [1] canonical#228
In fact, stsstack can work well on lxd cloud as well. So this patch is to enable stsstack to not only support underlying openstack cloud, but also support underlying lxd and maas cloud.