Conversation
We have added the UUID to the spaces in the serialized model for migrations, because we want to keep the UUIDs of entities across migrations.
wallyworld
left a comment
wallyworld
left a comment
There was a problem hiding this comment.
We need to think about removing ID, at least on export.
| @@ -15,6 +15,7 @@ type spaces struct { | |||
|
|
|||
| type space struct { | |||
| Id_ string `yaml:"id"` | |||
There was a problem hiding this comment.
With the addition of uuid, can't we do away with Id now?
space.go
Outdated
| } | ||
|
|
||
| func importSpaceV3(source map[string]interface{}) (*space, error) { | ||
| fields, defaults := spaceV2Fields() |
There was a problem hiding this comment.
Don't do this here - add a spaceV3Fields() method.
I see that V2 used this pattern - whomever did it originally did it wrong :-)
| return fields, defaults | ||
| } | ||
|
|
||
| func spaceV2Fields() (schema.Fields, schema.Defaults) { |
There was a problem hiding this comment.
So this method is added.
But we still don't use it, eg we still have this code in import v1
fields, defaults := spaceV1Fields()
fields["id"] = schema.String()
There was a problem hiding this comment.
sure, added V3 and used this func
space.go
Outdated
| }, nil | ||
| } | ||
|
|
||
| func importSpaceV3(source map[string]interface{}) (*space, error) { |
There was a problem hiding this comment.
We should use the pattern where the import version is passed in to a common import function (as is done elsewhere).
| // new internal subnet type that supports the Subnet interface. | ||
| type SubnetArgs struct { | ||
| // Deprecated in favor of UUID | ||
| ID string |
There was a problem hiding this comment.
Since this is v6, we can get rid of ID since it's being replaced bu UUID.
There was a problem hiding this comment.
So we discussed with @jameinel and @SimonRichardson and the idea is to keep these deprecated fields to ensure not breaking in the future (although this version shouldn't be used in juju <4). This is coherent to what was done previously in subnets with SpaceName.
There was a problem hiding this comment.
Yeah, we will need to reconcile V3 to V4 by the IDs anyway.
model.go
Outdated
| // TODO(nvinuesa): How will we identify the alpha space | ||
| // in UUID? | ||
| if subnet.SpaceUUID() == "" { | ||
| continue | ||
| } |
There was a problem hiding this comment.
IMO we shouldn't be doing these checks at this level, this should be a serialization/deserialization library and with these validate functions we are adding business logic. This particular check is not OK anymore and we should decide if we want to add the (new) alpha space UUID here or remove this validate altogether. @wallyworld @SimonRichardson
There was a problem hiding this comment.
after discussing with @manadart, we are keeping 0 as the uuid for the alpha space.
…h the rest of the entities
nvinuesa
force-pushed
the
add-uuid-spaces
branch
from
943abe1 to
5488033
Compare
| // new internal subnet type that supports the Subnet interface. | ||
| type SubnetArgs struct { | ||
| // Deprecated in favor of UUID | ||
| ID string |
There was a problem hiding this comment.
Yeah, we will need to reconcile V3 to V4 by the IDs anyway.
…ta-manifest #148 Cherry-pick of #147 to v7, to allow landing of 3.6 without bringing in #139 We can't land this into v6 as that is for main. So effectively, v6 branch is dead and we should never have cut a release for main against v6. The main branch is unfinished and we should have been just iterating on a hash.
3 participants
This patch adds the
UUIDfield to spaces and subnets. Also, it adds thespaceUUIDfield to subnets.The idea behind this, is that on Juju 4+ we will keep the entity's UUID when migrating them. One of the reasons is to keep the relation between entities sound after migration.
For migrations 3.5 -> 4.0, the UUID will naturally be empty when importing the model on the 4.0 controller and thus creating the entity with a new UUID.