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: use YAML format as experiment description #1029

Merged
merged 31 commits into from Oct 26, 2020

Conversation

g1eny0ung
Copy link
Member

@g1eny0ung g1eny0ung commented Oct 10, 2020

What problem does this PR solve?

This PR aims to use YAML format as an experiment detail in the dashboard.

Motivation:

Usually, we can create or update a chaos experiment by run kubectl apply -f. But the dashboard isn't, we reorder some spec values into different areas, especially in the update editor. The difficulty is The user will face a very huge JSON structure, but our documentation doesn't explain this (because we use inside the API). And also, to handle different chaos, our update codes add much more conditions and patches which are hard to maintain.

So to solve this problem it's better to make the experiment detail structure consistent with the CRD structure.

What is changed and how does it work?

  • JSON editor in the dashboard detail page to YAML editor
  • Update /experiments/detail/:uid to return the same structure as the chaos YAML
  • Update /experiments/update to receive the same structure as the chaos YAML
  • Also update archives related codes
  • Fix DeleteIncompleteExperiments, call it after fx providers already initialized.
  • Some bugs fixes

Checklist

Tests

  • Unit test
  • E2E test
  • Manual test (add detailed scripts or steps below)
  • No code

Side effects

  • Breaking backward compatibility

Related changes

  • Need to update the documentation

Does this PR introduce a user-facing change?

NONE

Signed-off-by: Yue Yang <g1enyy0ung@gmail.com>
Signed-off-by: Yue Yang <g1enyy0ung@gmail.com>
Signed-off-by: Yue Yang <g1enyy0ung@gmail.com>
@codecov-io
Copy link

codecov-io commented Oct 10, 2020

Codecov Report

Merging #1029 into master will decrease coverage by 7.89%.
The diff coverage is 26.37%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1029      +/-   ##
==========================================
- Coverage   55.78%   47.89%   -7.90%     
==========================================
  Files          68       72       +4     
  Lines        4383     4320      -63     
==========================================
- Hits         2445     2069     -376     
- Misses       1768     2042     +274     
- Partials      170      209      +39     
Impacted Files Coverage Δ
api/v1alpha1/common_types.go 0.00% <0.00%> (ø)
api/v1alpha1/common_validation.go 100.00% <ø> (ø)
api/v1alpha1/common_webhook.go 100.00% <ø> (ø)
api/v1alpha1/dnschaos_type.go 0.00% <0.00%> (ø)
api/v1alpha1/dnschaos_webhook.go 0.00% <0.00%> (ø)
api/v1alpha1/httpchaos_types.go 0.00% <0.00%> (ø)
api/v1alpha1/iochaos_types.go 0.00% <ø> (-40.00%) ⬇️
api/v1alpha1/kernelchaos_types.go 0.00% <ø> (-20.00%) ⬇️
api/v1alpha1/kernelchaos_webhook.go 100.00% <ø> (+14.81%) ⬆️
api/v1alpha1/kinds.go 27.27% <ø> (+0.60%) ⬆️
... and 107 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8b3f975...4bcf249. Read the comment docs.

Signed-off-by: Yue Yang <g1enyy0ung@gmail.com>
Signed-off-by: Yue Yang <g1enyy0ung@gmail.com>
Signed-off-by: Yue Yang <g1enyy0ung@gmail.com>
Signed-off-by: Yue Yang <g1enyy0ung@gmail.com>
Signed-off-by: Yue Yang <g1enyy0ung@gmail.com>
Signed-off-by: Yue Yang <g1enyy0ung@gmail.com>
Signed-off-by: Yue Yang <g1enyy0ung@gmail.com>
Signed-off-by: Yue Yang <g1enyy0ung@gmail.com>
@g1eny0ung g1eny0ung changed the title [WIP] feat: use YAML format as experiment description feat: use YAML format as experiment description Oct 12, 2020
@g1eny0ung g1eny0ung requested review from fewdan and cwen0 and removed request for fewdan October 12, 2020 10:07
Signed-off-by: Yue Yang <g1enyy0ung@gmail.com>
Signed-off-by: Yue Yang <g1enyy0ung@gmail.com>
Signed-off-by: Yue Yang <g1enyy0ung@gmail.com>
@fewdan
Copy link
Member

fewdan commented Oct 14, 2020

Please resolve conflicting files.

Signed-off-by: Yue Yang <g1enyy0ung@gmail.com>
Signed-off-by: Yue Yang <g1enyy0ung@gmail.com>
fewdan
fewdan previously approved these changes Oct 15, 2020
Copy link
Member

@fewdan fewdan left a comment

Choose a reason for hiding this comment

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

LGTM

pkg/store/dbstore/store.go Outdated Show resolved Hide resolved
Signed-off-by: Yue Yang <g1enyy0ung@gmail.com>
pkg/store/experiment/experiment.go Outdated Show resolved Hide resolved
pkg/store/experiment/experiment.go Outdated Show resolved Hide resolved
if exp.FinishTime.Add(ttl).Before(nowTime) {
if err := e.db.Table("archive_experiments").Unscoped().Delete(*exp).Error; err != nil {
if err := e.db.Table("experiments").Unscoped().Delete(*exp).Error; err != nil {
Copy link
Member

Choose a reason for hiding this comment

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

keep archive_experiments as the name of table

Copy link
Member

@cwen0 cwen0 left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@zhouqiang-cl zhouqiang-cl left a comment

Choose a reason for hiding this comment

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

LGTM

@zhouqiang-cl
Copy link
Contributor

/merge

@ti-srebot
Copy link
Contributor

/run-all-tests

@ti-srebot
Copy link
Contributor

/run-all-tests

@fewdan fewdan merged commit 4c55adb into chaos-mesh:master Oct 26, 2020
@g1eny0ung g1eny0ung deleted the feat/yaml-editor branch October 27, 2020 02:16
@dcalvin dcalvin mentioned this pull request Feb 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants