You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Chun-Hung Hsiao <ch...@apache.org> on 2019/03/20 06:15:04 UTC
Review Request 70247: Added an "unversioned" `VolumeCapability`.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70247/
-----------------------------------------------------------
Review request for mesos, Benjamin Bannier, James DeFelice, Jie Yu, and Jan Schlicht.
Bugs: MESOS-9625
https://issues.apache.org/jira/browse/MESOS-9625
Repository: mesos
Description
-------
To support both CSI v0 and v1, the "unversioned" `VolumeCapability` is
designed to satisfy the following compatibility guarantees:
1. It can be parsed from the JSON representation of a CSI v0 or v1
`VolumeCapability`. This is for making the `UriDiskProfileAdaptor`
backward compatible, and the profile service can simply provide
`VolumeCapability` of a specific CSI version without rework.
2. The unversioned `VolumeCapability` parsed from a versioned one should
be able to used to reconstruct the original versioned
`VolumeCapability`, and can be upgraded/downgraded to a different
CSI version and preserve as much semantics as possible.
3. If an backward-incompatible change is introduced in future CSI
`VolumeCapability`, the unversioned `VolumeCapability` can provide a
way to do a backward compatible upgrade.
Diffs
-----
include/mesos/csi/compat.hpp PRE-CREATION
include/mesos/csi/compat.proto PRE-CREATION
src/CMakeLists.txt 3397c3b1d4e8a7900b2e5f870679cc7aa30b4be2
src/Makefile.am d451d7cabe3bf5d4f5039cfac5de1b03ef891d07
src/csi/compat.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/70247/diff/1/
Testing
-------
Thanks,
Chun-Hung Hsiao
Re: Review Request 70247: Added an "unversioned" `VolumeCapability`.
Posted by Benjamin Bannier <be...@mesosphere.io>.
> On March 20, 2019, 3:09 p.m., Benjamin Bannier wrote:
> > include/mesos/csi/compat.proto
> > Lines 1 (patched)
> > <https://reviews.apache.org/r/70247/diff/1/?file=2132974#file2132974line1>
> >
> > I don't think these definitions need to be public as they never leave the agent. Let's just add them to `src/csi/` or sim?
>
> Chun-Hung Hsiao wrote:
> The definition will be used in the disk profile adaptor. This can be discussed, but I personally feel that it's better if our module interface doesn't depend on a third-party protobuf so we can control our own backward compatibility.
Makes sense, dropping.
> On March 20, 2019, 3:09 p.m., Benjamin Bannier wrote:
> > src/csi/compat.cpp
> > Lines 25-61 (patched)
> > <https://reviews.apache.org/r/70247/diff/1/?file=2132977#file2132977line25>
> >
> > `return google::protobuf::util::MessageDifferencer::Equals(left, right)`?
>
> Chun-Hung Hsiao wrote:
> The reason we generally don't use `MessageDifferencer` is that we probably want to define our own equality semantics, for example, the "ordering may not matter" constraint here (although it's not implemented). Also, do we want to compare unknown fields?
>
> If we want a complete message equality check, then yes I'll just use `MessageDifferencer`.
I'd much prefer a world where each of such functions either forwards to a proto message differencer _or_ explicitly declares why it deviates from that implementation. Could you update the code for that?
- Benjamin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70247/#review213837
-----------------------------------------------------------
On March 26, 2019, 6:10 a.m., Chun-Hung Hsiao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70247/
> -----------------------------------------------------------
>
> (Updated March 26, 2019, 6:10 a.m.)
>
>
> Review request for mesos, Benjamin Bannier, James DeFelice, Jie Yu, and Jan Schlicht.
>
>
> Bugs: MESOS-9625
> https://issues.apache.org/jira/browse/MESOS-9625
>
>
> Repository: mesos
>
>
> Description
> -------
>
> To support both CSI v0 and v1, the "unversioned" `VolumeCapability` is
> designed to satisfy the following compatibility guarantees:
>
> 1. The unversioned `VolumeCapability` parsed from a versioned one should
> be able to used to reconstruct the original versioned
> `VolumeCapability`, and can be upgraded/downgraded to a different
> CSI version and preserve as much semantics as possible.
>
> 2. If an backward-incompatible change is introduced in future CSI
> `VolumeCapability`, the unversioned `VolumeCapability` can provide a
> way to do a backward compatible upgrade.
>
>
> Diffs
> -----
>
> include/mesos/csi/types.hpp PRE-CREATION
> include/mesos/csi/types.proto PRE-CREATION
> src/CMakeLists.txt 3397c3b1d4e8a7900b2e5f870679cc7aa30b4be2
> src/Makefile.am d451d7cabe3bf5d4f5039cfac5de1b03ef891d07
> src/csi/types.cpp PRE-CREATION
> src/csi/utils.hpp 9145c6795c3ecdde5de5859a852763fe9aeb1ddf
> src/csi/utils.cpp fd6f95d88caf69e2ae197cf940beb93c164565bc
>
>
> Diff: https://reviews.apache.org/r/70247/diff/3/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Chun-Hung Hsiao
>
>
Re: Review Request 70247: Added an "unversioned" `VolumeCapability`.
Posted by Chun-Hung Hsiao <ch...@apache.org>.
> On March 20, 2019, 2:09 p.m., Benjamin Bannier wrote:
> > include/mesos/csi/compat.proto
> > Lines 1 (patched)
> > <https://reviews.apache.org/r/70247/diff/1/?file=2132974#file2132974line1>
> >
> > I don't think these definitions need to be public as they never leave the agent. Let's just add them to `src/csi/` or sim?
The definition will be used in the disk profile adaptor. This can be discussed, but I personally feel that it's better if our module interface doesn't depend on a third-party protobuf so we can control our own backward compatibility.
> On March 20, 2019, 2:09 p.m., Benjamin Bannier wrote:
> > include/mesos/csi/compat.proto
> > Lines 19 (patched)
> > <https://reviews.apache.org/r/70247/diff/1/?file=2132974#file2132974line19>
> >
> > Looking at https://reviews.apache.org/r/70248/ I see that the `VolumeCapability` we'd actually want to use most of the time would always come from `compat`.
> >
> > Can we define it e.g., in `mesos.csi` or alternatively `mesos.csi.types` instead? That would seem more natural to me.
Yeah I can rename it to types. Currently the `mesos.csi` namespace import `.csi.v0` to make it convenient for coding. Moving forwards, we can do either
1. make `mesos.csi.v0` import `.csi.v0` and `mesos.csi.v1` import `.csi.v1`. Thats the reason I'm using `mesos.csi.compat`, but `mesos.csi.types` also works. (Actually `types` is one of the name I was considering ;) )
2. Don't do the import. As as result, all CSI v0 protobufs would be like `::csi::v0::XXXMessage` in the codebase, which is probably fine, just look a bit tedious.
What's your opinion on this?
> On March 20, 2019, 2:09 p.m., Benjamin Bannier wrote:
> > include/mesos/csi/compat.proto
> > Lines 27-30 (patched)
> > <https://reviews.apache.org/r/70247/diff/1/?file=2132974#file2132974line27>
> >
> > This promises a pretty hard thing. Why are we so sure this is always possible?
> >
> > This would be less concerning if this file would contain just types never leaving a single node (i.e., no concerns about mixing different proto versions).
> >
> > Let's promise that we'll try to faithfully reconstruct types instead?
Again, this is for using it in the public module interface.
I guess I should have probably loosen the constraint about directly parsing it from a versioned CSI protobuf JSON. We should just ensure that this protobuf can hold all information for different versioned CSI protobuf without loosing the information, and let the profile adaptor module to do the convertios from a versioned CSI protobuf to this one.
> On March 20, 2019, 2:09 p.m., Benjamin Bannier wrote:
> > src/csi/compat.cpp
> > Lines 25-61 (patched)
> > <https://reviews.apache.org/r/70247/diff/1/?file=2132977#file2132977line25>
> >
> > `return google::protobuf::util::MessageDifferencer::Equals(left, right)`?
The reason we generally don't use `MessageDifferencer` is that we probably want to define our own equality semantics, for example, the "ordering may not matter" constraint here (although it's not implemented). Also, do we want to compare unknown fields?
If we want a complete message equality check, then yes I'll just use `MessageDifferencer`.
- Chun-Hung
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70247/#review213837
-----------------------------------------------------------
On March 20, 2019, 6:15 a.m., Chun-Hung Hsiao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70247/
> -----------------------------------------------------------
>
> (Updated March 20, 2019, 6:15 a.m.)
>
>
> Review request for mesos, Benjamin Bannier, James DeFelice, Jie Yu, and Jan Schlicht.
>
>
> Bugs: MESOS-9625
> https://issues.apache.org/jira/browse/MESOS-9625
>
>
> Repository: mesos
>
>
> Description
> -------
>
> To support both CSI v0 and v1, the "unversioned" `VolumeCapability` is
> designed to satisfy the following compatibility guarantees:
>
> 1. It can be parsed from the JSON representation of a CSI v0 or v1
> `VolumeCapability`. This is for making the `UriDiskProfileAdaptor`
> backward compatible, and the profile service can simply provide
> `VolumeCapability` of a specific CSI version without rework.
>
> 2. The unversioned `VolumeCapability` parsed from a versioned one should
> be able to used to reconstruct the original versioned
> `VolumeCapability`, and can be upgraded/downgraded to a different
> CSI version and preserve as much semantics as possible.
>
> 3. If an backward-incompatible change is introduced in future CSI
> `VolumeCapability`, the unversioned `VolumeCapability` can provide a
> way to do a backward compatible upgrade.
>
>
> Diffs
> -----
>
> include/mesos/csi/compat.hpp PRE-CREATION
> include/mesos/csi/compat.proto PRE-CREATION
> src/CMakeLists.txt 3397c3b1d4e8a7900b2e5f870679cc7aa30b4be2
> src/Makefile.am d451d7cabe3bf5d4f5039cfac5de1b03ef891d07
> src/csi/compat.cpp PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/70247/diff/1/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Chun-Hung Hsiao
>
>
Re: Review Request 70247: Added an "unversioned" `VolumeCapability`.
Posted by Benjamin Bannier <be...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70247/#review213837
-----------------------------------------------------------
This looks reasonable to me, but I haven't seen yet that it can adapt to both csi-v0 and csi-v1.
include/mesos/csi/compat.hpp
Lines 1 (patched)
<https://reviews.apache.org/r/70247/#comment299912>
Similar namespace suggestion as for the protos.
include/mesos/csi/compat.proto
Lines 1 (patched)
<https://reviews.apache.org/r/70247/#comment299910>
I don't think these definitions need to be public as they never leave the agent. Let's just add them to `src/csi/` or sim?
include/mesos/csi/compat.proto
Lines 19 (patched)
<https://reviews.apache.org/r/70247/#comment299909>
Looking at https://reviews.apache.org/r/70248/ I see that the `VolumeCapability` we'd actually want to use most of the time would always come from `compat`.
Can we define it e.g., in `mesos.csi` or alternatively `mesos.csi.types` instead? That would seem more natural to me.
include/mesos/csi/compat.proto
Lines 27-30 (patched)
<https://reviews.apache.org/r/70247/#comment299914>
This promises a pretty hard thing. Why are we so sure this is always possible?
This would be less concerning if this file would contain just types never leaving a single node (i.e., no concerns about mixing different proto versions).
Let's promise that we'll try to faithfully reconstruct types instead?
include/mesos/csi/compat.proto
Lines 35 (patched)
<https://reviews.apache.org/r/70247/#comment299906>
I don't think we lint protobuf files with cpplint or clang-tidy.
include/mesos/csi/compat.proto
Lines 37 (patched)
<https://reviews.apache.org/r/70247/#comment299911>
Let's move helpers and their tests for these types from https://reviews.apache.org/r/70248/ here.
src/csi/compat.cpp
Lines 25-61 (patched)
<https://reviews.apache.org/r/70247/#comment299913>
`return google::protobuf::util::MessageDifferencer::Equals(left, right)`?
- Benjamin Bannier
On March 20, 2019, 7:15 a.m., Chun-Hung Hsiao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70247/
> -----------------------------------------------------------
>
> (Updated March 20, 2019, 7:15 a.m.)
>
>
> Review request for mesos, Benjamin Bannier, James DeFelice, Jie Yu, and Jan Schlicht.
>
>
> Bugs: MESOS-9625
> https://issues.apache.org/jira/browse/MESOS-9625
>
>
> Repository: mesos
>
>
> Description
> -------
>
> To support both CSI v0 and v1, the "unversioned" `VolumeCapability` is
> designed to satisfy the following compatibility guarantees:
>
> 1. It can be parsed from the JSON representation of a CSI v0 or v1
> `VolumeCapability`. This is for making the `UriDiskProfileAdaptor`
> backward compatible, and the profile service can simply provide
> `VolumeCapability` of a specific CSI version without rework.
>
> 2. The unversioned `VolumeCapability` parsed from a versioned one should
> be able to used to reconstruct the original versioned
> `VolumeCapability`, and can be upgraded/downgraded to a different
> CSI version and preserve as much semantics as possible.
>
> 3. If an backward-incompatible change is introduced in future CSI
> `VolumeCapability`, the unversioned `VolumeCapability` can provide a
> way to do a backward compatible upgrade.
>
>
> Diffs
> -----
>
> include/mesos/csi/compat.hpp PRE-CREATION
> include/mesos/csi/compat.proto PRE-CREATION
> src/CMakeLists.txt 3397c3b1d4e8a7900b2e5f870679cc7aa30b4be2
> src/Makefile.am d451d7cabe3bf5d4f5039cfac5de1b03ef891d07
> src/csi/compat.cpp PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/70247/diff/1/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Chun-Hung Hsiao
>
>
Re: Review Request 70247: Added an "unversioned" `VolumeCapability`.
Posted by Chun-Hung Hsiao <ch...@apache.org>.
> On April 2, 2019, 11:30 a.m., James DeFelice wrote:
> > include/mesos/csi/types.proto
> > Lines 19 (patched)
> > <https://reviews.apache.org/r/70247/diff/6/?file=2136539#file2136539line19>
> >
> > Might it make sense to version these since they're part of an API that Mesos uses to communicate w/ an external component? i.e. should there exist a include/mesos/v1/csi/types.proto that offers a stable/versioned API against which components can integrate?
>
> Chun-Hung Hsiao wrote:
> No. The purpose of introducing this is to make it version-agnostic so Mesos can handle plugin upgrades across versions. If we're relying on versioned CSI protos then we should directly use the ones from the spec instead.
Oops. Apologies for my misunderstanding.
Some comments coming out from our offline chat:
This proto currently not exposed to a "public-facing" API, but only through a module interface, so no need for creating the Mesos v1 counterpart yet. Similarly, we don't have Mesos v1 protos for authorizer and authenticator modules.
That said, the question is actually about if we should publish the "API" used in our reference implementation (i.e., `disk_profile.proto` in the URI disk profile adaptor) and make it more stable. It's definitely worth thinking about, but we probably want to think about how do deal with CSI versioning in that API first. In the future we could consider make the module a first-class component in Mesos and make it an official Mesos API w/ compatibility guarantees.
- Chun-Hung
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70247/#review214271
-----------------------------------------------------------
On March 28, 2019, 7:51 a.m., Chun-Hung Hsiao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70247/
> -----------------------------------------------------------
>
> (Updated March 28, 2019, 7:51 a.m.)
>
>
> Review request for mesos, Benjamin Bannier, James DeFelice, Jie Yu, and Jan Schlicht.
>
>
> Bugs: MESOS-9625
> https://issues.apache.org/jira/browse/MESOS-9625
>
>
> Repository: mesos
>
>
> Description
> -------
>
> To support both CSI v0 and v1, the "unversioned" `VolumeCapability` is
> designed to satisfy the following compatibility guarantees:
>
> 1. The unversioned `VolumeCapability` parsed from a versioned one should
> be able to used to reconstruct the original versioned
> `VolumeCapability`, and can be upgraded/downgraded to a different
> CSI version and preserve as much semantics as possible.
>
> 2. If an backward-incompatible change is introduced in future CSI
> `VolumeCapability`, the unversioned `VolumeCapability` can provide a
> way to do a backward compatible upgrade.
>
>
> Diffs
> -----
>
> include/mesos/csi/types.hpp PRE-CREATION
> include/mesos/csi/types.proto PRE-CREATION
> src/CMakeLists.txt 3397c3b1d4e8a7900b2e5f870679cc7aa30b4be2
> src/Makefile.am 7c2131a9daf16d49e8e8d75b12f019e5c4df1da3
> src/csi/types.cpp PRE-CREATION
> src/csi/utils.hpp 9145c6795c3ecdde5de5859a852763fe9aeb1ddf
> src/csi/utils.cpp fd6f95d88caf69e2ae197cf940beb93c164565bc
> src/tests/CMakeLists.txt f3acd8259edd8da2316d55271682a7a335fb22d4
> src/tests/csi_utils_tests.cpp PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/70247/diff/6/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Chun-Hung Hsiao
>
>
Re: Review Request 70247: Added an "unversioned" `VolumeCapability`.
Posted by Chun-Hung Hsiao <ch...@apache.org>.
> On April 2, 2019, 11:30 a.m., James DeFelice wrote:
> > include/mesos/csi/types.proto
> > Lines 19 (patched)
> > <https://reviews.apache.org/r/70247/diff/6/?file=2136539#file2136539line19>
> >
> > Might it make sense to version these since they're part of an API that Mesos uses to communicate w/ an external component? i.e. should there exist a include/mesos/v1/csi/types.proto that offers a stable/versioned API against which components can integrate?
No. The purpose of introducing this is to make it version-agnostic so Mesos can handle plugin upgrades across versions. If we're relying on versioned CSI protos then we should directly use the ones from the spec instead.
- Chun-Hung
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70247/#review214271
-----------------------------------------------------------
On March 28, 2019, 7:51 a.m., Chun-Hung Hsiao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70247/
> -----------------------------------------------------------
>
> (Updated March 28, 2019, 7:51 a.m.)
>
>
> Review request for mesos, Benjamin Bannier, James DeFelice, Jie Yu, and Jan Schlicht.
>
>
> Bugs: MESOS-9625
> https://issues.apache.org/jira/browse/MESOS-9625
>
>
> Repository: mesos
>
>
> Description
> -------
>
> To support both CSI v0 and v1, the "unversioned" `VolumeCapability` is
> designed to satisfy the following compatibility guarantees:
>
> 1. The unversioned `VolumeCapability` parsed from a versioned one should
> be able to used to reconstruct the original versioned
> `VolumeCapability`, and can be upgraded/downgraded to a different
> CSI version and preserve as much semantics as possible.
>
> 2. If an backward-incompatible change is introduced in future CSI
> `VolumeCapability`, the unversioned `VolumeCapability` can provide a
> way to do a backward compatible upgrade.
>
>
> Diffs
> -----
>
> include/mesos/csi/types.hpp PRE-CREATION
> include/mesos/csi/types.proto PRE-CREATION
> src/CMakeLists.txt 3397c3b1d4e8a7900b2e5f870679cc7aa30b4be2
> src/Makefile.am 7c2131a9daf16d49e8e8d75b12f019e5c4df1da3
> src/csi/types.cpp PRE-CREATION
> src/csi/utils.hpp 9145c6795c3ecdde5de5859a852763fe9aeb1ddf
> src/csi/utils.cpp fd6f95d88caf69e2ae197cf940beb93c164565bc
> src/tests/CMakeLists.txt f3acd8259edd8da2316d55271682a7a335fb22d4
> src/tests/csi_utils_tests.cpp PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/70247/diff/6/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Chun-Hung Hsiao
>
>
Re: Review Request 70247: Added an "unversioned" `VolumeCapability`.
Posted by James DeFelice <ja...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70247/#review214271
-----------------------------------------------------------
include/mesos/csi/types.proto
Lines 19 (patched)
<https://reviews.apache.org/r/70247/#comment300477>
Might it make sense to version these since they're part of an API that Mesos uses to communicate w/ an external component? i.e. should there exist a include/mesos/v1/csi/types.proto that offers a stable/versioned API against which components can integrate?
- James DeFelice
On March 28, 2019, 7:51 a.m., Chun-Hung Hsiao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70247/
> -----------------------------------------------------------
>
> (Updated March 28, 2019, 7:51 a.m.)
>
>
> Review request for mesos, Benjamin Bannier, James DeFelice, Jie Yu, and Jan Schlicht.
>
>
> Bugs: MESOS-9625
> https://issues.apache.org/jira/browse/MESOS-9625
>
>
> Repository: mesos
>
>
> Description
> -------
>
> To support both CSI v0 and v1, the "unversioned" `VolumeCapability` is
> designed to satisfy the following compatibility guarantees:
>
> 1. The unversioned `VolumeCapability` parsed from a versioned one should
> be able to used to reconstruct the original versioned
> `VolumeCapability`, and can be upgraded/downgraded to a different
> CSI version and preserve as much semantics as possible.
>
> 2. If an backward-incompatible change is introduced in future CSI
> `VolumeCapability`, the unversioned `VolumeCapability` can provide a
> way to do a backward compatible upgrade.
>
>
> Diffs
> -----
>
> include/mesos/csi/types.hpp PRE-CREATION
> include/mesos/csi/types.proto PRE-CREATION
> src/CMakeLists.txt 3397c3b1d4e8a7900b2e5f870679cc7aa30b4be2
> src/Makefile.am 7c2131a9daf16d49e8e8d75b12f019e5c4df1da3
> src/csi/types.cpp PRE-CREATION
> src/csi/utils.hpp 9145c6795c3ecdde5de5859a852763fe9aeb1ddf
> src/csi/utils.cpp fd6f95d88caf69e2ae197cf940beb93c164565bc
> src/tests/CMakeLists.txt f3acd8259edd8da2316d55271682a7a335fb22d4
> src/tests/csi_utils_tests.cpp PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/70247/diff/6/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Chun-Hung Hsiao
>
>
Re: Review Request 70247: Added an "unversioned" `VolumeCapability`.
Posted by Chun-Hung Hsiao <ch...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70247/
-----------------------------------------------------------
(Updated March 28, 2019, 7:51 a.m.)
Review request for mesos, Benjamin Bannier, James DeFelice, Jie Yu, and Jan Schlicht.
Changes
-------
Adapted the `MessageDifferencer`.
Bugs: MESOS-9625
https://issues.apache.org/jira/browse/MESOS-9625
Repository: mesos
Description
-------
To support both CSI v0 and v1, the "unversioned" `VolumeCapability` is
designed to satisfy the following compatibility guarantees:
1. The unversioned `VolumeCapability` parsed from a versioned one should
be able to used to reconstruct the original versioned
`VolumeCapability`, and can be upgraded/downgraded to a different
CSI version and preserve as much semantics as possible.
2. If an backward-incompatible change is introduced in future CSI
`VolumeCapability`, the unversioned `VolumeCapability` can provide a
way to do a backward compatible upgrade.
Diffs (updated)
-----
include/mesos/csi/types.hpp PRE-CREATION
include/mesos/csi/types.proto PRE-CREATION
src/CMakeLists.txt 3397c3b1d4e8a7900b2e5f870679cc7aa30b4be2
src/Makefile.am bcafe48b2105575371464a29783bc6f3f1c2cf8d
src/csi/types.cpp PRE-CREATION
src/csi/utils.hpp 9145c6795c3ecdde5de5859a852763fe9aeb1ddf
src/csi/utils.cpp fd6f95d88caf69e2ae197cf940beb93c164565bc
src/tests/CMakeLists.txt ab7f6c2027d937038ee70145705b699c4fb8f05c
src/tests/csi_utils_tests.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/70247/diff/5/
Changes: https://reviews.apache.org/r/70247/diff/4-5/
Testing
-------
make check
Thanks,
Chun-Hung Hsiao
Re: Review Request 70247: Added an "unversioned" `VolumeCapability`.
Posted by Chun-Hung Hsiao <ch...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70247/
-----------------------------------------------------------
(Updated March 27, 2019, 5:52 a.m.)
Review request for mesos, Benjamin Bannier, James DeFelice, Jie Yu, and Jan Schlicht.
Changes
-------
Reordered the patch and squashed r/70258 into this patch.
Bugs: MESOS-9625
https://issues.apache.org/jira/browse/MESOS-9625
Repository: mesos
Description
-------
To support both CSI v0 and v1, the "unversioned" `VolumeCapability` is
designed to satisfy the following compatibility guarantees:
1. The unversioned `VolumeCapability` parsed from a versioned one should
be able to used to reconstruct the original versioned
`VolumeCapability`, and can be upgraded/downgraded to a different
CSI version and preserve as much semantics as possible.
2. If an backward-incompatible change is introduced in future CSI
`VolumeCapability`, the unversioned `VolumeCapability` can provide a
way to do a backward compatible upgrade.
Diffs (updated)
-----
include/mesos/csi/types.hpp PRE-CREATION
include/mesos/csi/types.proto PRE-CREATION
src/CMakeLists.txt 3397c3b1d4e8a7900b2e5f870679cc7aa30b4be2
src/Makefile.am bcafe48b2105575371464a29783bc6f3f1c2cf8d
src/csi/types.cpp PRE-CREATION
src/csi/utils.hpp 9145c6795c3ecdde5de5859a852763fe9aeb1ddf
src/csi/utils.cpp fd6f95d88caf69e2ae197cf940beb93c164565bc
src/tests/CMakeLists.txt ab7f6c2027d937038ee70145705b699c4fb8f05c
src/tests/csi_utils_tests.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/70247/diff/4/
Changes: https://reviews.apache.org/r/70247/diff/3-4/
Testing
-------
make check
Thanks,
Chun-Hung Hsiao
Re: Review Request 70247: Added an "unversioned" `VolumeCapability`.
Posted by Benjamin Bannier <be...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70247/#review214057
-----------------------------------------------------------
Ship it!
LGTM, modulo outstanding issue. Still unsure how this would hold up for v1, let's commit this in one go.
- Benjamin Bannier
On March 26, 2019, 6:10 a.m., Chun-Hung Hsiao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70247/
> -----------------------------------------------------------
>
> (Updated March 26, 2019, 6:10 a.m.)
>
>
> Review request for mesos, Benjamin Bannier, James DeFelice, Jie Yu, and Jan Schlicht.
>
>
> Bugs: MESOS-9625
> https://issues.apache.org/jira/browse/MESOS-9625
>
>
> Repository: mesos
>
>
> Description
> -------
>
> To support both CSI v0 and v1, the "unversioned" `VolumeCapability` is
> designed to satisfy the following compatibility guarantees:
>
> 1. The unversioned `VolumeCapability` parsed from a versioned one should
> be able to used to reconstruct the original versioned
> `VolumeCapability`, and can be upgraded/downgraded to a different
> CSI version and preserve as much semantics as possible.
>
> 2. If an backward-incompatible change is introduced in future CSI
> `VolumeCapability`, the unversioned `VolumeCapability` can provide a
> way to do a backward compatible upgrade.
>
>
> Diffs
> -----
>
> include/mesos/csi/types.hpp PRE-CREATION
> include/mesos/csi/types.proto PRE-CREATION
> src/CMakeLists.txt 3397c3b1d4e8a7900b2e5f870679cc7aa30b4be2
> src/Makefile.am d451d7cabe3bf5d4f5039cfac5de1b03ef891d07
> src/csi/types.cpp PRE-CREATION
> src/csi/utils.hpp 9145c6795c3ecdde5de5859a852763fe9aeb1ddf
> src/csi/utils.cpp fd6f95d88caf69e2ae197cf940beb93c164565bc
>
>
> Diff: https://reviews.apache.org/r/70247/diff/3/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Chun-Hung Hsiao
>
>
Re: Review Request 70247: Added an "unversioned" `VolumeCapability`.
Posted by Chun-Hung Hsiao <ch...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70247/
-----------------------------------------------------------
(Updated March 26, 2019, 5:10 a.m.)
Review request for mesos, Benjamin Bannier, James DeFelice, Jie Yu, and Jan Schlicht.
Changes
-------
Fixed a typo.
Bugs: MESOS-9625
https://issues.apache.org/jira/browse/MESOS-9625
Repository: mesos
Description
-------
To support both CSI v0 and v1, the "unversioned" `VolumeCapability` is
designed to satisfy the following compatibility guarantees:
1. The unversioned `VolumeCapability` parsed from a versioned one should
be able to used to reconstruct the original versioned
`VolumeCapability`, and can be upgraded/downgraded to a different
CSI version and preserve as much semantics as possible.
2. If an backward-incompatible change is introduced in future CSI
`VolumeCapability`, the unversioned `VolumeCapability` can provide a
way to do a backward compatible upgrade.
Diffs (updated)
-----
include/mesos/csi/types.hpp PRE-CREATION
include/mesos/csi/types.proto PRE-CREATION
src/CMakeLists.txt 3397c3b1d4e8a7900b2e5f870679cc7aa30b4be2
src/Makefile.am d451d7cabe3bf5d4f5039cfac5de1b03ef891d07
src/csi/types.cpp PRE-CREATION
src/csi/utils.hpp 9145c6795c3ecdde5de5859a852763fe9aeb1ddf
src/csi/utils.cpp fd6f95d88caf69e2ae197cf940beb93c164565bc
Diff: https://reviews.apache.org/r/70247/diff/3/
Changes: https://reviews.apache.org/r/70247/diff/2-3/
Testing
-------
make check
Thanks,
Chun-Hung Hsiao
Re: Review Request 70247: Added an "unversioned" `VolumeCapability`.
Posted by Chun-Hung Hsiao <ch...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70247/
-----------------------------------------------------------
(Updated March 20, 2019, 9:36 p.m.)
Review request for mesos, Benjamin Bannier, James DeFelice, Jie Yu, and Jan Schlicht.
Changes
-------
Addressed Benjamin's comments and moved `evolve` from r/70248.
Bugs: MESOS-9625
https://issues.apache.org/jira/browse/MESOS-9625
Repository: mesos
Description (updated)
-------
To support both CSI v0 and v1, the "unversioned" `VolumeCapability` is
designed to satisfy the following compatibility guarantees:
1. The unversioned `VolumeCapability` parsed from a versioned one should
be able to used to reconstruct the original versioned
`VolumeCapability`, and can be upgraded/downgraded to a different
CSI version and preserve as much semantics as possible.
2. If an backward-incompatible change is introduced in future CSI
`VolumeCapability`, the unversioned `VolumeCapability` can provide a
way to do a backward compatible upgrade.
Diffs (updated)
-----
include/mesos/csi/types.hpp PRE-CREATION
include/mesos/csi/types.proto PRE-CREATION
src/CMakeLists.txt 3397c3b1d4e8a7900b2e5f870679cc7aa30b4be2
src/Makefile.am d451d7cabe3bf5d4f5039cfac5de1b03ef891d07
src/csi/types.cpp PRE-CREATION
src/csi/utils.hpp 9145c6795c3ecdde5de5859a852763fe9aeb1ddf
src/csi/utils.cpp fd6f95d88caf69e2ae197cf940beb93c164565bc
Diff: https://reviews.apache.org/r/70247/diff/2/
Changes: https://reviews.apache.org/r/70247/diff/1-2/
Testing
-------
make check
Thanks,
Chun-Hung Hsiao