-
Notifications
You must be signed in to change notification settings - Fork 70
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
ADD tls-e support for edpm_nova #588
ADD tls-e support for edpm_nova #588
Conversation
{% endif %} | ||
"/etc/localtime:/etc/localtime:ro", | ||
"/lib/modules:/lib/modules:ro", | ||
"/dev:/dev", | ||
"/run/openvswitch:/run/openvswitch", | ||
"/run/openvswitch:/run/openvswitch:shared", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fyi the only reason i added shared is for parity with libvirt and incase for some reason the nova_compute container stated before ovs on the host
in that case shared should allow nova to see the ovsdb socket when it gets created on the host.
i have not tested this outside of molecule so i will try and create an env with tls enabled. it would be ideal if podified-multinode-edpm-deployment-crc or a similar tempst zuul job could be used to validate this instead. i currently do not have a CRC deploy so it will take me a day or too to test this end to end. |
Build failed (check pipeline). Post https://review.rdoproject.org/zuul/buildset/b15fe6633333470a8350cc0ef48b3ad2 ✔️ openstack-k8s-operators-content-provider SUCCESS in 2h 03m 55s |
Build failed (check pipeline). Post https://review.rdoproject.org/zuul/buildset/ce739d5f15e746ec998ef07941aed9ea ✔️ openstack-k8s-operators-content-provider SUCCESS in 1h 52m 34s |
[libvirt] | ||
live_migration_with_native_tls = true | ||
live_migration_scheme = tls | ||
{% endif %} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, we agreed that we don't have information in nova-operator if TLS is enabled in the dataplane or not, so we cannot provide this information via the cell config. If in the future we make every nova compute config a j2 template then we can move this template logic to the cell config. However we might not want to as that will couple the nova-operator with the edpm ansible vars.
recheck |
Build failed (check pipeline). Post https://review.rdoproject.org/zuul/buildset/650ff5dae0da430dbf9d2b291661feb2 ✔️ openstack-k8s-operators-content-provider SUCCESS in 2h 00m 29s |
Merge Failed. This change or one of its cross-repo dependencies was unable to be automatically merged with the current state of its repository. Please rebase the change and upload a new patchset. |
the logs are now in /var/log/messages i plan to submit a patch to the ci framework or must gather to make them easier to read.. the current error is when os-vif connects to ovs via the UNIX socket ovsdbapp raises a error stating it cant retrieve the schema. I'm not seeing nay deinals with the service logs or in the ovsdb log looks like i need a rebase too so ill do that now then try and reproduce this locally |
This was the error
i have been using lnav -q https://logserver.rdoproject.org/88/588/23fe056c41b9d2bba62646efa8f781f7f847a7f8/github-check/podified-multinode-edpm-deployment-crc/78957db/controller/ci-framework-data/logs/192.168.122.100/log/messages to view them |
Build failed (check pipeline). Post https://review.rdoproject.org/zuul/buildset/cf5a6d0e4d124f7f9e48af0be36a4699 ✔️ openstack-k8s-operators-content-provider SUCCESS in 2h 03m 01s |
The OVN TLS support is only for the central DBs so not relevant here. IIRC OVN manages TLS on it's local OVS using a built-in CA |
(so not sure how nova-compute could previously have been connecting to it over TLS) |
@@ -10,10 +10,13 @@ | |||
}, | |||
"volumes": [ | |||
"/var/lib/openstack/config/nova:/var/lib/kolla/config_files:ro", | |||
{% if edpm_nova_tls_certs_enabled %} | |||
"{{ edpm_nova_tls_ca_src_dir }}/tls-ca-bundle.pem:/etc/pki/CA/cacert.pem:ro", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
isn't that the path to the libvirt CA? The CA bundle should be mounted to /etc/pki/ca-trust/extracted/pem/tls-ca-bundle.pem
(at least that's where it's mounted on the control plane pods)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is the path @vakwetu previously used so i just used the same but i can mount it to
/etc/pki/ca-trust/extracted/pem/tls-ca-bundle.pem
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ack, fixed that in #591, it should have been the libvirt CA, not the bundle (as it's mTLS)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
... not sure why libvirt uses such a generic path for it's CA cert
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ya i asked them and they mentioned it was also probably a poor naming choice. they have libvirt in thename or path for all the rest except that root ca cert.
roles/edpm_nova/defaults/main.yml
Outdated
# will not need the libvirt or ovs client certs or ca. nova will communicate other services | ||
# via there api endpoints and will connect to rabbitmq. To support this we will need to trust | ||
# the general ca root cert. | ||
edpm_nova_tls_ca_src_dir: /var/lib/openstack/cacerts/nova |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
on my deployment it's /var/lib/openstack/cacerts/nova-custom (OpenStackDataPlaneService name). @vakwetu FYI
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hum that was not ment ot change based on the dataplane service name
i don't think we have access to that in ansible
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the dataplane-operator doesn't know anything about whether the service name is nova or nova-custom or whatever. It just knows that there is a dataplane service named X and so it puts its ca bundle under /var/lib/openstack/cacerts/X or certs and keys (and now ca.certs) under /var/lib/openstack/certs/X.
We can fix this in two ways. 1) We can add something to the service spec -- something like "mountAlias" that tells the dataplane-operator to use a different path instead. 2) we can add an ansible param that specifies the service name
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i tought about both 2 would only work if differnt named service shared the same tls ca
i.e. if two libivrt serivce had the same ca
i dont think you can do that automatically
so instead of a mount alias i think we need to extend the dataplane service with a type field.
then use that to generate the crts and deifne the mount point.
so type=ovs, type=libvirt or type=nova
type=nova with name=nova-custom woudl result in the tls certs beign placed in .../nova ignoring the service name "nova-custom"
that woudl map to /var/lib/openstack/cacerts/nova
do we think that is feisable?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, edpm-nova handles this discrepancy between nova and nova-custom by discovering the config files
- name: Discover configmaps in {{ edpm_nova_config_src }} |
install-certs has no such logic. We just copy the right certs, keys, cacerts for the node from /var/lib/openstack/cacerts in the ansibleEE pod to /var/lib/openstack/cacerts on the compute node. Same for /var/lib/openstack/certs.
So, we can solve this either by mounting the certs in the ansibleEE container to the right place (/var/lib/openstack/cacerts/nova) to begin with -- by using the mountAlias thing I just mentioned.
Or we can add an ansible param that provides the service-name and then have the edpm-nova role pick things up from the right path.
Which do you guys prefer?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes so i briefly ran the type idea past gibi and his reaction is that would allow use to add validating/default webhooks for the standard datapalne services to be able to check for example that if you define a dataplane service of type=nova tha the secrete always have a migration ssh key defined.
it would also allow use to enforce that for any node set we only have one datapalneservice for any given type.
the value of type for the standard service would basically map to the last component of the playbook
so type=nova is related to playbook: osp.edpm.nova
we don't have to enforce that but if we are wondering what the type would be for the exiting service that's what i would use.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think option 1 is correct. If we have 2 libvirt services (not sure what the use-case would be but hypothetically....) then they have independent config. If they happen to be using the same CA that's fine, but they should get individual keys/certs/secrets
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
seems to work openstack-k8s-operators/dataplane-operator@dda1a9d with olliewalsh@13b1158
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the service name can solve this problem for this specific usecase so i updated the patch to support that.
i think have a type on the dataplane service so we can use that for validation or defaulting is still worth considering but that can be done separately.
@olliewalsh are you going to submit a pr to the dataplane operator for that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Included in the OVN dataplane PR openstack-k8s-operators/dataplane-operator#745
i just pushed this before i finish for the week. i have it running locally but the package installation time is very long for some reason so I'm just going to leave it run and let ci run on it over the weekend. |
Nova already connected to libvirt via unix socket so it does not need the libvirt tls certs for local communcitaiton. nova does not need to connect to remote libvirt, as a result we do not need to provide access to the libvirt client cert. The 02-nova-host-specific.conf.j2 template has been updated to enable libvirt native tls for live migration when tls is enabled. molecule test coverage is added for the above changes. Closes: OSPRH-5053
check-rdo i added a depends on to openstack-k8s-operators/dataplane-operator#754 to enable tls by default and openstack-k8s-operators/dataplane-operator#745 to pass the edpm_service_name. |
Build failed (check pipeline). Post https://review.rdoproject.org/zuul/buildset/b1f048ca87f2454ba61556f8877128b3 ✔️ openstack-k8s-operators-content-provider SUCCESS in 2h 22m 47s |
check-rdo |
Build failed (check pipeline). Post https://review.rdoproject.org/zuul/buildset/c71c25960c9a4f93b4bee01ab22b2bde ✔️ openstack-k8s-operators-content-provider SUCCESS in 2h 23m 46s |
check-rdo this should not pass i belive as podified-multinode-edpm-deployment-crc now passes in openstack-k8s-operators/dataplane-operator#754 |
Build failed (check pipeline). Post https://review.rdoproject.org/zuul/buildset/d679e2684ecb4a6ba0470c71edd3b324 ❌ openstack-k8s-operators-content-provider FAILURE in 10m 19s |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: olliewalsh, 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 |
check-rdo |
fcbb4f2
into
openstack-k8s-operators:main
Nova already connected to libvirt via unix socket so it does not
need the libvirt tls certs for local communcitaiton. nova does
not need to connect to remote libvirt, as a result we do not need
to provide access to the libvirt client cert.
The 02-nova-host-specific.conf.j2 template has been updated to
enable libvirt native tls for live migration when tls is enabled.
molecule test coverage is added for the above changes.
Closes: OSPRH-5053