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

refactor: optimize allInjected and allRecovered states #4199

Merged

Conversation

g1eny0ung
Copy link
Member

What problem does this PR solve?

Optimize allInjected and allRecovered states when targets are not selected.

Before:

2023-09-16T02:51:42.517Z	INFO	condition	condition/controller.go:122	updating conditions
{"conditions": [{"type":"Paused","status":"False"},{"type":"Selected","status":"False"},{"type":"AllInjected","status":"True"},{"type":"AllRecovered","status":"True"}]}

After:

2023-09-16T03:57:50.937Z	INFO	condition	condition/controller.go:132	updating conditions
{"conditions": [{"type":"Paused","status":"False"},{"type":"Selected","status":"False"},{"type":"AllInjected","status":"False"},{"type":"AllRecovered","status":"False"}]}

What's changed and how it works?

Instead of arbitrarily setting the initial values of allInjected and allRecovered, the value is assigned based on the current selected state.

Related changes

  • This change also requires further updates to the website (e.g. docs)
  • This change also requires further updates to the UI interface

Cherry-pick to release branches (optional)

This PR should be cherry-picked to the following release branches:

  • release-2.6
  • release-2.5

Checklist

CHANGELOG

Must include at least one of them.

  • I have updated the CHANGELOG.md
  • I have labeled this PR with "no-need-update-changelog"

Tests

Must include at least one of them.

  • Unit test
  • E2E test
  • Manual test

Side effects

  • Breaking backward compatibility

DCO

If you find the DCO check fails, please run commands like below (Depends on the actual situations. For example, if the failed commit isn't the most recent) to fix it:

git commit --amend --signoff
git push --force

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
Copy link
Member Author

/cherrypick release-2.6

@chaos-mesh-cherrypick-robot

@g1eny0ung: once the present PR merges, I will cherry-pick it on top of release-2.6 in a new PR and assign it to you.

In response to this:

/cherrypick release-2.6

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@codecov
Copy link

codecov bot commented Sep 16, 2023

Codecov Report

Merging #4199 (45f6f17) into master (466578e) will decrease coverage by 0.07%.
Report is 3 commits behind head on master.
The diff coverage is 70.49%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4199      +/-   ##
==========================================
- Coverage   38.43%   38.36%   -0.07%     
==========================================
  Files         167      169       +2     
  Lines       13787    13900     +113     
==========================================
+ Hits         5299     5333      +34     
- Misses       8052     8131      +79     
  Partials      436      436              
Files Coverage Δ
controllers/common/step.go 0.00% <0.00%> (ø)
controllers/common/condition/controller.go 46.23% <79.62%> (ø)

... and 3 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4e3ee29...45f6f17. 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>
@cwen0 cwen0 self-requested a review October 31, 2023 09:40
Signed-off-by: Yue Yang <g1enyy0ung@gmail.com>
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

@cwen0
Copy link
Member

cwen0 commented Oct 31, 2023

/lgtm

@chaotic-prow
Copy link

chaotic-prow bot commented Oct 31, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cwen0

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

@chaotic-prow chaotic-prow bot removed the lgtm label Nov 1, 2023
@chaotic-prow
Copy link

chaotic-prow bot commented Nov 1, 2023

New changes are detected. LGTM label has been removed.

Signed-off-by: Yue Yang <g1enyy0ung@gmail.com>
@g1eny0ung g1eny0ung merged commit c2d7fa5 into chaos-mesh:master Nov 1, 2023
54 of 56 checks passed
@g1eny0ung g1eny0ung deleted the refactor/all-injected-recovered branch November 1, 2023 03:37
@chaos-mesh-cherrypick-robot

@g1eny0ung: #4199 failed to apply on top of branch "release-2.6":

Applying: refactor: allInjected and allRecovered conditions
Applying: chore: update
Using index info to reconstruct a base tree...
M	CHANGELOG.md
Falling back to patching base and 3-way merge...
Auto-merging CHANGELOG.md
CONFLICT (content): Merge conflict in CHANGELOG.md
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0002 chore: update
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

In response to this:

/cherrypick release-2.6

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

g1eny0ung added a commit to g1eny0ung/chaos-mesh that referenced this pull request Nov 1, 2023
…h#4199)

* refactor: allInjected and allRecovered conditions

Signed-off-by: Yue Yang <g1enyy0ung@gmail.com>

* chore: update

Signed-off-by: Yue Yang <g1enyy0ung@gmail.com>

* Update CHANGELOG.md

Signed-off-by: Yue Yang <g1enyy0ung@gmail.com>

* chore: add tests

Signed-off-by: Yue Yang <g1enyy0ung@gmail.com>

* Update condition_suite_test.go

Signed-off-by: Yue Yang <g1enyy0ung@gmail.com>

* Update controller_test.go

Signed-off-by: Yue Yang <g1enyy0ung@gmail.com>

* fix: remove unnecessary else

Signed-off-by: Yue Yang <g1enyy0ung@gmail.com>

* chore: add comments

Signed-off-by: Yue Yang <g1enyy0ung@gmail.com>

* Update ci.yml

Signed-off-by: Yue Yang <g1enyy0ung@gmail.com>

---------

Signed-off-by: Yue Yang <g1enyy0ung@gmail.com>
g1eny0ung added a commit that referenced this pull request Nov 1, 2023
…4243)

Signed-off-by: Yue Yang <g1enyy0ung@gmail.com>
wilcoweb pushed a commit to raymyers/chaos-mesh that referenced this pull request Nov 30, 2023
…h#4199)

* refactor: allInjected and allRecovered conditions

Signed-off-by: Yue Yang <g1enyy0ung@gmail.com>

* chore: update

Signed-off-by: Yue Yang <g1enyy0ung@gmail.com>

* Update CHANGELOG.md

Signed-off-by: Yue Yang <g1enyy0ung@gmail.com>

* chore: add tests

Signed-off-by: Yue Yang <g1enyy0ung@gmail.com>

* Update condition_suite_test.go

Signed-off-by: Yue Yang <g1enyy0ung@gmail.com>

* Update controller_test.go

Signed-off-by: Yue Yang <g1enyy0ung@gmail.com>

* fix: remove unnecessary else

Signed-off-by: Yue Yang <g1enyy0ung@gmail.com>

* chore: add comments

Signed-off-by: Yue Yang <g1enyy0ung@gmail.com>

* Update ci.yml

Signed-off-by: Yue Yang <g1enyy0ung@gmail.com>

---------

Signed-off-by: Yue Yang <g1enyy0ung@gmail.com>
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

3 participants