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

add interpret command #2750

Merged
merged 1 commit into from
Nov 22, 2022
Merged

Conversation

ikaven1024
Copy link
Member

@ikaven1024 ikaven1024 commented Nov 5, 2022

Signed-off-by: yingjinhui yingjinhui@didiglobal.com

What type of PR is this?
/kind feature

What this PR does / why we need it:
add skeleton of interpreter command.

Which issue(s) this PR fixes:
part of #2371

Special notes for your reviewer:

Usage

Check the valid of customizations. Or show the result of the rules running.

  1. 
 Check valid. Given the customization by NAME or '-f' option, this tool will check the rules in customization, and print
'PASS' or 'FAIL' for each rule. The fail reasons are also printed.
  2. 
 Execute rules and show result. Given the customization by NAME or '-f' option, the operation by 'operation' option, and
arguments by options(desired, desired-file, observed, observed-file, status, status-file, desired-replica),  this tool
will run the rule and print result. The arguments for operations see below.

  *  retention                : (desired|desired-file), (observed|observed-file)
  *  replicaResource          : (desired|desired-file|observed|observed-file)
  *  replicaRevision          : (desired|desired-file|observed|observed-file), desired-replica
  *  statusReflection         : (desired|desired-file|observed|observed-file)
  *  statusAggregation        : (desired|desired-file|observed|observed-file), status-file...
  *  healthInterpretation     : (desired|desired-file|observed|observed-file)
  *  dependencyInterpretation : (desired|desired-file|observed|observed-file)

Examples:
  # Check the customizations in file
  karmadactl interpret -f customization.json --check
  # Check the customizations on server
  karmadactl interpret customization --check
  # Check the retention rule
  karmadactl interpret customization --operation retention --check
  
  # Execute the retention rule for local resource
  karmadactl interpret customization --operation retention --desired-file desired.json --observed-file observed.json
  # Execute the retention rule for server resource
  karmadactl interpret customization --operation retention --desired-file desired.json --observed deployments/foo

Options:
    --check=false:
        Check the given customizations

    --desired='':
        The type and name of resource on server to use as desiredObj argument in rule script. For example:
        deployment/foo.

    --desired-file='':
        Filename, directory, or URL to files identifying the resource to use as desiredObj argument in rule script.

    --desired-replica=0:
        The desiredReplica argument in rule script.

    -f, --filename=[]:
        Filename, directory, or URL to files containing the customizations

    --karmada-context='':
        The name of the kubeconfig context to use

    -n, --namespace='':
        If present, the namespace scope for this CLI request

    --observed='':
        The type and name of resource on server to use as observedObj argument in rule script. For example:
        deployment/foo.

    --observed-file='':
        Filename, directory, or URL to files identifying the resource to use as observedObj argument in rule script.

    --operation='':
        The interpret operation to use. One of:
        (retention,replicaResource,replicaRevision,statusReflection,statusAggregation,healthInterpretation,dependencyInterpretation)

    -R, --recursive=false:
        Process the directory used in -f, --filename recursively. Useful when you want to manage related manifests
        organized within the same directory.

    --status-file=[]:
        Filename, directory, or URL to files identifying the resource to use as statusItems argument in rule script.

Usage:
  karmadactl interpret (-f FILENAME | NAME) (--operation OPERATION) [-n NAMESPACE] [--ARGS VALUE]...  [options]

Use "karmadactl options" for a list of global command-line options (applies to all commands).

Check

Prepare a customization.yaml

apiVersion: config.karmada.io/v1alpha1
kind: ResourceInterpreterCustomization
metadata:
  name: customization
spec:
  target:
    apiVersion: apps/v1
    kind: Deployment
  customizations:
    retention:
      luaScript: >
        function Retain(desiredObj, runtimeObj)
          desiredObj.spec.fieldFoo = runtimeObj.spec.fieldFoo
          return desiredObj
        end
    replicaResource:
      luaScript: >
        bad format
interpret -f customization.yml --check

Output:

-----------------------------------
SOURCE: customization
TARGET: apps/v1 Deployment   
RULERS:
    retention:                  PASS
    replicaResource:            ERROR: <string> line:1(column:10) near 'format':   parse error   
    replicaRevision:            UNSET
    statusReflection:           UNSET
    statusAggregation:          UNSET
    healthInterpretation:       UNSET
    dependencyInterpretation:   UNSET

Does this PR introduce a user-facing change?:

`karmadactl`: add interpreter command for resource interpret customizations.

@karmada-bot karmada-bot added the kind/feature Categorizes issue or PR as related to a new feature. label Nov 5, 2022
@karmada-bot karmada-bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Nov 5, 2022
@ikaven1024
Copy link
Member Author

ikaven1024 commented Nov 5, 2022

@RainbowMango: PLZ check the usage of this command.

@codecov-commenter
Copy link

codecov-commenter commented Nov 5, 2022

Codecov Report

Merging #2750 (79b63d6) into master (594ad9f) will decrease coverage by 0.05%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           master    #2750      +/-   ##
==========================================
- Coverage   37.91%   37.85%   -0.06%     
==========================================
  Files         189      190       +1     
  Lines       17644    17696      +52     
==========================================
+ Hits         6689     6699      +10     
- Misses      10548    10590      +42     
  Partials      407      407              
Flag Coverage Δ
unittests 37.85% <0.00%> (-0.06%) ⬇️

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

Impacted Files Coverage Δ
...ceinterpreter/configurableinterpreter/luavm/lua.go 55.83% <0.00%> (-2.21%) ⬇️
...armadactl/cmdinit/karmada/webhook_configuration.go 81.91% <0.00%> (-6.53%) ⬇️
pkg/karmadactl/util/idempotency.go 6.74% <0.00%> (-1.60%) ⬇️
pkg/karmadactl/taint/taint.go 43.75% <0.00%> (ø)
pkg/karmadactl/cmdinit/utils/rbac.go 0.00% <0.00%> (ø)
pkg/karmadactl/cmdinit/karmada/deploy.go 0.00% <0.00%> (ø)
.../resourceinterpreter/defaultinterpreter/default.go 0.00% <0.00%> (ø)
pkg/util/metrics/metrics.go 71.42% <0.00%> (ø)
pkg/karmadactl/cmdinit/karmada/rbac.go 88.57% <0.00%> (+0.45%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@ikaven1024 ikaven1024 force-pushed the inpreter-ctl branch 3 times, most recently from 3a0076c to b32ea52 Compare November 6, 2022 12:55
@ikaven1024
Copy link
Member Author

@RainbowMango Will you merge this PR when you feel ok? I will do this work in several PRs.

@lonelyCZ
Copy link
Member

lonelyCZ commented Nov 7, 2022

Hi, @ikaven1024 perhaps you can develop it in this PR as multiple commits.

Because the karmadactl will have a big change recently at #2664

@karmada-bot karmada-bot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Nov 8, 2022
@ikaven1024 ikaven1024 changed the title add skeleton of interpreter command [WIP] add interpreter command Nov 8, 2022
@karmada-bot karmada-bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 8, 2022
@ikaven1024 ikaven1024 force-pushed the inpreter-ctl branch 2 times, most recently from 039d962 to 1b0b5be Compare November 14, 2022 08:29
@ikaven1024
Copy link
Member Author

Waiting for #2794 ready

@ikaven1024 ikaven1024 force-pushed the inpreter-ctl branch 2 times, most recently from f5b5699 to f129cb6 Compare November 16, 2022 12:14
@ikaven1024 ikaven1024 changed the title [WIP] add interpreter command add interpreter command Nov 16, 2022
@karmada-bot karmada-bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 16, 2022
@ikaven1024
Copy link
Member Author

@RainbowMango @XiShanYongYe-Chang It's ready for review.

@ikaven1024
Copy link
Member Author

OK. Please help to rebase your code since we have solved some fake tests in these two days.

@XiShanYongYe-Chang I have rebase the master, but still fail:
https://github.com/karmada-io/karmada/actions/runs/3485018546/jobs/5830406361#step:5:3394

@karmada-bot karmada-bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Nov 17, 2022
@ikaven1024 ikaven1024 changed the title add interpreter command add interpret command Nov 17, 2022
@ikaven1024
Copy link
Member Author

I have pick out the execute mod into #2824, make this pr smaller

@RainbowMango
Copy link
Member

/assign

@XiShanYongYe-Chang
Copy link
Member

@XiShanYongYe-Chang I have rebase the master, but still fail:
https://github.com/karmada-io/karmada/actions/runs/3485018546/jobs/5830406361#step:5:3394

This error is caused by the failure to delete the Kind node and can be ignored.

@ikaven1024 ikaven1024 force-pushed the inpreter-ctl branch 2 times, most recently from 4d1053d to 3de2389 Compare November 18, 2022 08:15
Copy link
Member

@RainbowMango RainbowMango left a comment

Choose a reason for hiding this comment

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

I believe this approach is much better now.

The review is still ongoing.

pkg/karmadactl/interpret/interpret.go Outdated Show resolved Hide resolved
pkg/karmadactl/interpret/interpret.go Outdated Show resolved Hide resolved
pkg/karmadactl/interpret/interpret.go Outdated Show resolved Hide resolved
pkg/karmadactl/interpret/interpret.go Outdated Show resolved Hide resolved
pkg/karmadactl/interpret/check.go Outdated Show resolved Hide resolved
Copy link
Member

@RainbowMango RainbowMango left a comment

Choose a reason for hiding this comment

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

Some small touches, otherwise LGTM.

pkg/karmadactl/interpret/interpret.go Show resolved Hide resolved
pkg/karmadactl/interpret/check.go Outdated Show resolved Hide resolved
pkg/karmadactl/interpret/interpret.go Outdated Show resolved Hide resolved
pkg/karmadactl/interpret/interpret.go Outdated Show resolved Hide resolved
pkg/karmadactl/interpret/interpret.go Outdated Show resolved Hide resolved
pkg/karmadactl/interpret/interpret.go Outdated Show resolved Hide resolved
pkg/karmadactl/interpret/interpret.go Outdated Show resolved Hide resolved
pkg/karmadactl/interpret/interpret.go Outdated Show resolved Hide resolved
pkg/karmadactl/interpret/interpret.go Outdated Show resolved Hide resolved
Signed-off-by: yingjinhui <yingjinhui@didiglobal.com>
Copy link
Member

@RainbowMango RainbowMango left a comment

Choose a reason for hiding this comment

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

/lgtm
/approve

@karmada-bot karmada-bot added the lgtm Indicates that a PR is ready to be merged. label Nov 22, 2022
@karmada-bot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: RainbowMango

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@karmada-bot karmada-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 22, 2022
@karmada-bot karmada-bot merged commit 9605609 into karmada-io:master Nov 22, 2022
@ikaven1024 ikaven1024 deleted the inpreter-ctl branch November 28, 2022 15:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. kind/feature Categorizes issue or PR as related to a new feature. lgtm Indicates that a PR is ready to be merged. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants