Skip to content

Conversation

@dprince
Copy link
Contributor

@dprince dprince commented Jan 16, 2024

No description provided.

-netapp
```

## Custom Images for Neutron
Copy link
Contributor

Choose a reason for hiding this comment

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

I understand that you are targeting updates process specifically; but this new CR may be helpful in third party neutron ml2 driver integration. Let me know if this is out of scope.


To integrate a custom neutron ml2 driver, one would build a custom image with the driver python package (and any other dependencies). Then this custom image should replace the default one. How do we do it? Are we allowed to override RELATED_IMAGE_..._DEFAULT for a particular deployment? Should there be serviceOverrides in addition to serviceExcludes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Need to update the docs today to reflect recent feedback. But custom ML2 drivers would get handled the same way we do custom Cinder volume backends. The administrator would edit/patch/update the OpenStackVersion spec so that a custom ML2 driver containerImage would get deployed instead of the default.

While an administrator could override RELATED_IMAGE_.. a change like that would be non-persistent on upgrades to the operator. Also, it would involve editing RELATED_IMAGE... values in multiple places (for different operator deployments). I think keeping the customizedImages in a CR might be cleaner.

Copy link
Contributor

Choose a reason for hiding this comment

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

For Cinder we currently set the containerImage manually in the OpenStackControlPlane for the specific cinder volume backend: https://github.com/openstack-k8s-operators/cinder-operator/blob/5e8e748d6046beee57bf3e4271565ff3bb7f8d11/config/samples/backends/hpe/3par/iscsi/backend.yaml#L24

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@booxter I added expanded the section in this doc about Neutron. Still a TBD I think. There is currently no formal interface to specify custom ML2 drivers other than OVN (correct me if I am wrong here). So I think before we can add the ML2 section to this CR we should first define where and how those images would be used. As far as I know every other image we have listed on OpenStackVersion satisfies these conditions.

Copy link
Contributor

@gibizer gibizer left a comment

Choose a reason for hiding this comment

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

Looks good to me.

## Custom ordering of service updates

Container image updates currently occur in parallel for a given CR.
First OVN will be updated on the Dataplane CRs. Then the Controlplane will be updated.
Copy link
Contributor

Choose a reason for hiding this comment

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

note that ovn-controller is running on both OCP workers and EDP nodes, and these should be updated before northd/ovndbclusters (both running on OCP workers only). So the first step for OVN should include both Dataplane CR update and ovnController section update in ControlPlane CR.

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 update the docs to better word this. While there are 2 conditions for OVN Controllers (one for dataplane and one for controlplane) the intent is that they can occur independently. See here: https://github.com/openstack-k8s-operators/openstack-operator/blob/main/controllers/core/openstackversion_controller.go#L162

Then initial PR for OpenStackVersion only had the controlplane parts implemented. Now that we have those we can proceed with implementing things on the Dataplane side too.

First OVN will be updated on the Dataplane CRs. Then the Controlplane will be updated.
Finally the rest of the Dataplane CRs container images will be updated.

If a situation arises where we need to update one service before
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the situation just arised (see ovnController remark).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is well understood that both the OVN controllers on the dataplane and controlplane need to be updated for our initial minor updates support. This comment was really about future unforeseen situations that may arise here. I hope this makes sense...

@ciecierski ciecierski self-requested a review January 31, 2024 15:44
Copy link
Contributor

@Akrog Akrog left a comment

Choose a reason for hiding this comment

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

Thanks for doing a spec/doc on this work.

Comment on lines 63 to 70
In order to allow administrators to configure customized Cinder volume backends
with custom containerImages for each backend you must specify those images
from automatic updates. This is because there is no way to fully automate
the creation of those images at this time. To configure a custom
Cinder volume named 'netapp' updates you can add the name 'netapp' to the
'cinderVolumeCustomImages' map on the OpenStackVersion CR and configure
a custom image for that volume backend. Multiple named Cinder volume backends may
be configured in this manner.
Copy link
Contributor

Choose a reason for hiding this comment

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

There are a couple of things that I would like to update in this text, instead of talking about custom containerImages these would be certified vendor containerImages, also the "specify those images from automatic updates" sounds weird to me, then we talk about creation of images which can be misleading...

I think a reworked paragraph could work better in the line of:

In order to allow administrators to configure customized Cinder volume backends
with certified 3rd party vendor containerImages for any of the backends, 
administrators will be allowed to explicitly specify those images overriding the
default containerImage during automatic updates. This is a manual operation 
because there is no way to fully automate the selection of those images at this
time. To configure a Cinder volume backend named 'pure' with a different image 
during the updates, the administrator can add the name 'netapp' to the
'cinderVolumeCustomImages' map on the OpenStackVersion CR and set a custom image
to be used for that volume backend. Multiple named Cinder volume backends may
be configured in this manner.

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 accepted the text above. I did however change the name 'pure' to netapp. Let me know if this was not correct.

Copy link
Contributor

Choose a reason for hiding this comment

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

NetApp doesn't require a custom container, so it's misleading. I would use Pure as an example since it does need a custom container.

-netapp
```

## Custom Images for Neutron
Copy link
Contributor

Choose a reason for hiding this comment

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

For Cinder we currently set the containerImage manually in the OpenStackControlPlane for the specific cinder volume backend: https://github.com/openstack-k8s-operators/cinder-operator/blob/5e8e748d6046beee57bf3e4271565ff3bb7f8d11/config/samples/backends/hpe/3par/iscsi/backend.yaml#L24

administrators will be allowed to explicitly specify those images overriding the
default containerImage during automatic updates. This is a manual operation
because there is no way to fully automate the selection of those images at this
time. To configure a Cinder volume backend named 'netapp' with a different image
Copy link
Contributor

Choose a reason for hiding this comment

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

NetApp doesn't need a custom cinder-volume container.
Maybe we should replace it with pure which does.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah, I see. I misread your last suggestion. I will switch to pure...

```shell
$ oc get openstackversion
NAME TARGET VERSION AVAILABLE VERSION DEPLOYED VERSION
openstack 1.0.0 1.0.1 1.0.0
Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you for adding the deployed version field. :-)

Copy link
Contributor

@gibizer gibizer left a comment

Choose a reason for hiding this comment

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

still looks good after the recent updates. Thanks for making this doc @dprince.

@dprince dprince merged commit 31f8c6a into openstack-k8s-operators:main Apr 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants