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

Chore: tidy appHandler with serveral deprecation #6300

Merged
merged 4 commits into from
Aug 28, 2023

Conversation

chivalryq
Copy link
Member

@chivalryq chivalryq commented Aug 25, 2023

Description of your changes

This PR mainly have several effects:

  1. Depracate PackagedWorkloadResources and PackagedTraitResources field in ComponentManifest. They are for helm-based component which has been depracated in schematic of ComponentDefinition
  2. Rename StandardWorkload -> ComponentOutput and Traits -> ComponentOutputAndTraits. The original names (StandardWorkload and Traits) can not reflect the actual usage of the fields.
  • ComponentOutput contains K8s resource generated from "output" block of ComponentDefinition
  • ComponentOutputsAndTraits contains both resources generated from "outputs" block of ComponentDefinition and resources generated from TraitDefinition
  1. Remove some unreachable code after depracation.

🤖 Generated by Copilot at 70e261c

Summary

🚚🗑️🛠️

Refactored the appfile package and related packages to use a new data model for application components and their outputs. Renamed some structs and fields to align with the OAM spec and improve clarity. Simplified the application revision logic to use component manifests directly. Updated the tests and CLI commands to reflect the changes.

Sing, O Muse, of the mighty refactor that reshaped the appfile
And how the valiant developers renamed the fields of ComponentOutput
To match the OAM spec and avoid confusion in the code
Like the wise Athena changing her form to aid the heroes.

Walkthrough

  • Rename StandardWorkload to ComponentOutput and Traits to ComponentOutputsAndTraits in ComponentManifest type to avoid confusion with OAM spec (link)
  • prepareArtifactsData and SetOAMContract in pkg/appfile/appfile.go (link, link)
  • evalWorkloadWithContext in pkg/appfile/appfile.go and pkg/appfile/appfile_test.go (link, link, link, link)
  • PrepareBeforeApply and HandleCheckManageWorkloadTrait in pkg/controller/core.oam.dev/v1beta1/application/assemble/assemble.go (link, link, link)
  • generateManifest in pkg/appfile/dryrun/diff.go (link, link)
  • PrintDryRun in pkg/appfile/dryrun/dryrun.go (link, link)
  • TestGenerateAppRevision and TestGenerateAppRevisionWithWorkflow in pkg/controller/core.oam.dev/v1beta1/application/application_controller_test.go (link, link)
  • TestGenerateComponentFromCUEModule, TestEvalWorkloadWithContext, TestPrepareArtifactsData, and TestGenerateComponentFromCUEModuleWithWorkflow in pkg/appfile/appfile_test.go (link, link, link, link, link)
  • renderComponentsAndTraits, getComponentResources, and overrideTraits in pkg/controller/core.oam.dev/v1beta1/application/generator.go (link, link, link, link)
  • Rename overrideTraits to redirectTraitToLocalIfNeed in pkg/controller/core.oam.dev/v1beta1/application/generator.go to better reflect its purpose and update the reference in renderComponentsAndTraits (link, link)
  • Remove ProduceArtifacts function and related constants and imports in pkg/controller/core.oam.dev/v1beta1/application/apply.go and pkg/controller/core.oam.dev/v1beta1/application/revision.go as they are no longer needed after refactoring the application revision logic to use the component manifests directly (link, link, link, link, link)
  • Remove ComputeComponentRevisionHash function and related import in pkg/controller/core.oam.dev/v1beta1/application/revision.go as it is no longer needed after refactoring the application revision logic to use the component revision name and hash labels (link, link)
  • Remove calls to ProduceArtifacts function in various tests in pkg/controller/core.oam.dev/v1beta1/application/revision_test.go as the function is no longer needed and removed (link, link, link, link, link, link)
  • Update debugComponents function in references/cli/debug.go to use the new field names of ComponentOutput and ComponentOutputsAndTraits when creating the maps of components and traits from the component manifests (link)

Fixes #

I have:

  • Read and followed KubeVela's contribution process.
  • Related Docs updated properly. In a new feature or configuration option, an update to the documentation is necessary.
  • 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

Special notes for your reviewer

Signed-off-by: Qiaozp <qiaozhongpei.qzp@alibaba-inc.com>
@codecov
Copy link

codecov bot commented Aug 25, 2023

Codecov Report

Patch coverage: 71.05% and project coverage change: -33.18% ⚠️

Comparison is base (bdf9bf1) 66.30% compared to head (32fcdd1) 33.13%.
Report is 1 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff             @@
##           master    #6300       +/-   ##
===========================================
- Coverage   66.30%   33.13%   -33.18%     
===========================================
  Files         183      180        -3     
  Lines       23954    23770      -184     
===========================================
- Hits        15883     7875     -8008     
- Misses       6508    14912     +8404     
+ Partials     1563      983      -580     
Flag Coverage Δ
core-unittests ?
e2e-multicluster-test 31.35% <71.05%> (+0.28%) ⬆️
e2etests 33.08% <78.78%> (+0.14%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed Coverage Δ
pkg/appfile/dryrun/diff.go 27.08% <0.00%> (-54.18%) ⬇️
pkg/appfile/dryrun/dryrun.go 1.40% <0.00%> (-52.34%) ⬇️
...ntroller/core.oam.dev/v1beta1/application/apply.go 60.68% <ø> (-25.27%) ⬇️
...oller/core.oam.dev/v1beta1/application/revision.go 67.85% <ø> (-2.41%) ⬇️
pkg/appfile/appfile.go 46.45% <72.72%> (-21.07%) ⬇️
...e.oam.dev/v1beta1/application/assemble/assemble.go 72.72% <75.00%> (+8.95%) ⬆️
...ller/core.oam.dev/v1beta1/application/generator.go 74.19% <92.85%> (-9.77%) ⬇️

... and 124 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Signed-off-by: Qiaozp <qiaozhongpei.qzp@alibaba-inc.com>
Signed-off-by: Qiaozp <qiaozhongpei.qzp@alibaba-inc.com>
Signed-off-by: qiaozp <qiaozhongpei.qzp@alibaba-inc.com>
Copy link
Collaborator

@wonderflow wonderflow left a comment

Choose a reason for hiding this comment

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

this is not a refactor, it's a deprecation with several API changes.

@chivalryq chivalryq changed the title Refactor: tidy appHandler Chore: tidy appHandler Aug 26, 2023
@chivalryq
Copy link
Member Author

this is not a refactor, it's a deprecation with several API changes.

Yes, I've chanegd the title.

@wonderflow
Copy link
Collaborator

@chivalryq It's still not clear, you'd better make a clarification about which features are deprecated.

@chivalryq
Copy link
Member Author

ping @wonderflow

@chivalryq chivalryq merged commit 94cbcad into kubevela:master Aug 28, 2023
@wonderflow wonderflow changed the title Chore: tidy appHandler Chore: tidy appHandler with serveral deprecation Aug 28, 2023
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