You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Copy file name to clipboardExpand all lines: docs/hugo/content/design/ADR-2023-09-Skipping-Properties/_index.md
+49-16Lines changed: 49 additions & 16 deletions
Display the source diff
Display the rich diff
Original file line number
Diff line number
Diff line change
@@ -24,8 +24,6 @@ Consider a CRM system containing details of people. In v3 of the system, we capt
24
24
25
25
<imgsrc="./conversion-v3-v4-class.png"alt="Conversion from v3 to v4" />
26
26
27
-
28
-
29
27
When we convert from v3 to v4 (v3 -> v4) the `ResidentialAddress` property gets serialized into the `PropertyBag` on the v4 Person. Note that the bag contains a v3 `Address` in a serialized form.
30
28
31
29
To illustrate, consider this concrete example:
@@ -83,7 +81,7 @@ In v5, the `ResidentialAddress` is reintroduced, but with a different shape. Ins
83
81
84
82
<img src="./conversion-v4-v5-class.png" alt="Conversion from v4 to v5" />
85
83
86
-
When we convert from v5 to v4, again the `ResidentialAddress` property gets serialized into the `PropertyBag` on the v4 Person.
84
+
When we convert from v5 to v4, again the `ResidentialAddress` property gets serialized into the `PropertyBag` on the v4 Person.
87
85
88
86
Again, it's useful to see a concrete example:
89
87
@@ -116,7 +114,7 @@ Conversion back and forward between versions v4 and v5 works fine.
116
114
117
115
### The Problem
118
116
119
-
Observe how we can end up with two different variants of v4 `Person`.
117
+
Observe how we can end up with two different variants of v4 `Person`.
120
118
121
119
In one case, we have a v4 `Person` where the property bag contains a v3 `Address`.
122
120
In the other case, we have a v4 `Person` the property bag contains a v5 `Address`:
@@ -147,8 +145,7 @@ While round trips between _adjacent_ versions work fine, we run into problems wh
147
145
148
146
<img src="./conversion-v3-v4-v5-class.png" alt="Conversion from v3 to v4 to v5" />
149
147
150
-
151
-
Compare the two different kinds of v4 Person we end up with for Mickey.
148
+
Compare the two different kinds of v4 Person we end up with for Mickey.
152
149
153
150
First when we start from v3:
154
151
@@ -170,15 +167,15 @@ propertyBag:
170
167
residentialAddress: "street:1313 S. Harbor Blvd;suburb:;city:Anaheim, CA 92803;country:USA"
171
168
```
172
169
173
-
If we have a v4 `Person` that contains a v3 `Address` in the property bag, conversion to the v5 `Person` will fail when we try to deserialize a v5 `Address` from the v3 item.
170
+
If we have a v4 `Person` that contains a v3 `Address` in the property bag, conversion to the v5 `Person` will fail when we try to deserialize a v5 `Address` from the v3 item.
174
171
175
-
This will cause the operator to fail at runtime.
172
+
This will cause the operator to fail at runtime.
176
173
177
174
Similarly f we end up with a v4 `Person` that contains a v5 `Address` in the property bag, we can't deserialize a v3 `Address` and again the operator will fail at runtime.
178
175
179
176
### Constraint: Serialization format
180
177
181
-
For each resource type, there's a specific hub version that's used for serialization - this is the version that's persisted within a cluster. All other versions are created on-the-fly by conversion from the hub version. By convention in ASO, this hub version is based on the the latest stable version of the resource, or the latest preview version if no stable version is available.
178
+
For each resource type, there's a specific hub version that's used for serialization - this is the version that's persisted within a cluster. All other versions are created on-the-fly by conversion from the hub version. By convention in ASO, this hub version is based on the the latest stable version of the resource, or the latest preview version if no stable version is available.
182
179
183
180
In most cases, the hub version will be the version affected by changes to solve the skipping-property problem, so we have to assume that there are extant resources of that version.
184
181
@@ -217,11 +214,13 @@ In our example here, we would add a new `ResidentialAddress` property to the v4
217
214
The shape of `Address` when reintroduced will always match the shape of `Address` when removed, so we can always serialize the same shape in-to or out-of the property bag, ensuring any `Address` has the a consistent shape.
218
215
219
216
Pros:
217
+
220
218
* Works the way existing conversions do.
221
219
* We have an existing contract for augmenting conversions with custom code, allowing us to handcode the conversion between v4 and v5 to handle the mismatch in the shapes of `Address`.
222
220
* Addresses the central inconsistency problem, ensuring we always serialize the same shape into intermediate property bags
223
221
224
222
Cons:
223
+
225
224
* Changes the serialization format for existing v4 resources. This is a breaking change for existing resources.
226
225
* Changes the object model of released resources, a breaking change for code consumers of ASO `api` packages.
227
226
* When there are multi-generational skips, other questions arise: Do we introduce the new type into every intermediate version, only the last, or both first and last? Each approach has advantages and drawbacks.
@@ -280,7 +279,6 @@ We're already generating support to allow augmentation of the generated conversi
280
279
281
280
It's highly desirable that we leverage this existing functionality for skipping properties, rather than building something new.
282
281
283
-
284
282
### Proposed Solution
285
283
286
284
Given that any existing resources will have property bags containing instances in the earlier shape, standardize on using that shape - and only that shape - when storing the instance in the property bag.
@@ -307,7 +305,7 @@ if source.ResidentialAddress != nil {
307
305
} else {
308
306
propertyBag.Remove("ResidentialAddress")
309
307
}
310
-
```
308
+
```
311
309
312
310
Reading the value from the property bag will also need to change. Instead of:
313
311
@@ -325,6 +323,7 @@ if propertyBag.Contains("ResidentialAddress") {
325
323
destination.ResidentialAddress = nil
326
324
}
327
325
```
326
+
328
327
We will read the earlier shape and convert it as required.
329
328
330
329
```go
@@ -371,8 +370,8 @@ To avoid problems with circular dependencies (as noted above), we will clone the
371
370
372
371
Our existing conversion framework supports writing hand-crafted conversions between shapes, migrating data between the two shapes. By introducing a new type and leveraging the existing framework for the conversion, we gain support for writing these hand-crafted conversions between shapes.
373
372
374
-
375
373
Pros:
374
+
376
375
* Addresses the central inconsistency problem, ensuring we always serialize the same shape into intermediate property bags
377
376
* Conversions are still driven by the earlier version, avoiding any issues with circular dependencies or needing to move conversion code elsewhere (e.g. to the later version).
378
377
* Isolates the compatibility type in a dedicated subpackage, where it won't inadvertently be picked up by documentation tools or other code consumers. (If this is not important, we can instead target the main v4 package.)
@@ -381,9 +380,9 @@ Pros:
381
380
* Serialization format of v4 remains unchanged, ensuring compatibility with existing releases of ASO.
382
381
383
382
Cons:
384
-
* Requires modification of the conversion-graph, which to this point has been immutable once constructed (we can't delay construction until after we inject the compatibility package as we need it to identify skipping properties).
385
-
* When code generating for property conversions using a property bag, we'll need to detect and use the `compat` subpackage.
386
383
384
+
* Requires modification of the conversion-graph, which to this point has been immutable once constructed (we can't delay construction until after we inject the compatibility package as we need it to identify skipping properties).
385
+
* When code generating for property conversions using a property bag, we'll need to detect and use the `compat` subpackage.
387
386
388
387
## Decision
389
388
@@ -399,9 +398,43 @@ TBC
399
398
400
399
## Experience Report
401
400
402
-
TBC
401
+
As implemented by [PR#3614](https://github.com/Azure/azure-service-operator/pull/3614), we can successfully handle most situations without requiring any manual intervention.
402
+
403
+
There's an edge case, however, when the _type_ of the property changes between removal and reintroduction, as discovered with `containerservice.ManagedCluster`.
404
+
405
+
The property `ManagedClusterAgentPoolProfile.GPUProfile` had the type `AgentPoolGPUProfile` in version `2024-04-02-preview`, but when it was reintroduced in `2025-08-01`, the type had been renamed to `GPUProfile`.
406
+
407
+
This resulted in testing failures when the shape persisted in the property bag was inconsistent between directions, as predicted above.
408
+
409
+
```
410
+
Round trip from ManagedClustersAgentPool to hub returns original: Falsified after 1 passed tests.
411
+
Labels of failing property:
412
+
converting from destination to hub:
413
+
converting from destination to hub:
414
+
converting from destination to hub:
415
+
calling AssignProperties_To_ManagedClustersAgentPool_Spec() to populate field Spec:
416
+
pulling'GpuProfile' from propertyBag:
417
+
pulling "GpuProfile" from PropertyBag: json:
418
+
unknown field "installGPUDriver"
419
+
```
420
+
421
+
Automatic repair of the skipping property failed in this case because the code was unaware that the type had been renamed from `AgentPoolGPUProfile` to `GPUProfile`.
422
+
423
+
A code fix has been implemented, but requires additional configuration to activate if this edge case recurs, as follows:
424
+
425
+
On the version where the property is last seen, add `$nameInNextVersion: NewName` to the objectModelConfiguration section for each property type that's been renamed. Remember to do both the spec and status variants if applicable.
0 commit comments