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: Implement Issue #1779: Add MinReplicas option to SetCanaryScale #1938

Closed

Conversation

ssanders1449
Copy link
Contributor

Implements issue #1779
See #1779 for Use Cases

@ssanders1449 ssanders1449 changed the title Issue 1779: Add MinReplicas option to SetCanaryScale feat: Implement Issue 1779: Add MinReplicas option to SetCanaryScale Mar 28, 2022
@ssanders1449 ssanders1449 changed the title feat: Implement Issue 1779: Add MinReplicas option to SetCanaryScale feat: Implement Issue #1779: Add MinReplicas option to SetCanaryScale Mar 28, 2022
@codecov
Copy link

codecov bot commented Mar 28, 2022

Codecov Report

Base: 81.55% // Head: 81.55% // Decreases project coverage by -0.00% ⚠️

Coverage data is based on head (edb6dca) compared to base (93c2520).
Patch coverage: 88.88% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1938      +/-   ##
==========================================
- Coverage   81.55%   81.55%   -0.01%     
==========================================
  Files         124      124              
  Lines       18931    18947      +16     
==========================================
+ Hits        15439    15452      +13     
- Misses       2702     2703       +1     
- Partials      790      792       +2     
Impacted Files Coverage Δ
utils/rollout/rolloututil.go 90.15% <0.00%> (-2.16%) ⬇️
utils/replicaset/canary.go 92.95% <100.00%> (+0.22%) ⬆️

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

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

@ssanders1449 ssanders1449 force-pushed the issue_1779_minReplicas branch from 37e5b45 to 93c7595 Compare March 28, 2022 17:28
@sonarcloud
Copy link

sonarcloud bot commented Apr 27, 2022

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 0 Code Smells

No Coverage information No Coverage information
3.5% 3.5% Duplication

Shlomo Sanders added 2 commits June 8, 2022 17:46
@ssanders1449 ssanders1449 force-pushed the issue_1779_minReplicas branch from dcfcd9e to eab0416 Compare June 8, 2022 15:11
Shlomo Sanders added 2 commits June 8, 2022 18:22
@sonarcloud
Copy link

sonarcloud bot commented Jun 9, 2022

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 0 Code Smells

No Coverage information No Coverage information
3.5% 3.5% Duplication

@ssanders1449
Copy link
Contributor Author

@jessesuen - can you please review this? We need this in order to ensure high availability when using TrafficRoutedCanary

@nissy34 - FYI

@github-actions
Copy link
Contributor

This PR is stale because it has been open 90 days with no activity.

@ssanders1449
Copy link
Contributor Author

ssanders1449 commented Nov 3, 2022

I'm surprised that no one else has this issue. We worked around it so far, by adding a default-backend service to the nginx ingress which routes to all of pods (both stable and canary), so that if one of the replicasets has only one POD and it goes down (e.g. it is being moved to another node), the requests will get routed to the other pods via the default-backend.

However, I'm not sure that other traffic-routing options (e.g. ALB) have a similar 'default-backend' concept

@github-actions
Copy link
Contributor

github-actions bot commented Nov 8, 2022

Go Published Test Results

1 800 tests   1 800 ✔️  2m 30s ⏱️
   102 suites         0 💤
       1 files           0

Results for commit edb6dca.

♻️ This comment has been updated with latest results.

@ssanders1449 ssanders1449 force-pushed the issue_1779_minReplicas branch 2 times, most recently from f930113 to 0f7ebb0 Compare November 8, 2022 10:38
Shlomo Sanders added 2 commits November 8, 2022 13:46
Signed-off-by: Shlomo Sanders <[email protected]>
Signed-off-by: Shlomo Sanders <[email protected]>
@github-actions
Copy link
Contributor

github-actions bot commented Nov 8, 2022

E2E Tests Published Test Results

    2 files      2 suites   1h 29m 46s ⏱️
  89 tests   82 ✔️ 3 💤 4
184 runs  172 ✔️ 6 💤 6

For more details on these failures, see this check.

Results for commit edb6dca.

♻️ This comment has been updated with latest results.

@sonarcloud
Copy link

sonarcloud bot commented Nov 9, 2022

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 0 Code Smells

No Coverage information No Coverage information
3.4% 3.4% Duplication

@alexef alexef added enhancement New feature or request canary Canary related issue traffic-routing labels Nov 13, 2022
@ssanders1449
Copy link
Contributor Author

@zachaller - can you take a look at this? We are currently building from a fork, and would like this to be eventually included

@ssanders1449
Copy link
Contributor Author

Based on #1779 (comment), this can be replaced by

#2410

@ssanders1449
Copy link
Contributor Author

closing in favor of #2448

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
canary Canary related issue enhancement New feature or request traffic-routing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants