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

[edpm_nova]Configure my_ip to ctlplane_ip for nova-compute #478

Conversation

gibizer
Copy link
Contributor

@gibizer gibizer commented Oct 25, 2023

This is needed as nova-compute, if my_ip is not configured, tries to figure out its own IP address in complicated way and in a system with multiple network interfaces there is no guarantee that it selects the IP of the designated control plane interface.

The my_ip configuration is EDPM node specific so it cannot be generated globally for a cell in nova-operator. So this is generated by ansible now.

There is an upstream nova blueprint to eventually get rid of the need of this configuration: https://blueprints.launchpad.net/nova/+spec/libvirt-migrate-with-hostname-instead-of-ip

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Oct 25, 2023

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@gibizer gibizer marked this pull request as ready for review October 25, 2023 18:01
@openshift-ci openshift-ci bot requested review from bshephar and slagle October 25, 2023 18:01
@gibizer gibizer requested review from SeanMooney and removed request for slagle and bshephar October 25, 2023 18:01
@@ -0,0 +1,2 @@
[DEFAULT]
my_ip = {{ ctlplane_ip }}
Copy link
Contributor

Choose a reason for hiding this comment

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

so... the main openquestion i have is should we introduce an ansible var to select what interface this should be or hardcode the control plane.

for context this IP will need to be included in certs and or know hosts files so we likely need a way to align this to one of the ansibel managed interface rather then just the default route.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will look into making the nova management network configurable in the role and default it to ctlplane

Copy link
Contributor Author

@gibizer gibizer Nov 8, 2023

Choose a reason for hiding this comment

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

For the sake of progress I suggest to merge this as is and get back to extend configuration later. I noted the possible future improvement inline now.

@SeanMooney
Copy link
Contributor

my_ip in nova is basically the ip of the interface with the default route.
that is not in general garunetted to be the contolplane interface so i agree this is required if we want it to be the control plane interface

i have one openquestion in line so not approving for now until we agree on if we need this to be configurable as an ansible var (which would default to the contolplane IP)

@rebtoor
Copy link
Contributor

rebtoor commented Oct 27, 2023

my_ip in nova is basically the ip of the interface with the default route. that is not in general garunetted to be the contolplane interface so i agree this is required if we want it to be the control plane interface

If we need to retrieve the ip of the interface with the default route, what about using ansible_default_ipv4?

edit: obviously we need to be sure to have that fact first (an ansible.builtin.setup task retrieving only the network subset of facts, for example)

i have one openquestion in line so not approving for now until we agree on if we need this to be configurable as an ansible var (which would default to the contolplane IP)

@@ -0,0 +1,2 @@
[DEFAULT]
my_ip = {{ ctlplane_ip }}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will look into making the nova management network configurable in the role and default it to ctlplane

@gibizer
Copy link
Contributor Author

gibizer commented Oct 29, 2023

my_ip in nova is basically the ip of the interface with the default route. that is not in general garunetted to be the contolplane interface so i agree this is required if we want it to be the control plane interface

If we need to retrieve the ip of the interface with the default route, what about using ansible_default_ipv4?

edit: obviously we need to be sure to have that fact first (an ansible.builtin.setup task retrieving only the network subset of facts, for example)

In the contrary, we don't want the nova default behavior (which is looking up the default route from the host) to be used as that is not guaranteed to be the IP we need to ssh between the compute nodes. So we want to set this explicitly. By default to the ctlplane_ip but maybe we want to make it configurable so the network can be defined to be used to sshe between computes.

@gibizer gibizer force-pushed the nova-host-specific-config branch from 3843382 to 9bc9fe9 Compare November 8, 2023 14:45
# remove this host specific configuration in the future (not earlier than
# openstack Caracal)
# https://blueprints.launchpad.net/nova/+spec/libvirt-migrate-with-hostname-instead-of-ip
- {"src": "02-nova-host-specific.conf.j2", "dest": "02-nova-host-specific.conf"}
Copy link
Contributor

Choose a reason for hiding this comment

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

+1

This is needed as nova-compute, if my_ip is not configured,
tries to figure out its own IP address in complicated way and in a
system with multiple network interfaces there is no guarantee that
it selects the IP of the designated control plane interface.

The my_ip configuration is EDPM node specific so it cannot be generated
globally for a cell in nova-operator. So this is generated by ansible
now.

There is an upstream nova blueprint to eventually get rid of the need of
this configuration:
https://blueprints.launchpad.net/nova/+spec/libvirt-migrate-with-hostname-instead-of-ip
@gibizer gibizer force-pushed the nova-host-specific-config branch from 9bc9fe9 to 819977e Compare November 9, 2023 14:15
@openshift-ci openshift-ci bot removed the lgtm label Nov 9, 2023
Copy link
Contributor

@SeanMooney SeanMooney left a comment

Choose a reason for hiding this comment

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

reapproving after the rebase

@openshift-ci openshift-ci bot added the lgtm label Nov 9, 2023
Copy link
Contributor

openshift-ci bot commented Nov 9, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: gibizer, SeanMooney

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

@openshift-merge-bot openshift-merge-bot bot merged commit 24462ed into openstack-k8s-operators:main Nov 9, 2023
29 checks passed
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