Skip to content

Add internalapi routes for scenario A adoption #995

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Add internalapi routes for scenario A adoption #995

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,9 @@ NETWORK_STORAGE_ADDRESS_PREFIX ?= 172.18.0
NETWORK_TENANT_ADDRESS_PREFIX ?= 172.19.0
NETWORK_STORAGEMGMT_ADDRESS_PREFIX ?= 172.20.0
NETWORK_DESIGNATE_ADDRESS_PREFIX ?= 172.28.0
# Additional routes that will be forwarded to nncp and net-attach-def
INTERNALAPI_HOST_ROUTES ?=
STORAGE_HOST_ROUTES ?=
TENANT_HOST_ROUTES ?=
STORAGEMGMT_HOST_ROUTES ?=

# network isolation
NETWORK_ISOLATION ?= true
Expand Down Expand Up @@ -2395,6 +2394,8 @@ netattach: export DESIGNATE_PREFIX=${NETWORK_DESIGNATE_ADDRESS_PREFIX}
netattach: export VLAN_START=${NETWORK_VLAN_START}
netattach: export VLAN_STEP=${NETWORK_VLAN_STEP}
netattach: export CTLPLANE_IP_ADDRESS_PREFIX=${NNCP_CTLPLANE_IP_ADDRESS_PREFIX}
netattach: export NETATT_INTERNALAPI_HOST_ROUTES=${INTERNALAPI_HOST_ROUTES}
netattach: export NETATT_TENANT_HOST_ROUTES=${TENANT_HOST_ROUTES}
netattach: namespace ## Creates network-attachment-definitions for the networks the workers are attached via nncp
$(eval $(call vars,$@,netattach))
bash scripts/gen-netatt.sh
Expand Down
26 changes: 24 additions & 2 deletions scripts/gen-netatt.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,8 +120,19 @@ if [ -n "$IPV4_ENABLED" ]; then
cat >> ${DEPLOY_DIR}/internalapi.yaml <<EOF_CAT
"range": "${INTERNALAPI_PREFIX}.0/24",
"range_start": "${INTERNALAPI_PREFIX}.30",
"range_end": "${INTERNALAPI_PREFIX}.70"
"range_end": "${INTERNALAPI_PREFIX}.70",
EOF_CAT
# In the data-plane adoption scenario A (where different IP subnet ranges
# between next-gen and wallaby are used) the net-attach-def needs additional routes.
Copy link
Contributor

@karelyatin karelyatin Jan 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should allow generic routes to be specified set with parameters, by default non will be set and by exporting env vars those could be set.

Copy link
Contributor Author

@averdagu averdagu Jan 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea, implementing it now

Copy link
Contributor

@karelyatin karelyatin Jan 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wanted to have this generic for all the networks not just internalapi
Also can't we reuse other existing vars *_HOST_ROUTES instead of creating similar var?

if [ -n "$NETATT_INTERNALAPI_HOST_ROUTES" ]; then
cat >> ${DEPLOY_DIR}/internalapi.yaml <<EOF_CAT
"routes": [
{
"dst": "${NETATT_INTERNALAPI_HOST_ROUTES}"
}
]
EOF_CAT
fi
elif [ -n "$IPV6_ENABLED" ]; then
cat >> ${DEPLOY_DIR}/internalapi.yaml <<EOF_CAT
"range": "fd00:bbbb::/64",
Expand Down Expand Up @@ -188,8 +199,19 @@ if [ -n "$IPV4_ENABLED" ]; then
cat >> ${DEPLOY_DIR}/tenant.yaml <<EOF_CAT
"range": "${TENANT_PREFIX}.0/24",
"range_start": "${TENANT_PREFIX}.30",
"range_end": "${TENANT_PREFIX}.70"
"range_end": "${TENANT_PREFIX}.70",
EOF_CAT
# In the data-plane adoption scenario A (where different IP subnet ranges
# between next-gen and wallaby are used) the net-attach-def needs additional routes.
if [ -n "$NETATT_TENANT_HOST_ROUTES" ]; then
cat >> ${DEPLOY_DIR}/tenant.yaml <<EOF_CAT
"routes": [
{
"dst": "${NETATT_TENANT_HOST_ROUTES}"
}
]
EOF_CAT
fi
Copy link
Contributor

@karelyatin karelyatin Feb 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what about storage and storage_mgmt networks?

Copy link
Contributor Author

@averdagu averdagu Feb 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As per discussion a while back on nova channel, it is not possible to modify this two networks during dataplane adoption

Copy link
Contributor

@karelyatin karelyatin Feb 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The route in net-attach is generic, not specific to adoption, just needed for adoption case. so having it generic for all involved network is good.

Copy link
Contributor

@karelyatin karelyatin Feb 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CI fails as malformed json is generated, i think that could be handled by always adding the empty routes section, and when overrides are provided add to the routes section ?
Also i think let's be generic, i.e the patch should be Allow passing routes to all the network attachments including storage/storagemgmt etc. Similar to what being done for nncp. And remove the comments specific to adoption scenarios.
Dataplane scenario is just an use case of this support.

Copy link
Contributor Author

@averdagu averdagu Feb 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Found out why json was malformated, handled already.
Also I added the ability to pass routes to all networks (storage/storagemgmt) as on the nncp section.
Still missing dataplane and designate. don't know if I should also add it

elif [ -n "$IPV6_ENABLED" ]; then
cat >> ${DEPLOY_DIR}/tenant.yaml <<EOF_CAT
"range": "fd00:dddd::/64",
Expand Down