Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

config: CNF installation (2.1) Transformer for old version of config #2147

Merged
merged 1 commit into from
Sep 26, 2024

Conversation

svteb
Copy link
Collaborator

@svteb svteb commented Sep 16, 2024

IMPORTANT

To speed up the development of the #2120 epic we are currently experimenting with a different approach to pull requests. This pull request has been created anew from a different branch, if you want to check the already existing discussion around it, view the old #2133.

Description

The transformer transforms the old config according to the structure proposed in #2129.

Notes

  1. To allow for some input validation, the v1 config was reimplemented as class in config_v1.cr.
  2. Certain arguments are found in cnf-testsuite.yml files spread across the samples and examples that are unused anywhere in the code, these are still parsed but ignored to avoid too much manual intervention in later stages.
  3. A new task has been added to allow for transformation, users can execute it by calling this command:

./cnf-testsuite transform_config OLD_FILE_PATH NEW_FILE_PATH

  1. The ConfigTransformer class found in config_transformer.cr automatically detects the version of the old config and transforms it to the latest version (currently only v1 -> v2), but allows for future extendibility.
  2. Upon determining the version of the old config, the main transform function is called which does the actual transformation in code (through use of Hashes and YAML::Any).
  3. The transform function uses the appropriate transformation rules (V1ToV2Transformation class in v1_to_v2_transformation.cr file)
  4. To make future extension of transformation rules easier, the function process_data has been added (TransformationBase class in transformation.cr file), this function converts the underlying hashes and arrays to the YAML::Any type and removes any nil branches so they don't clutter the final output.
  5. After the transformation is done the result can be printed to either String or dumped to a file through the serialize_to_string and serialize_to_file functions.
  6. Most edge cases have been resolved through the use of exceptions and their catching, giving users a readable output in case of any errors.

Issues:

Refs: #2130

How has this been tested:

  • Covered by existing integration testing
  • Added integration testing to cover
  • Verified all A/C passes
    • develop
    • master
    • tag/other branch
  • Test environment
    • Shared Packet K8s cluster
    • New Packet K8s cluster
    • Kind cluster
  • Have not tested

Types of changes:

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update

Checklist:

Documentation

  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • No updates required.

Code Review

  • Does the test handle fatal exceptions, ie. rescue block

Issue

  • Tasks in issue are checked off

@svteb svteb changed the title config: Transformer for old version of config config: CNF installation (2.1) Transformer for old version of config Sep 16, 2024
CNF_TESTSUITE_YML_USAGE.md Outdated Show resolved Hide resolved
CNF_TESTSUITE_YML_USAGE.md Outdated Show resolved Hide resolved
CNF_TESTSUITE_YML_USAGE.md Outdated Show resolved Hide resolved
src/tasks/transform_config.cr Outdated Show resolved Hide resolved
src/tasks/transform_config.cr Outdated Show resolved Hide resolved
@svteb svteb force-pushed the 2120-config-transformer branch 2 times, most recently from ec9816a to 9ecc279 Compare September 23, 2024 08:07
Copy link
Collaborator

@kosstennbl kosstennbl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Refs: #2130
- Generic transformer for cnf-testsuite.yml configs to newer versions.
- Extendable through addition of new transformation rules.
- The current functionality transforms to configv2, the structure of which
was proposed in #2129.

Signed-off-by: svteb <slavo.valko@tietoevry.com>
Copy link
Collaborator

@martin-mat martin-mat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

Copy link
Collaborator

@Smitholi67 Smitholi67 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@martin-mat martin-mat merged commit 6c76d20 into main Sep 26, 2024
167 of 174 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants