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

Feat: support resource update policy #6003

Merged

Conversation

Somefive
Copy link
Collaborator

@Somefive Somefive commented May 16, 2023

Description of your changes

  1. Support "resource-update" policy to allow users customize the update behavior for selected resources.
  2. Rename FeatureGate "ApplyResourceByUpdate" to "ApplyResourceByReplace" to avoid confusion.

resource-update policy can allow users to customize the update behavior for selected resources.

apiVersion: core.oam.dev/v1beta1
kind: Application
metadata:
  name: recreate
spec:
  components:
    - type: k8s-objects
      name: recreate
      properties:
        objects:
          - apiVersion: v1
            kind: Secret
            metadata:
              name: recreate
            data:
              key: dgo=
            immutable: true
  policies:
    - type: resource-update
      name: resource-update
      properties:
        rules:
          - selector:
              resourceTypes: ["Secret"]
            strategy:
              recreateFields: ["data.key"]

By specifying recreateFields, the application will recreate the target resource (Secret here) when the field changes (data.key here). If the field is not changed, the application will use the normal update (patch here).

apiVersion: core.oam.dev/v1beta1
kind: Application
metadata:
  name: recreate
spec:
  components:
    - type: k8s-objects
      name: recreate
      properties:
        objects:
          - apiVersion: v1
            kind: ConfigMap
            metadata:
              name: recreate
            data:
              key: val
  policies:
    - type: resource-update
      name: resource-update
      properties:
        rules:
          - selector:
              resourceTypes: ["ConfigMap"]
            strategy:
              op: replace

By specifying op to replace, the application will update the given resource (ConfigMap here) by replace. Compared to patch, which leverages three-way merge patch to only modify the fields managed by KubeVela application, "replace" will update the object as a whole and wipe out other fields even if it is not managed by the KubeVela application. It can be seen as an "application-level" ApplyResourceByReplace.

Fixes #5594

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

@Somefive Somefive force-pushed the feat/support-resource-update-policy branch 2 times, most recently from fba151f to 580d8fd Compare May 16, 2023 08:58
Signed-off-by: Somefive <yd219913@alibaba-inc.com>
@Somefive Somefive force-pushed the feat/support-resource-update-policy branch from 580d8fd to 08b028f Compare May 16, 2023 09:33
@codecov
Copy link

codecov bot commented May 16, 2023

Codecov Report

Patch coverage: 76.11% and project coverage change: -1.11 ⚠️

Comparison is base (da8588c) 60.81% compared to head (08b028f) 59.71%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6003      +/-   ##
==========================================
- Coverage   60.81%   59.71%   -1.11%     
==========================================
  Files         224      224              
  Lines       31206    31270      +64     
==========================================
- Hits        18979    18672     -307     
- Misses      10454    10788     +334     
- Partials     1773     1810      +37     
Flag Coverage Δ
core-unittests 55.93% <31.34%> (-0.05%) ⬇️
e2e-multicluster-test 24.85% <71.64%> (+0.02%) ⬆️
e2etests ?

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

Impacted Files Coverage Δ
pkg/features/controller_features.go 100.00% <ø> (ø)
pkg/resourcekeeper/resourcekeeper.go 58.62% <0.00%> (-3.20%) ⬇️
pkg/appfile/parser.go 61.99% <50.00%> (-0.75%) ⬇️
pkg/utils/apply/apply.go 83.63% <76.47%> (-3.25%) ⬇️
pkg/resourcekeeper/dispatch.go 72.72% <100.00%> (-5.40%) ⬇️
pkg/resourcekeeper/statekeep.go 59.13% <100.00%> (+1.36%) ⬆️
pkg/resourcekeeper/utils.go 100.00% <100.00%> (ø)

... and 28 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Member

@chivalryq chivalryq left a comment

Choose a reason for hiding this comment

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

LGTM

@chivalryq chivalryq merged commit 530d7c5 into kubevela:master May 17, 2023
23 of 24 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.

[Feature] ImmutableAfterDeploy
2 participants