Jump to: navigation, search

Difference between revisions of "CinderXenaPTGSummary"

m (add "Fix up volume driver classes")
m (add "Cinder throttle and cgroup v2")
Line 214: Line 214:
  
 
===Cinder throttle and cgroup v2===
 
===Cinder throttle and cgroup v2===
 +
Cinder has been using cgroup v1 for (optional, operator-configured) throttling for a long time.  cgroup v2 has been around for a long time as well, but there hasn't been a big reason to switch to v2 ... until Fall 2020, when container systems began supporting v2.  So now linux distributions don't have a reason to worry about v1 any more, and many are making v2 the default and talking about removing v1 support all together.
 +
 +
Given the above, it looks like we can just switch to using cgroup v2 and not worry about also supporting v1.
 +
 +
The [https://github.com/libcgroup/libcgroup libcgroup library] has code merged (commits 9fe521f, 8fb91a0, da13073)--but not yet released--that adds cgroup v2 support to the cgroup tools we are currently using, so once that's released, we'll be able to use our current code (with a minor change for the different control group name used for i/o in v2).
 
====Conclusions====
 
====Conclusions====
 +
* We should move cinder to using cgroup v2 in Xena; further, there is no reason to worry about v1-compatibility
 +
* rosmaita check on the status of of a new libcgroup release and report back at the R-18 midcycle (the current release is 0.42.0, and 2.0rc1 was just tagged, so we should be able to get this done in cinder before M-3 for sure)
  
 
===Cross-project meeting with Nova===
 
===Cross-project meeting with Nova===

Revision as of 18:39, 27 April 2021

Contents

Introduction

This page contains a summary of the subjects covered during the Cinder project sessions at the Project Team Gathering for the Xena development cycle, held virtually April 19-23, 2021. The Cinder project team met from Tuesday 20 April to Friday 23 April, for 3 hours each day (1300-1600 UTC), with Friday's sessions stretching to 4 hours.

The Cinder Team at the Xena (Virtual) PTG, April 2021.


This document aims to give a summary of each session. More context is available on the cinder PTG etherpad:


The sessions were recorded, so to get all the details of any discussion, you can watch/listen to the recording. Links to the recordings are located at appropriate places below.

Tuesday 20 April

recording

Greetings and some Cinder project business

Business in no particular order:

  • The final release from stable/train for all deliverables is 12 May 2021, so check for any critical bugs that should be backported and get those moving along right away.
  • We need to do a release from the rbd-iscsi-client to verify that all the tooling is working. Want to do this before any critical bug is discovered that will require a release. The patch formatting rbd-iscsi-client as an OpenStack project still needs reviews: https://review.opendev.org/c/openstack/rbd-iscsi-client/+/774748/
  • There's a patch up placing cinder-specific dates on the OpenStack release schedule. The only notable change from the usual dates is to move the spec freeze to 2 weeks after the R-18 midcycle. (One week didn't give people enough time to revise and resubmit, which required a bunch of spec freeze exceptions. Hopefully we can avoid that this time.) Please check over the dates and leave comments if you see any problems: https://review.opendev.org/c/openstack/releases/+/786951
  • The two midcycles at weeks R-18 and R-9 have been working well, so let's do that again. Format will be 2 hours. I propose holding them in the place of the cinder weekly meeting that week; that way, everyone should be able to attend for at least the one hour they have set aside for the cinder meeting. Time would probably be 1300-1500 UTC. Please check for conflicts:
    • R-18 midcycle: 2 June
    • R-9 midcycle: 4 August
  • Some quick links to be aware of
    • general info on cinder project: tiny.cc/cinder-info
    • cinder group info: tiny.cc/cinder-groups
  • We've got someone interested in helping with documentation. I've asked her to start by reviewing release notes on patches for clarity, grammar, etc. These are user-facing documents so it's helpful to get them correct.
  • We didn't schedule time for a Wallaby Release Cycle Retrospective, so maybe we can do that during happy hour today.

Not possible for a non-admin user to determine via API if a volume will be multi-attach

Here's the context for the discussion: https://github.com/kubernetes/cloud-provider-openstack/pull/1368#issuecomment-819442191 and the comments following that one.

Basic idea is that cloud end users want to run kubernetes in OpenStack clouds. The kubernetes installation "operator" (kubernetes term, we're talking about a program, not the human operator of the OpenStack cloud) for an end user thus runs with normal OpenStack project credentials (not admin). For this particular issue, the kubernetes installer program wants to allow multi-attach block devices if the underlying cinder in that cloud supports it. (It does this by looking at the volume types available to the end user and creating a kubernetes storage class for each volume type.) Multiattach info is in volume-type extra-specs that are only visible to admins (because they could contain backend-specific info about the deployment that shouldn't be exposed to end users). So currently the only way for the installer to determine if multi-attach is possible is to create a volume of each available volume-type and to check if the resulting volume is multiattach. This is non-optimal (some might say time-consuming and stupid).

The issue of exposing non-sensitive extra-specs info to end users has come up before in the history of cinder, and the suggestion was to either name volume-types appropriately (e.g., "Gold level with multiattach") or to include this info in the volume-type description. But this is variable from cloud to cloud and not suitable for machine consumption. It would be better to have some kind of standard field for this in the volume-type-show response.

A few issues came up in the course of this discussion:

  • We should probably provide some clear documentation around this. For instance, if you have a volume-type that can match multiple backends, whether a built volume is multiattach will depend both on (a) whether the volume type supports multi-attach, and (b) the scheduler puts the volume on a backend that also supports multiattach (which also depends on: (c) how you've configured the scheduler). So to support this use case (or, really, any use case where an end user wants to know whether a volume will be multiattach without having to build a volume first), a cinder admin needs to make sure that volume-types supporting multiattach are tied to backends that also support it (and the scheduler is configured appropriately).
    • but what if the backend runs out of space? Well, that's ok, the end user won't be able to create a volume in it. But that seems better than getting a volume that doesn't have the properties you want.
    • this didn't come up in discussion, but it looks like there's a tension between having very general volume types (e.g., could be multiattach, but maybe not, but as long as there's space on some backend, you'll get a volume) and very specific types (e.g., only has multiattach <is> True extra spec if all the backends that map to the type support multiattach). If we're just going to expose the multiattach extra spec, then a cinder admin can't have a general type that supports multiattach when available, because that won't meet the kubernetes use case we're discussing. Now maybe that's not a big deal, because for multiattach, it seems like if you want multiattach for your workload, you really want it, and so it would be kind of pointless to have a multiattach volume-type that sometimes didn't give you a multiattach volume. But I wonder if that's the case for other non-sensitive extra-specs. What I'm getting at is I wonder whether what we want is to introduce volume-type-metadata where a cinder admin could expose whatever info the admin deems suitable for end users. The downside is that the admin would have to configure info twice, once in extra-specs and once in volume-type-metadata, but it would also make it possible to add key/values for particular consumers (e.g., "k8s:supports_multiattach": "true" (these would have to be string values), "k8s:whatever": "something", or even metadata indicating that the volume-type should be used by kubernetes at all). The upside is that the key/values could be defined by the kubernetes-cinder-csi operator, not cinder.
  • Manila currently exposes a set of backend capabilities to end users while keeping others secret: https://docs.openstack.org/manila/latest/admin/capabilities_and_extra_specs.html
  • This is related to the capabilities reporting that Eric discussed at the Denver PTG and put up an initial spec for: https://review.opendev.org/c/openstack/cinder-specs/+/655939
    • the basic idea is that although there's capabilities reporting defined (and implemented in some drivers), it's not specific enough to help an operator to write extra-specs (e.g., supports_qos: True doesn't help you write qos extra-specs for a backend (though it does tell you to look at the driver docs or code to figure out what to do))
    • implementing this wouldn't help the kubernetes use case directly (because this capabilities API wouldn't be exposed to end users), but getting it defined could help in the effort to identify suitable backend-capabilities that could later be exposed to end users (or at least developing a suitable vocabulary)
  • Walt pointed out that we currently expose some volume-admin-metadata to the cinder admin in the volume-show response, so that could be a model for what we do here, expose select extra-specs to end users in the volume-type-show response
  • might be better to add an API to ask what volume types support a feature the end user is interested in
  • useful capabilities to expose:
    • multiattach
    • availability zones
    • volume replication
    • online extend capability
    • QoS
    • might want to make this configurable (sort of like resource_filters) so cinder admin can decide what to expose?
  • this would be useful to Horizon or other UIs -- for example, don't offer an end user to try to do an extend of an attached volume of the volume-type doesn't support it

Conclusions

  • This would be a good addition to Cinder
  • For the OpenShift/kubernetes use case, exposing some extra-specs in a standard format to non-admin users is sufficient.
  • Matt Booth and Tom Barron will draft a spec
  • the Cinder team will review and help keep the spec moving along
  • Eric will resuscitate his capabilities API spec; the team expressed general support for it

The Interoperability Working Group

Part of what the Interop WG does is to determine a set of tests that cloud operators can run to demonstrate the portability of workloads across OpenStack clouds. Vendors who pass the tests can use the OpenStack logo. Currently, these are a specific subset of tempest tests defined in the interop git repository.

Latest: https://opendev.org/osf/interop/src/branch/master/2020.11.json

The idea is that the tests cover two releases back and one forward. So the 2020.11 guidelines cover ussuri and victoria and wallaby.

You can have "required" or "advisory" capabilities. The tests for "required" capabilities must pass for a vendor to be allowed to use the logo. A capability must be "advisory" for at least one cycle before it can become required.

The Interop WG is interested in learning what new features have been added to Cinder in Wallaby that should be included as advisory capabilities. What they're looking for is anything that might impact workload portability.

If we identify such a capability, we can write a tempest test for it, and then propose it to the Interop WG for inclusion as an advisory capability. What happens is that vendors use refstack to run the tests in their clouds. Refstack allows submission of results directly to the Foundation. That way, the Interop WG can get data about advisory capabilities (for example, if those aren't being exposed in OpenStack clouds, then they may not be candidates for becoming "required").

Conclusions

  • The current tempest tests cover at most the 3.0 microversion.
  • Many of the subsequent microversions make the Block Storage API more user friendly, but may not impact workload portability.
  • Cinder team: will review mv changes between 3.0 and 3.64 to see if any "advisory" tests should be added
  • Arkady: will follow up with the cinder team

Wallaby cycle retrospective

We clearly have a review bandwidth problem and need to start looking for new cores to help us out.

People currently working with Cinder who are interested should reach out to the PTL (rosmaita) or any of the current cores to get some guidance both about what we'd like to see from the Cinder side as well as some guidance about how to approach your management to get yourself sufficient time to devote to the project.

Gorka asked whether we could have "specialist" cores who have expertise in specific areas. We actually have a precedent for this with volume drivers. Lucio (who unfortunately for us switched jobs) had done a lot of high-quality driver reviews, and the core team agreed that although he wasn't experienced with all of Cinder yet, he could continue to develop that knowledge and only +2 patches that he was comfortable with. This allowed us to increase review bandwidth without sacrificing review quality.

So the point is don't feel like you have to know all of Cinder before even thinking about becoming a core. Similarly, current cores should keep this in mind when looking for candidates.

Conclusions

  • Current cores should keep a watch for people who might be helpful. Since this is a personnel matter, I'd like to keep a bit of confidentiality here, so circulate an email only to the current cores (that is, not on the mailing list) so we can discuss among ourselves and the PTL can contact the candidate directly with constructive feedback. Of course, when we reach the stage that someone's been identified as a serious core candidate, we will have the usual vote in public on the mailing list.
  • Some tips for core hopefuls:
    • don't leave +1 or -1 without comments on patches ... say something about what you looked at in the patch. If the code looks great, check the unit tests. If they look great too, check the test coverage report. If it's a driver, check the third-party CI results. You may not find anything negative, which is perfectly OK, but leave something more than your +1 indicating what's good about the patch
    • look at -1s left by core reviewers to see what kind of issues they're looking for when reviewing patches

Wednesday 21 April

recordings

Removing the Block Storage API v2

The Block Storage API v2 was deprecated in Pike, and has been eligible for removal for quite a while. It didn't happen in Wallaby, and the lesson learned there is that it must be removed by Milestone-1, or it won't happen.

Xena Milestone-1 is week R-19 (the week of 24 May), so roughly 1 month away.

We've got two deliverables affected by this: the python-cinderclient, and cinder itself.

python-cinderclient

We proposed removing v2 support from the cinderclient in a series of emails to the ML in Fall 2020:

There were no objections raised, so we should be good to go.

This will happen in a series of patches:

  1. there's a client class that gives you a v2 or v3 client; change this to return a v3 client only
  2. remove the v2 client class completely and revise tests
  3. remove any v2 classes that only import v3 classes and revise tests
  4. for any v3 classes that simply import the v2 class, move the v2 class to v3 and revise any affected tests
  5. for any remaining v2 classes, replace 'v2' in the module name with 'internal' (or something else that indicates that they aren't supposed to be used directly by consumers) and revise tests

cinder

Unlike the cinderclient v2 classes, that are specifically designed to be used by consumers as python language bindings for the Block Storage API v2, the cinder v2 classes aren't meant to be exposed to consumers. Thus it's not critical to remove/rename those classes (at least not right now), so we can just leave those as they are. (This will also make backports easier.)

What we need to do for cinder, then, is:

  1. no longer return v2 as a version option
  2. return 404 when the v2 API is requested (or whatever we currently do for v1)
  3. update tests
  4. the v2 section of the api-ref will need to be removed
  5. documentation update to remove v2 references

other teams

Impact on other teams:

  • osc/sdk
    • osc has code that checks for cinderclient v1 by trying to do an import from whatever cinderclient it's got available; could put up a patch with same check for v2
    • openstacksdk currently only supports v2 and v3; it's branched, so could remove v2 from the xena development branch (looks like the v3 classes have no dependencies on the v2 classes)
  • keystone/devstack
    • will need to remove the v2 endpoint from the service catalog (probably more a devstack thing than a keystone thing?)
  • nova
    • verify that they're using v3 (checked during the cross-project session with the nova team; they have no v2 dependency)
  • ironic
  • triple-O
    • alan has already removed v2 dependency/support
  • general
    • send another general announcement to the ML saying that we really mean it this time


Conclusions

  • cinderclient: someone posed this question: "Since this is a backwards incompatible change, how are we going to prevent deployments that still have v2 from breaking with a pip -U ?"
    • I don't know that there's anything we can do other than send a message to the mailing list announcing that version 8 of the python-cinderclient will no longer support the Block Storage API v2, and if anyone requires v2 support, any occurrence of 'pip install -U python-cinderclient' in a script (or written instructions) should be replaced with "pip install -U 'python-cinderclient<8.0.0'"
  • rosmaita will post the python-cinderclient patches soon-ish
  • rosmaita will coordinate with enriquetaso and whoami-rajat (and anyone else interested) about the cinder-side changes
  • rosmaita will check with osc/sdk team
  • rosmaita will check with qa team about devstack
  • rosmaita will send general announcement to the ML

mypy status and next steps

Eric reported that there are still some cinder mypy patches unreviewed, and some new os-brick patches: https://review.opendev.org/q/(project:openstack/cinder+OR+project:openstack/os-brick)+topic:%2522mypy%2522+(status:open)

The pile of patches is getting large and they depend on each other, so it's a PITA to keep them current. We decided to do a review festival so everyone can set aside some time to review them.

Eric reminded us that we're not shooting for full coverage right now, we're just trying to cover as much as possible with minimal interruptions and hit the interesting cases first. Also, these changes aren't likely to impact the way the code is running now.

There are some open issues:

  • still need to figure out how to handle cinder oslo versioned objects
  • need to figure out how to handle libraries that we import that don't have annotations
  • Eric has a patch up to automatically add annotations to oslo libraries that we use, though he's not sure if this is a good long-term solution


Conclusions

Quotas testing

Gorka fixed a bunch of long-standing quotas bugs in Wallaby and has some ideas for simplifying the code and fixing some more. It would be good to have some testing to give the code a workout and be able to detect regressions.

Ivan had suggested that we could use Rally to test quotas, but he couldn't find a good example for using it to do what we want. Also, we probably want high concurrency, and Rally doesn't look very concurrent.

Some possible tests:

  • Need multiple users doing things at the same time.
  • Create/delete volumes and snapshots
  • Probably want to start with artificially low quotas.
  • Not sure how much space we have to test in the check ... but if we use the fake driver, that's not an issue
    • Ivan isn't sure that the fake driver is still working, but it should be easy to fix if there are issues
  • Manage/unmanage testing
    • this functionality would have to be added to the fake driver, so we can hold off on this for now


Gorka mentioned that some of the bugs he fixed are related to temporary resources, so maybe we can add code to do a whole tempest run and validate that the quotas are correct at the end. Ivan mentioned that he's not sure that the current tempest tests clean everything properly, and we want to make sure we're testing cinder quotas, not tempest. (Gorka mentioned later that tempest cleanup doesn't matter for these tests. Cinder is in charge of creating/deleting temporary resources, so even if the "real" resources aren't cleaned up afterward by tempest, if what we're interested in is making sure temporary resources are correctly counted, we should be ok.)

Someone asked about the status of Keystone Unified Limits, but no one has looked into that recently.

Conclusions

  • start with adding quota checking to the tempest jobs
  • Ivan will continue to look at Rally

Fix up volume driver classes

The situation is:

  • we have some abstract driver classes, namely, BaseVD, CloneableImageVD, MigrateVD, ManageableVD, and ManageableSnapshotsVD
  • we also have a concrete driver class VolumeDriver that inherits from ManageableVD, CloneableImageVD, ManageableSnapshotsVD, MigrateVD, BaseVD
  • most of the in-tree third party drivers just inherit from VolumeDriver
  • plus we have the interface classes that are used to test that drivers implement the required functions

Breaking out functionality into the various abstract VD classes doesn't seem to have helped much, and makes it confusing for new driver implementers. It would be better to go back to just having the VolumeDriver.

So, how can we remove the abstract VD classes? First step would be to deprecate them. We can fix any impacted in-tree drivers, but there could be out-of-tree drivers that will be impacted.

Conclusions

  • rosmaita send a note to the mailing list to warn out of tree driver maintainers that this is coming. See if we get a response.

Cinder throttle and cgroup v2

Cinder has been using cgroup v1 for (optional, operator-configured) throttling for a long time. cgroup v2 has been around for a long time as well, but there hasn't been a big reason to switch to v2 ... until Fall 2020, when container systems began supporting v2. So now linux distributions don't have a reason to worry about v1 any more, and many are making v2 the default and talking about removing v1 support all together.

Given the above, it looks like we can just switch to using cgroup v2 and not worry about also supporting v1.

The libcgroup library has code merged (commits 9fe521f, 8fb91a0, da13073)--but not yet released--that adds cgroup v2 support to the cgroup tools we are currently using, so once that's released, we'll be able to use our current code (with a minor change for the different control group name used for i/o in v2).

Conclusions

  • We should move cinder to using cgroup v2 in Xena; further, there is no reason to worry about v1-compatibility
  • rosmaita check on the status of of a new libcgroup release and report back at the R-18 midcycle (the current release is 0.42.0, and 2.0rc1 was just tagged, so we should be able to get this done in cinder before M-3 for sure)

Cross-project meeting with Nova

Conclusions

Thursday 22 April (Drivers' Day)

recordings

Using Software Factory for Cinder Third Party CI

Conclusions

NVMe-oF and MDRAID replication approach - next steps for connector and agent

Conclusions

How to handle retyping/migrating nonencrypted volumes to encrypted volumes of the same size

Conclusions

Small topics

What gate tests need to be done for A/A coverage of a driver?

Using cinder oslo-versioned-objects (OVOs) in tests instead of dicts

Friday 23 April

recordings

Market trends and new cinder features

Conclusions

Snapshotting attached volumes

Conclusions

Multiple volumes goes to a same backend/pool as scheduler intances (HA) use only its internal state to balance the volumes among pools

Conclusions

Making the backup process asynchronous

Conclusions

Several small topics

Volume list queried by error state should contain error_managing state

Support revert any snapshot to the volume

Update volume az after service restart when the volume backend az was changed

OpenStack client still doesn't support cinder microversions

Conclusions

Consistent and Secure RBAC

Conclusions