Skip to content
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.

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

[netconfig] limit network names to be one of the known #272

Closed
wants to merge 1 commit into from

Conversation

stuggi
Copy link
Contributor

@stuggi stuggi commented Sep 11, 2024

network names should be one of

  • ctlplane
  • internalapi
  • external
  • tenant
  • storage
  • storagemgmt

The dataplane is creating the inventory based on the network name and passes ip information as ansible vars like e.g. tenant_ip.

If a custom network name is used, as a follow up custom information needs to be passed into the services. Therefore only allow network names to be one of the expected, while the subnet names can be customized.

network names should be one of
- ctlplane
- internalapi
- external
- tenant
- storage
- storagemgmt

The dataplane is creating the inventory based on the network name
and passes ip information as ansible vars like e.g. tenant_ip.

If a custom network name is used, as a follow up custom information
needs to be passed into the services. Therefore only allow network
names to be one of the expected, while the subnet names can be
customized.

Signed-off-by: Martin Schuppert <[email protected]>
@openshift-ci openshift-ci bot requested review from dprince and viroel September 11, 2024 10:36
@stuggi stuggi requested review from rabi and removed request for dprince and viroel September 11, 2024 10:36
Copy link
Contributor

openshift-ci bot commented Sep 11, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: stuggi

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@stuggi
Copy link
Contributor Author

stuggi commented Sep 11, 2024

if custom network names are used you'd have to check any role and pass in the correct information like e.g. https://github.com/openstack-k8s-operators/edpm-ansible/blob/c21563b42bcb15c6ee1c16cc991a9b2717e24728/roles/edpm_ovn/defaults/main.yml#L54

@rabi
Copy link
Contributor

rabi commented Sep 11, 2024

if custom network names are used you'd have to check any role and pass in the correct information like e.g. https://github.com/openstack-k8s-operators/edpm-ansible/blob/c21563b42bcb15c6ee1c16cc991a9b2717e24728/roles/edpm_ovn/defaults/main.yml#L54

As discussed in slack, we don't have to do above, if we've some kind of mapping in netconfig and use it. Though I'm ok, if we decide not to support custom network names and it has no future implications.

apiVersion: network.openstack.org/v1beta1
kind: NetConfig
metadata:
name: netconfig
spec:
networks:

  • name: net1
    serviceNet: ctlplane
    ...
  • name: net2
    serviceNet: internalapi

@stuggi
Copy link
Contributor Author

stuggi commented Sep 11, 2024

ok this version will introduce an issue with https://github.com/openstack-k8s-operators/architecture/blob/a2f05988589a26c4a12d660c236b8cf337c1683b/examples/va/hci/edpm-pre-ceph/nodeset/values.yaml#L125-L126 where a custom swift network got added

then we could do either

  • what Rabi proposed in Add ServiceNetwork attribute to networks in netconfig #271 with the downside that a user would have to know details of the roles and what vars of the role to updated with custom network name information.
  • my other initial proposal when we were discussing this, to have a network type parameter which you can set on the network to reflect that this is ctlplane, tenant, internalapi or whatever. if no type is set the network name is used in the reservation.

What I do not like is that a user would have to read all the roles to see what information should be passed in when we do not have the default network names.

@abays @rabi @hjensas what are your thoughts?

@stuggi
Copy link
Contributor Author

stuggi commented Sep 11, 2024

if custom network names are used you'd have to check any role and pass in the correct information like e.g. https://github.com/openstack-k8s-operators/edpm-ansible/blob/c21563b42bcb15c6ee1c16cc991a9b2717e24728/roles/edpm_ovn/defaults/main.yml#L54

As discussed in slack, we don't have to do above, if we've some kind of mapping in netconfig and use it. Though I'm ok, if we decide not to support custom network names and it has no future implications.

apiVersion: network.openstack.org/v1beta1 kind: NetConfig metadata: name: netconfig spec: networks:

  • name: net1
    serviceNet: ctlplane
    ...
  • name: net2
    serviceNet: internalapi

yes thats basically same as the network type I mentioned initially

@hjensas
Copy link
Contributor

hjensas commented Sep 11, 2024

So we don't have composable networks and a service net map to allow the user to specify what network to use for what service like we did with tripleo? If we lock down the names like this how do we add for example a network used for Manila NFS (What was called GaneshaNetwork in tripleo)?

@stuggi
Copy link
Contributor Author

stuggi commented Sep 11, 2024

So we don't have composable networks and a service net map to allow the user to specify what network to use for what service like we did with tripleo? If we lock down the names like this how do we add for example a network used for Manila NFS (What was called GaneshaNetwork in tripleo)?

yes thats a use case I was not thinking about. @rabi has a new proposal in #271 to add a ServiceNetwork parameter.

@stuggi stuggi closed this Sep 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants