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

fix(trafficrouting): apply stable selectors on canary service on rollout abort #2781 #2818

Merged

Conversation

meeech
Copy link
Contributor

@meeech meeech commented May 31, 2023

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 with a list of types and scopes found here, (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.
  • [n/a] My organization is added to USERS.md.

@meeech
Copy link
Contributor Author

meeech commented May 31, 2023

waiting for tests to run, want to see what fails, then will look into how to make a test to cover this.

@github-actions
Copy link
Contributor

github-actions bot commented May 31, 2023

Go Published Test Results

1 990 tests   1 990 ✔️  2m 35s ⏱️
   118 suites         0 💤
       1 files           0

Results for commit 9ffec5e.

♻️ This comment has been updated with latest results.

@github-actions
Copy link
Contributor

github-actions bot commented May 31, 2023

E2E Tests Published Test Results

    4 files      4 suites   3h 30m 58s ⏱️
  97 tests   84 ✔️   5 💤   8
408 runs  368 ✔️ 20 💤 20

For more details on these failures, see this check.

Results for commit 9ffec5e.

♻️ This comment has been updated with latest results.

@meeech meeech force-pushed the AR-2781-re-apply-old-canary-selector branch from 944b0e5 to 0b2cc3a Compare June 2, 2023 01:56
@codecov
Copy link

codecov bot commented Jun 2, 2023

Codecov Report

Patch coverage: 50.00% and project coverage change: +0.01 🎉

Comparison is base (bf6fb11) 81.66% compared to head (9ffec5e) 81.67%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2818      +/-   ##
==========================================
+ Coverage   81.66%   81.67%   +0.01%     
==========================================
  Files         133      133              
  Lines       20192    20193       +1     
==========================================
+ Hits        16490    16493       +3     
+ Misses       2849     2847       -2     
  Partials      853      853              
Impacted Files Coverage Δ
rollout/trafficrouting.go 73.83% <ø> (+1.29%) ⬆️
rollout/service.go 78.18% <50.00%> (-0.80%) ⬇️

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

@meeech meeech force-pushed the AR-2781-re-apply-old-canary-selector branch 2 times, most recently from 45bb421 to 9726f7c Compare June 12, 2023 13:14
@meeech meeech force-pushed the AR-2781-re-apply-old-canary-selector branch 3 times, most recently from 286bd1e to 97e4731 Compare June 21, 2023 22:00
@meeech meeech force-pushed the AR-2781-re-apply-old-canary-selector branch from a7db314 to 3aa470d Compare June 29, 2023 01:22
@meeech
Copy link
Contributor Author

meeech commented Jun 29, 2023

fwiw: all tests passed on my second run locally

=== Skipped
=== SKIP: test/e2e TestAPISIXSuite (0.06s)

=== SKIP: test/e2e TestAppMeshSuite (0.01s)

=== SKIP: test/e2e TestAWSSuite/TestALBBlueGreenUpdate (0.06s)
time="2023-06-28T23:34:02-04:00" level=info msg="Deleting e2e-test-name=TestALBBlueGreenUpdate"
time="2023-06-28T23:34:02-04:00" level=info msg="Deleting e2e-test-name=TestALBBlueGreenUpdate"
    --- SKIP: TestAWSSuite/TestALBBlueGreenUpdate (0.06s)

=== SKIP: test/e2e TestAWSSuite/TestALBCanaryUpdate (0.06s)
time="2023-06-28T23:34:02-04:00" level=info msg="Deleting e2e-test-name=TestALBCanaryUpdate"
time="2023-06-28T23:34:02-04:00" level=info msg="Deleting e2e-test-name=TestALBCanaryUpdate"
    --- SKIP: TestAWSSuite/TestALBCanaryUpdate (0.06s)

=== SKIP: test/e2e TestHeaderRoutingSuite (0.00s)

=== SKIP: test/e2e TestIstioSuite (0.00s)

=== SKIP: test/e2e TestMirrorRouteSuite (0.00s)

=== SKIP: test/e2e TestSMIIngressSuite (0.01s)

=== SKIP: test/e2e TestSMISuite (0.01s)

DONE 84 tests, 9 skipped in 1324.813s

…ting is used

fixes argoproj#2781

* remove redundant block of code

Signed-off-by: mitchell amihod <[email protected]>
Basically a copy/pasta of test that was checking for this when using traffic routing

Signed-off-by: mitchell amihod <[email protected]>
@meeech meeech force-pushed the AR-2781-re-apply-old-canary-selector branch from aefc7ee to 9ffec5e Compare June 29, 2023 22:30
@sonarcloud
Copy link

sonarcloud bot commented Jun 29, 2023

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
17.8% 17.8% Duplication

@zachaller zachaller added this to the v1.6 milestone Jul 10, 2023
@zachaller zachaller merged commit 3c5ac36 into argoproj:master Jul 11, 2023
agaudreault added a commit to agaudreault/argo-rollouts that referenced this pull request Mar 22, 2024
… on rollout abort argoproj#2781 (argoproj#2818)"

This reverts commit 3c5ac36.

Signed-off-by: Alexandre Gaudreault <[email protected]>
zachaller added a commit to zachaller/argo-rollouts that referenced this pull request Mar 25, 2024
zachaller added a commit to zachaller/argo-rollouts that referenced this pull request Jun 14, 2024
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.

2 participants