flyte icon indicating copy to clipboard operation
flyte copied to clipboard

[BUG] Invalid "resources" scope in clusterresourcesync/deployment.yaml

Open lowc1012 opened this issue 1 year ago • 1 comments

Tracking issue

Closes #4915

Why are the changes needed?

fix a bug in clusterresourcesync/deployment.yaml when users identify resources

What changes were proposed in this pull request?

  1. Add resources: {} into charts/flyte-core/values.yaml as default value
  2. Use with to render resources for deployment if the .Values.cluster_resource_manager.resources is not empty.

How was this patch tested?

Setup process

Screenshots

Check all the applicable boxes

  • [x] I updated the documentation accordingly.
  • [ ] All new and existing tests passed.
  • [x] All commits are signed-off.

Related PRs

Docs link

lowc1012 avatar Feb 17 '24 12:02 lowc1012

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 58.97%. Comparing base (b43af79) to head (f406f4b). Report is 7 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #4916   +/-   ##
=======================================
  Coverage   58.97%   58.97%           
=======================================
  Files         645      645           
  Lines       55506    55559   +53     
=======================================
+ Hits        32732    32765   +33     
- Misses      20174    20198   +24     
+ Partials     2600     2596    -4     
Flag Coverage Δ
unittests 58.97% <ø> (+<0.01%) :arrow_up:

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

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Feb 18 '24 11:02 codecov[bot]

Verified that resources is a property of io.k8s.api.core.v1.Container (in https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/)

eapolinario avatar Feb 29 '24 22:02 eapolinario