Redesign proposition for the image mappings module, based on #207 #237
Replies: 4 comments 1 reply
-
|
Hi @ivan-zdravkov . First of all thanks for the amazing amount of work and detail for this proposal and the analysis you did! This is really some great stuff!!! I have a few questions, which we can take on the next meeting and proceed with finalising this design:
|
Beta Was this translation helpful? Give feedback.
-
|
1+2. We decided that if |
Beta Was this translation helpful? Give feedback.
-
|
After a discussion the following was decided:
Take a look at this example: images:
ignoredRegions:
- "us-west-1"
regions:
- "eu-east-1"
- "eu-east-2"
- "eu-east-3"
default: "eu-east-2" |
Beta Was this translation helpful? Give feedback.
-
|
Writing as a comment to respect the history of the proposition and comments. As @Michaelpalacce mentioned, we had a discussion and decided to keep using the JSON structuresingle-redesign.jsondual-redesign.jsontriple-redesign.jsonIn all of the cases, the The ExamplesPushing the
|
Beta Was this translation helpful? Give feedback.
-
|
This design proposal is truly great unfortunately we didn't get to cover that in 2.x.x milestone. In 3.0.0 (work on which is has already started) we plan to completely remove infrastructure related content support like the current mappings thus closing this topic. Again, @ivan-zdravkov thank you for the huge effort you put in this! |
Beta Was this translation helpful? Give feedback.
-
Table of Contents
Bug Report #207
Background
Context
Build tools for VMware Aria has a functionality to pull and push image mappings and regions.
Regionsare the unique vCenterdata centers, imported for the linked Ariacloud accountsfollowing the{cloudAccountName} / {dataCenterName}naming convention.Imagesare the available VM templates.Image Mappingsare named M:M relations between existingimagesandregionsImage mappings can be defined in Aria Automation.
And every image mapping can map to a specific image in each of its respective regions.
To do a pull, we have to specify the
image-mappingnames in Orchestrator'scontent.yamlas well as theregion-mappingtags.Pulling
Pulling generates a correct representation of the image mappings, grouped by the respective regions.
Each regions's
src-region-profile.jsonuniquely maps the region by the target'scloudAccountIdandregionIditself.{ "cloudAccountId": "ab6eca9a-7ca7-4a24-b51b-85e1a4dafac8", "regionId": "8522d781-89ee-422b-92a1-bce602033258", "regionType": "vsphere", "tags": [ "env:dev" ] }Wherever an image mapping has multiple regions, duplicate records for the image mapping are created in the respective region's directory, containing the specific region's image
externalId.{ "isPrivate": false, "externalId": "wwcoe_contentlib / centOS_non_existing", "name": "wwcoe_contentlib / centOS_non_existing" }Pushing
Originally, and in line with the project's other resource elements, pushing is expected to populate Aria with the resources as described in code.
Issue
When importing the image mappings, it seems like the last one to be processed, would overwrite the data for the previous regions. Meaning that if you have image mappings that point to:
Test-1,Test-2,Test-3images, then after import the one that was processed last will be the one that is in use everywhere.In the tested environment we had 3 data centers and one of the data center had 2 regions, if that matters
Steps to reproduce
Expected behavior
The region mappings to be mapped correctly per region.
Actual behavior
Last one overwrites all the rest.
Reproduces how often
Component:
Environment
Client
Server
Additional Context
This is when pulling and pushing to the SAME environment.
Reproduction
Steps
Single Region, a mapping with a single region.Dual Region, a mapping with two regions.Triple Region, a mapping with three regions.Results
Executing the reproduction steps produces an incorrect environment state, just as described in the Issue section.
{ "constraints": [], "isPrivate": false, "externalId": "wwcoe_contentlib / centOS_non_existing", "name": "Triple Region" }Execution changes all of the environment image mappings to have 3 regions and all of the regions to have the same image, regardless of what was previously there.
There isn't any apparent indication as to which of the code
externalIdof the original images gets populated after the push. It does not appear to be the file system's latest one, as indicated in the issue.Root Cause Analysis
As originally performed by Eman Elsayed and later confirmed by Ivan Zdravkov and Sugeesh Chandraweera, the cause of the bug is a missing check in the importContent method of the VraNgImageMappingStore.java file.
As a result the regions are not filtered properly, and every image mapping gets inserted every region resulting in the latest execution to override every change previously.
Proposed Solution
A simple nearly identical solution was proposed by each of the investigating members which adds a check to ensure each region correctly processes only their own records.
// list all directories in the regions folder Arrays.asList(regionsFolder.listFiles(File::isDirectory)).forEach(regionProfileDir -> { try { VraNgCloudRegionProfile cloudRegionProfile = VraNgRegionalContentUtils.getCloudRegionProfile(regionProfileDir); cloudAccounts .stream() .filter(cloudAccount -> VraNgRegionalContentUtils.isIntersecting(cloudAccount.getTags(), importTags)) .filter(cloudAccount -> cloudAccount.getType().equals(cloudRegionProfile.getRegionType())) .forEach(cloudAccount -> cloudAccount.getRegionIds() - .forEach(regionId -> this.importImageProfilesInRegion + .forEach(regionId -> { + boolean shouldImport = regionProfileDir.getName().contains(regionId); + + if (shouldImport) { + this.importImageProfilesInRegion( regionId, regionProfileDir, - imageProfilesByRegion))); + imageProfilesByRegion); + } + } + )); } catch (IOException e) { e.printStackTrace(); } });Solution Results
As a result of the implemented solution, pushing the regions now correctly populates them in Aria, resulting in the original state of the environment.
Solution Denial
Why
In a high level discussion with Stefan Genov, the proposed solution was denied.
It turns out that the alleged original intention of the module is very environmentally dependent. All of the entities in question (Cloud Account, Region, Image, etc.) are kept in code by their ids and not by their name, tag or another environmentally agnostic handler.
This means that pulling from a dev environment and trying to push into a client environment will never work, since no matching of ids will ever occur.
But it's been working fine... ish
What
accidentallyhad been happening all along is that:developwith asingledev region.clientusually hasmultipleregions.allof the regions on the client environment.Partial Conclusion
Fixing the issue as proposed will
restorethe original design, but will actuallyworsenthe dev and client experience.We discussed a number of ways of how to address this, so that it would both work as intended and continue doing meaningful work, and decided against implementing a
patchon top of abugon top of theoriginaldesign,while simultaneously trying to both fix it and keep the accidental behavior of
Since the issue of how to map
devandcliententity ids is both hard and requiring a lot of manual work post-implementation, we are currently looking into a solution redesign, which will try to solve the problems ofRedesign Proposition
The current solution relies on entity ids to define image mappings and regions, which makes it difficult to achieve environmental independence.
Having a full dev-client environment mapping as discussed earlier is both difficult and probably needless.
The following redesign proposition aims to
fully reworkthe image mapping capabilities of the project in order to:JSON structure
Setup
Dev Environment Regions
vCenter / dc-01aregion.vCenter / dc-02aregion.vCenter / dc-03aregion.Client Environment Regions
vCenter / dc-01aregion.vCenter / dc-02aregion.vCenter / dc-03aregion.vCenter / dc-04aregion.vCenter / dc-05aregion.Dev Environment Image Mappings
where the
Cloud Configurationvalue ofvCenter / dc-02aispassword:((sensitive:123456))where the
Cloud Configurationvalue ofvCenter / dc-02aispassword:((sensitive:123456))vCenter / dc-03aispassword:((secret:v1:gdq7gdgeggfqu882yhf==))Pulling
Pulling from an environment should generate a single
{imageMappingName}.jsonfile per a image mapping in theimage-mappingsdirectory.By default pulling from an environment pulls
allimage mappings specified in thecontent.yamlfile, with their first region set asdefault, and all additional regions (if any), pulled into theregionsarray.ignoredRegionsis never populated on a pull and is used to indicate which client regions won't be populated during a push.single-redesign.json
{ "name": "Single Redesign", "default": { "image": "wwcoe_contentlib / empty_VM", "constraints": [], "script": "" }, "regions": [], "ignoredRegions": [] }The
single-redesign.jsonwould be the most common scenario we are developing for. This is the scenario, where we have an image mapping with a single region, which is set asdefaultwhen pulling, expected to later be pushed to every client environment region.dual-redesign.json
{ "name": "Dual Redesign", "default": { "image": "wwcoe_contentlib / empty_VM", "constraints": [], "script": "" }, "regions": [ { "name": "vCenter / dc-02a", "image": "wwcoe_contentlib / Ubuntu_18.04", "constraints": ["env:dev:hard", "nsx:hard"], "script": "password:((sensitive:123456))" } ], "ignoredRegions": [] }triple-redesign.json
{ "name": "Triple Redesign", "default": { "image": "wwcoe_contentlib / empty_VM", "constraints": [], "script": "" }, "regions": [ { "name": "vCenter / dc-02a", "image": "wwcoe_contentlib / Ubuntu_18.04", "constraints": ["env:dev:hard", "nsx:hard"], "script": "password:((sensitive:123456))" }, { "name": "vCenter / dc-03a", "image": "wwcoe_contentlib / Win2022", "constraints": ["access:ext:hard"], "script": "password:((secret:v1:gdq7gdgeggfqu882yhf==))" } ], "ignoredRegions": [] }Pushing
As shown above, the
defaultproperty is region agnostic and contains the properties that will be populated forallclient regions, unless specified otherwise in theregionsorignoredRegionsarrays.Algorithm
Same Environment
Pushing on the same or identical environment is expected to recreate exactly the described json.
Different Environment
Pushing on a different environment is expected to:
ignoredRegionsstring array if any.regionsarray if any.defaultJSON properties to every other environment region.Examples
Default Scenario
Pushing as-is is the most common scenario where we are pulling from a single dev region and pushing to multiple client regions.
{ "name": "Single Redesign", "default": { "image": "wwcoe_contentlib / empty_VM", "constraints": [], "script": "" }, "regions": [], "ignoredRegions": [] }In this case, since the client has 3 entirely different regions
vCenter / dc-03avCenter / dc-04avCenter / dc-05athan the original dev environment's region
vCenter / dc-01aand since we don't have additional
regionsdefinitions orignoredRegionsthe expectation is that thedefaultimage mapping will be applied to all client regions.Ignoring a Region
Ignoring the
vCenter / dc-05aclient region in theignoredRedionsarray will as expected not populate it on a push.{ "name": "Single Redesign", "default": { "image": "wwcoe_contentlib / empty_VM", "constraints": [], "script": "" }, "regions": [], "ignoredRegions": ["vCenter / dc-05a"] }Customizing a Region
Having definitions in the
regionsarray will override thedefaultvalues for the listed regions, while all other regions will get thedefaultdata.{ "name": "Dual Redesign", "default": { "image": "wwcoe_contentlib / empty_VM", "constraints": [], "script": "" }, "regions": [ { "name": "vCenter / dc-02a", "image": "wwcoe_contentlib / Ubuntu_18.04", "constraints": ["env:dev:hard", "nsx:hard"], "script": "password:((sensitive:123456))" } ], "ignoredRegions": [] }Backwards Compatibility
Functionality
Functionally wise, the proposed solution will behave similarly if not identically to what currently exists for the most common scenario of pulling from a single region and pushing to all client regions.
For multi-region scenarios, the proposed solution will work as described above, solving the original bug that triggered the whole redesign.
Most importantly pulling and pushing as-is will work mostly the same for both the same and different environments.
Structure
Structure wise, the proposed solution uses an entirely different file and JSON structure. As such the newly proposed solution is not structurally backwards compatible.
Release Planning
A release planning could help solve the backwards compatibility issues. We could implement both the original and proposed solutions and still have the original solution as default, until the proper time and version where we can switch the default version to the newly proposed one, while still keeping the availability of users to opt-in and use the legacy version.
Further down the line in the next major release, we might decommission the legacy version entirely.
Risks and Mitigation Strategies
The major risks identified with the proposed solution, besides the usual bugs and regressions, all revolve around actually implementing a solution to solve the correct problem.
One of the main purposes of this document is to clearly outline the original design and problem as well as the new solution proposition to make it easier for stakeholders to identify discrepancies or logic lapses.
Some of the mitigation strategies we can use to reduce the technical and business risks include:
Impact Analysis
Functional
Initially the impact of the new solution in terms of functionality does not seem huge, since the described bug is a bit of niche scenario. Additionally most user's day to day usage won't change much, besides some slimmer syntax.
Besides the obvious, implementing the solution will both simplify the JSON structure and bring it closer to the actual aria design.
Introducing environmental agnosticism, which coupled with the proposed ignore and default override functionality brings more flexibility for edge case development and a more wide range of capabilities for our deliverables.
Non-Functional
Major refactoring proposition processto simplify future support and improve the knowledge base.Next Steps
Beta Was this translation helpful? Give feedback.
All reactions