Redesign proposition for the image mappings module, based on #207

[ivan-zdravkov]

Table of Contents

• Bug Report #207
  • Background
    • Context
    • Pulling
    • Pushing
  • Issue
    • Steps to reproduce
    • Expected behavior
    • Actual behavior
    • Reproduces how often
    • Component
    • Environment
      • Client
      • Server
    • Additional Context
  • Reproduction
    • Steps
    • Results
  • Root Cause Analysis
  • Proposed Solution
  • Solution Results
  • Solution Denial
    • Why
    • But it's been working fine... ish
  • Partial Conclusion
• Redesign Proposition
  • JSON structure
  • Setup
    • Dev Environment Regions
    • Client Environment Regions
    • Dev Environment Image Mappings
  • Pulling
    • single-redesign.json
    • dual-redesign.json
    • triple-redesign.json
  • Pushing
    • Algorithm
    • Same Environment
    • Different Environment
    • Examples
      • Default Scenario
      • Ignoring a Region
      • Customizing a Region
  • Backwards Compatibility
    • Functionality
    • Structure
    • Release Planning
  • Risks and Mitigation Strategies
  • Impact Analysis
    • Functional
    • Non-Functional
• Next Steps

Bug Report #207

Background

Context

Build tools for VMware Aria has a functionality to pull and push image mappings and regions.

• Regions are the unique vCenter data centers, imported for the linked Aria cloud accounts following the {cloudAccountName} / {dataCenterName} naming convention.

• Images are the available VM templates.

• Image Mappings are named M:M relations between existing images and regions

Image 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-mapping names in Orchestrator's content.yaml as well as the region-mapping tags.

blueprint:
subscription:
flavor-mapping: []
image-mapping: 
  - Single Region
  - Dual Region
  - Triple Region
storage-profile: []
region-mapping:
  cloud-account-tags:
    export-tag: "env:dev"
    import-tags: ["env:dev"]
catalog-item:
custom-resource:
resource-action:
catalog-entitlement:
content-source:
policy:

Pulling

Pulling generates a correct representation of the image mappings, grouped by the respective regions.

Each regions's src-region-profile.json uniquely maps the region by the target's cloudAccountId and regionId itself.

{
  "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-3 images, 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

• Create an image mapping with 2 regions with different values for the same image.
• Pull them and observe the data.
• Push the image mappings and both will have the same value instead of different ones.

Expected behavior

The region mappings to be mapped correctly per region.

Actual behavior

Last one overwrites all the rest.

Reproduces how often

• 100%

Component:

• artifact-manager

Environment

Client

• Build Tools for VMware Aria Version: 2.36.0

Server

• vRealize Automation Version: 8.13
• vRealize Orchestrator Version: 8.13

Additional Context

This is when pulling and pushing to the SAME environment.

Reproduction

Steps

• Setting up an environment with 3 regions and 3 image mappings, as can be seen in the Background section.
  • Single Region, a mapping with a single region.
  • Dual Region, a mapping with two regions.
  • Triple Region, a mapping with three regions.
• Pulling from the environment, as can be seen in the Pulling section.
• Pushing without any changes.
• Expecting the environment image mappings to remain the same.

Results

Executing the reproduction steps produces an incorrect environment state, just as described in the Issue section.

- main\resources\regions\
  - vCenter~8522d781-89ee-422b-92a1-bce602033258\
    - image-mappings\
      - Triple Region.json

{
  "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 externalId of 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 accidentally had been happening all along is that:

1. We usually develop with a single dev region.
2. The client usually has multiple regions.
3. We are pulling image mappings for a single region from our environment.
4. Due to the bug, the mapping populates for all of the regions on the client environment.
5. This is usually a kind-of OK scenario regarding what the client gets in the end, although based on a bug on top of a the original design.

Partial Conclusion

Fixing the issue as proposed will restore the original design, but will actually worsen the 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

• patch on top of a bug on top of the original design,

while simultaneously trying to both fix it and keep the accidental behavior of

• pushing our image mappings on all client regions

Since the issue of how to map dev and client entity 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 of

• Reliably and environmentally independently define image mappings in code.
• Ensuring backwards compatibility.

Redesign 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 rework the image mapping capabilities of the project in order to:

• Better align with the needs of both developers and clients to define, pull and push image mappings.
• Better structure the code with code organized around image mappings, instead of regions.
• Create a solution which works easily for the most common scenario, where we are defining an image mapping for a single dev region, pulling it and then pushing it to every client region.
• Give better and clearer flexibility in ignoring and redefining the default behavior.

JSON structure

{
  name: string,
  default: {
    image: string,
    constraints: string[],
    script: string
  },
  regions: [
    {
      name: string,
      {
        image: string,
        constraints: string[],
        script: string
      }
    }
  ],
  ignoredRegions: string[]
}

Setup

Dev Environment Regions

• vCenter / dc-01a region.
• vCenter / dc-02a region.
• vCenter / dc-03a region.

Client Environment Regions

• vCenter / dc-01a region.
• vCenter / dc-02a region.
• vCenter / dc-03a region.
• vCenter / dc-04a region.
• vCenter / dc-05a region.

Dev Environment Image Mappings

where the Cloud Configuration value of

• vCenter / dc-02a is password:((sensitive:123456))

where the Cloud Configuration value of

• vCenter / dc-02a is password:((sensitive:123456))
• vCenter / dc-03a is password:((secret:v1:gdq7gdgeggfqu882yhf==))

Pulling

Pulling from an environment should generate a single {imageMappingName}.json file per a image mapping in the image-mappings directory.

By default pulling from an environment pulls all image mappings specified in the content.yaml file, with their first region set as default, and all additional regions (if any), pulled into the regions array.

ignoredRegions is 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.json would 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 as default when 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 default property is region agnostic and contains the properties that will be populated for all client regions, unless specified otherwise in the regions or ignoredRegions arrays.

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:

1. Ignore every environment region specified in the ignoredRegions string array if any.
2. Apply the region specific JSON properties, specified in the regions array if any.
3. Apply the default JSON 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-03a
• vCenter / dc-04a
• vCenter / dc-05a

than the original dev environment's region

• vCenter / dc-01a

and since we don't have additional regions definitions or ignoredRegions the expectation is that the default image mapping will be applied to all client regions.

Ignoring a Region

Ignoring the vCenter / dc-05a client region in the ignoredRedions array 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 regions array will override the default values for the listed regions, while all other regions will get the default data.

{
  "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:

• Have a wider stakeholder discussion of the solution design.
• Agree on a release plan and backwards compatibility.
• Draft a technical design document.
• Have a test plan and test the solution.
• Include a QA in the process.
• Use the wider CoE user base to feature test a release candidate.
• Release functionality in parallel to the original one to mitigate production use risks.

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

• Increasing the overall domain knowledge of the image-mapping module
• An opportunity to use this process and draft a Major refactoring proposition process to simplify future support and improve the knowledge base.

Next Steps

• Have a solution design discussion
• Decide on actionable items

[VenelinBakalov]

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:

1. Regarding the default region, would it make sense to add an optional "name" field in there as well - if there is a "name" value, we would push the default only to that region and not have to use "ignoredRegions" field. My question is regarding a use cases where a customer might have 10 regions and want to push only to single region, so in that case he would not have to list the remaining 9 regions in the "ignoredRegions" field.
2. Related to 1., is the default field mandatory? If not, that makes my 1. question obsolete, because I guess the customer can just list the single region in the "regions" array and leave the "default" empty. To be honest, I actually think that if possible to implement, this would be even better approach than my 1. proposal.
3. Regarding backwards compatibility, I saw you proposed to have both this and the original solution for some time until we deprecate the original. My question is, how would we decide which one to use at what time? And actually, my proposal was that maybe we can check the file system and if we find any files, that have the original folder structure, we resolve to using the legacy solution for both pulling and pushing, otherwise go with the new approach. What do you think about that idea? Do you have any other proposal?

[Michaelpalacce]

1+2. We decided that if default is not present, we won't have one
3. We will use a new structure for the proposal, if the new structure is detected, we ignore the old one. (Note: if this is too complicated to do, we will put this as a breaking change for 3.0.0)

[Michaelpalacce]

After a discussion the following was decided:

1. We will keep the data that the user needs to edit in the content.yaml
2. We will keep the content.yaml "resource-centric"
3. We should respect what is in the content.yaml when pulling and pushing
4. If possible, we want to support: regions: [] means no regions, regions: means all regions and if values are given, use that as filter

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"

[ivan-zdravkov]

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 content.yaml to instruct pushing and pulling. The basic redesign proposition pitch is still valid and we'd still need the json files to store the actual data, but with this in mind the previous proposition examples would start looking like this:

JSON structure

{
  name: string,
  regions: [
    {
      name: string,
      image: {
        name: string,
        constraints: string[],
        script: string
      }
    }
  ]
}

single-redesign.json

{
  "name": "Single Redesign",
  regions: [
    {
      name: "vCenter / dc-01a"
      image: {
        "name": "wwcoe_contentlib / empty_VM",
        "constraints": [],
        "script": ""
      }
    }
  ]
}

dual-redesign.json

{
  "name": "Dual Redesign",
  regions: [
    {
      name: "vCenter / dc-01a"
      image: {
        "name": "wwcoe_contentlib / empty_VM",
        "constraints": [],
        "script": ""
      }
    },
    {
      name: "vCenter / dc-02a"
      image: {
        "name": "wwcoe_contentlib / Ubuntu_18.04",
        "constraints": ["env:dev:hard", "nsx:hard"],
        "script": "password:((sensitive:123456))"
      }
    }
  ]
}

triple-redesign.json

{
  "name": "Triple Redesign",
  regions: [
    {
      name: "vCenter / dc-01a"
      image: {
        "name": "wwcoe_contentlib / empty_VM",
        "constraints": [],
        "script": ""
      }
    },
    {
      name: "vCenter / dc-02a"
      image: {
        "name": "wwcoe_contentlib / Ubuntu_18.04",
        "constraints": ["env:dev:hard", "nsx:hard"],
        "script": "password:((sensitive:123456))"
      }
    },
    {
      name: "vCenter / dc-03a"
      image: {
        "name": "wwcoe_contentlib / Win2022",
        "constraints": ["access:ext:hard"],
        "script": "password:((secret:v1:gdq7gdgeggfqu882yhf==))"
      }
    }
  ]
}

In all of the cases, the JSON files contain only the data of the image/region relation.

The content.yaml file contains the logic of how to pull, build and push those.

Examples

Pushing the vCenter / dc-02a region as default to all client regions.

images:
  ignoredRegions: []
  regions: []
  default: "vCenter / dc-02a"

Pushing the vCenter / dc-02a region as default for all client regions except for vCenter / dc-01a which gets ignored.

images:
  ignoredRegions:
    - "vCenter / dc-01a"
  regions: []
  default: "vCenter / dc02a"

Pushing the vCenter / dc-02a region as default for all client regions except for vCenter / dc-01a which gets it's own values overridden.

images:
  ignoredRegions: []
  regions:
    - "vCenter / dc-01a"
  default: "vCenter / dc02a"

Pushing the vCenter / dc-02a region as default for all client regions except for vCenter / dc-01a which gets it's own values overridden and vCenter / dc05a and vCenter / dc06a which are ignored.

images:
  ignoredRegions:
    - "vCenter / dc05a"
    - "vCenter / dc06a"
  regions:
    - "vCenter / dc-01a"
  default: "vCenter / dc02a"

Not having a default and pushing all specified regions.

images:
  ignoredRegions: []
  regions:
    - "vCenter / dc-02a"
    - "vCenter / dc-03a"
  default:

Limitations

Thus proposed, the solution lacks the availability to have more specific image/region instructions and won't be able to handle cases where one image mapping populates for a certain set of regions and another image mapping for another set of regions.

[VenelinBakalov]

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.
It can be re-opened in case for some reason we need to go back and implement the redesign in some 2.x.x related branch.

Again, @ivan-zdravkov thank you for the huge effort you put in this!