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: configurable and more aggressive cleanup of old AnalysisRuns and Experiments #1342

Merged

Conversation

perenesenko
Copy link
Member

@perenesenko perenesenko commented Jul 12, 2021

Fixes #1214

Signed-off-by: Andrii Perenesenko [email protected]

Checklist:

  • Either (a) I've created an enhancement proposal and discussed it with the community, (b) this is a bug fix, or (c) this is a chore.
  • The title of the PR is (a) conventional, (b) states what changed, and (c) suffixes the related issues number. E.g. "fix(controller): Updates such and such. Fixes #1234".
  • I've signed my commits with DCO
  • I have written unit and/or e2e tests for my change. PRs without these are unlikely to be merged.
  • My builds are green. Try syncing with master if they are not.
  • My organization is added to USERS.md.

@perenesenko perenesenko changed the title feat: More aggressive cleanup of old AnalysisRuns and Experiments feat: More aggressive cleanup of old AnalysisRuns and Experiments. Fixes #1214 Jul 12, 2021
@codecov
Copy link

codecov bot commented Jul 12, 2021

Codecov Report

Merging #1342 (01b16c6) into master (e3dc6af) will increase coverage by 0.07%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1342      +/-   ##
==========================================
+ Coverage   81.26%   81.33%   +0.07%     
==========================================
  Files         108      108              
  Lines       10033     9955      -78     
==========================================
- Hits         8153     8097      -56     
+ Misses       1319     1306      -13     
+ Partials      561      552       -9     
Impacted Files Coverage Δ
rollout/analysis.go 80.75% <100.00%> (+0.14%) ⬆️
rollout/experiment.go 85.71% <100.00%> (+0.18%) ⬆️
utils/analysis/filter.go 97.93% <100.00%> (+0.46%) ⬆️
utils/defaults/defaults.go 92.00% <100.00%> (+3.29%) ⬆️
utils/experiment/filter.go 100.00% <100.00%> (ø)
metricproviders/kayenta/kayenta.go 83.11% <0.00%> (-0.85%) ⬇️
rollout/bluegreen.go 70.91% <0.00%> (ø)
utils/replicaset/replicaset.go 89.58% <0.00%> (+0.10%) ⬆️
rollout/replicaset.go 68.42% <0.00%> (+0.89%) ⬆️
utils/replicaset/canary.go 82.56% <0.00%> (+0.91%) ⬆️
... and 4 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 e3dc6af...01b16c6. Read the comment docs.

examples/analysis-templates.yaml Show resolved Hide resolved
pkg/apis/rollouts/v1alpha1/types.go Outdated Show resolved Hide resolved
utils/analysis/filter.go Outdated Show resolved Hide resolved
utils/analysis/filter.go Outdated Show resolved Hide resolved
utils/analysis/filter.go Outdated Show resolved Hide resolved
utils/analysis/filter_test.go Show resolved Hide resolved
utils/defaults/defaults.go Outdated Show resolved Hide resolved
utils/defaults/defaults.go Outdated Show resolved Hide resolved
Copy link
Member

@huikang huikang left a comment

Choose a reason for hiding this comment

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

Some minor comments; otherwise it looks good.

rollout/analysis.go Outdated Show resolved Hide resolved
@perenesenko perenesenko force-pushed the perenesenko/1214-cleanup-analysisrun branch from 2a3f0ad to d37650d Compare July 26, 2021 18:01
Signed-off-by: Andrii Perenesenko <[email protected]>
pkg/apis/rollouts/v1alpha1/types.go Outdated Show resolved Hide resolved
docs/features/specification.md Outdated Show resolved Hide resolved
pkg/apis/rollouts/v1alpha1/types.go Outdated Show resolved Hide resolved
…view fix: field renaming

Signed-off-by: Andrii Perenesenko <[email protected]>
…view fix: field renaming

Signed-off-by: Andrii Perenesenko <[email protected]>
…view fix: field renaming

Signed-off-by: Andrii Perenesenko <[email protected]>
…name Analysis => AnalysisStrategy

Signed-off-by: Andrii Perenesenko <[email protected]>
@perenesenko
Copy link
Member Author

On standup was decided to reuse the same parameters analysis.* for the Experiments cleaning

@perenesenko
Copy link
Member Author

On a standup also discussed about the changing analysis.failed... => analysis.unsuccessful... and that parameter should includes all 3 phases: Failed, Error, Inconclusive

…e unsuccessful for 3 phases: error, failed and inconclusive

Signed-off-by: Andrii Perenesenko <[email protected]>
Copy link
Member

@huikang huikang left a comment

Choose a reason for hiding this comment

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

Just some minor questions. Thanks.

pkg/apis/rollouts/v1alpha1/types.go Outdated Show resolved Hide resolved
utils/analysis/filter.go Outdated Show resolved Hide resolved
… AnalysisStrategy=>AnalysisRunStrategy

Signed-off-by: Andrii Perenesenko <[email protected]>
@huikang
Copy link
Member

huikang commented Aug 4, 2021

LGTM

Signed-off-by: Andrii Perenesenko <[email protected]>
@sonarcloud
Copy link

sonarcloud bot commented Aug 4, 2021

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 2 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@jessesuen jessesuen changed the title feat: More aggressive cleanup of old AnalysisRuns and Experiments. Fixes #1214 feat: configurable and more aggressive cleanup of old AnalysisRuns and Experiments Aug 5, 2021
@jessesuen jessesuen merged commit ffe70da into argoproj:master Aug 5, 2021
mdb pushed a commit to mdb/argo-rollouts that referenced this pull request Aug 10, 2021
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.

More aggressive cleanup of old AnalysisRuns and Experiments
3 participants