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

Rename migration.Converter.ComposedTemplates as Composition #148

Merged
merged 7 commits into from
Feb 2, 2023

Conversation

ulucinar
Copy link
Collaborator

@ulucinar ulucinar commented Jan 4, 2023

Description of your changes

This PR proposes changes to the migration.Converter interface so that:

  • migration.Converter.ComposedTemplates is renamed to Composition
  • migration.Converter.Composition accepts the spec.patchSets of a Composition, and
  • Allows conversion of the spec.patchSets of Composition's by the registered migration converters.

Also we have the following improvements for the migration converters:

  • Include references to patch sets in the set of patches for migration targets by default (assumption is patch sets are reusable and thus probably applicable to the migration targets, so include refs to them by default). Target schemas are checked and if a referenced patchset does not conform to the target's schema, it's removed from the set of patches.
  • Remove patches from targets if the patch target/source field does not exist or is of a different type in the target, by default
  • Include patches in targets if the patch target/source field exists and is of the same type in the target, by default

I have:

  • Read and followed Crossplane's contribution process.
  • Run make reviewable to ensure this PR is ready for review.
  • Added backport release-x.y labels to auto-backport this PR if necessary.

How has this code been tested

@ulucinar ulucinar force-pushed the migrate-patchsets branch 10 times, most recently from 0ee754c to 99b4416 Compare January 10, 2023 21:57
Copy link
Member

@ytsarev ytsarev left a comment

Choose a reason for hiding this comment

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

Using the code from this PR, I successfully created a set of working converters, so I believe it is mergeable.

Thanks a lot for the outstanding work, @ulucinar!

Copy link
Member

@sergenyalcin sergenyalcin left a comment

Choose a reason for hiding this comment

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

Thanks @ulucinar I left comments about documentation.

pkg/migration/converter.go Show resolved Hide resolved
pkg/migration/patches.go Show resolved Hide resolved
- And allow conversions on a Composite's spec.patchSets

Signed-off-by: Alper Rifat Ulucinar <[email protected]>
- This method is called on a single migration source MR
  to have it converted/upgraded. So the semantics is
  to convert a resource.

Signed-off-by: Alper Rifat Ulucinar <[email protected]>
… migration converters

Signed-off-by: Alper Rifat Ulucinar <[email protected]>
…nd target kinds do not match

- Generate unique names for target composed templates

Signed-off-by: Alper Rifat Ulucinar <[email protected]>
…targets if they conform to the target's schema

- Remove patches from targets if the patch target/source field does not exist or is of a different type in the target
- Include patches in targets if the patch target/source field exists and is of the same type in the target

Signed-off-by: Alper Rifat Ulucinar <[email protected]>
@ulucinar ulucinar merged commit aeb5f15 into crossplane:main Feb 2, 2023
@ulucinar ulucinar deleted the migrate-patchsets branch February 2, 2023 12:14
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.

3 participants