chore: Add exception to `requireCanaryStableServices` to disable validation when using the `hashicorp/consul` plugin #3339

wilkermichael · 2024-01-30T20:46:02Z

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.
My organization is added to USERS.md.

github-actions · 2024-01-30T20:54:49Z

Go Published Test Results

2 124 tests 2 124 ✅ 2m 50s ⏱️
118 suites 0 💤
1 files 0 ❌

Results for commit 74cf023.

♻️ This comment has been updated with latest results.

codecov · 2024-01-30T20:55:16Z

Codecov Report

Attention: 69 lines in your changes are missing coverage. Please review.

Comparison is base (8405f2e) 81.83% compared to head (74cf023) 82.86%.
Report is 9 commits behind head on master.

Files	Patch %	Lines
controller/controller.go	21.68%	61 Missing and 4 partials ⚠️
utils/record/record.go	50.00%	3 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3339      +/-   ##
==========================================
+ Coverage   81.83%   82.86%   +1.02%     
==========================================
  Files         135      135              
  Lines       20688    16845    -3843     
==========================================
- Hits        16931    13959    -2972     
+ Misses       2883     2012     -871     
  Partials      874      874

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

github-actions · 2024-01-30T21:18:47Z

E2E Tests Published Test Results

4 files 4 suites 3h 31m 5s ⏱️
106 tests 96 ✅ 6 💤 4 ❌
428 runs 400 ✅ 24 💤 4 ❌

For more details on these failures, see this check.

Results for commit 74cf023.

♻️ This comment has been updated with latest results.

david-yu · 2024-02-09T03:19:05Z

Hi @zachaller friendly nudge, if you're able to review this PR for Consul Service Mesh.

zachaller · 2024-02-12T14:51:14Z

pkg/apis/rollouts/validation/validation_test.go

@@ -129,6 +129,80 @@ func TestValidateRolloutStrategyBlueGreen(t *testing.T) {
 	assert.Equal(t, ScaleDownLimitLargerThanRevisionLimit, allErrs[1].Detail)
 }

+func TestValidateRolloutStrategyCanaryMissingServiceNames(t *testing.T) {


Can we add a few test that use two traffic routers one where both are required to have canary service and one where it one requires it and one does not.

Added some test cases with multiple routers where one requires the canary services and one or more do not to ensure they error as expected.

…when using the `hashicorp/consul` plugin Co-authored-by: Ashwin Venkatesh <[email protected]> Signed-off-by: Michael Wilkerson <[email protected]>

Co-authored-by: Ashwin Venkatesh <[email protected]> Signed-off-by: Michael Wilkerson <[email protected]>

- added test cases for exceptions to requiring checks for Canary/Stable services - added test cases for verifying errors when Canary/Stable services are missing Co-authored-by: Ashwin Venkatesh <[email protected]> Signed-off-by: Michael Wilkerson <[email protected]>

Co-authored-by: Ashwin Venkatesh <[email protected]> Signed-off-by: Michael Wilkerson <[email protected]>

- added test cases for exceptions to requiring checks for Canary/Stable services - added test cases for verifying errors when Canary/Stable services are missing Co-authored-by: Ashwin Venkatesh <[email protected]> Signed-off-by: Michael Wilkerson <[email protected]>

sonarcloud · 2024-02-12T17:06:07Z

Quality Gate passed

Issues
0 New issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

thisisnotashwin · 2024-02-12T17:43:33Z

Thanks so much for helping with this PR @zachaller

zachaller changed the title ~~Add exception to requireCanaryStableServices to disable validation when using the hashicorp/consul plugin~~ chore: Add exception to requireCanaryStableServices to disable validation when using the hashicorp/consul plugin Jan 30, 2024

wilkermichael force-pushed the master branch 2 times, most recently from 28a3cbe to d0287fd Compare February 1, 2024 21:44

wilkermichael marked this pull request as ready for review February 5, 2024 19:11

wilkermichael force-pushed the master branch 5 times, most recently from 25eee48 to 647ca89 Compare February 6, 2024 16:08

zachaller added this to the v1.7 milestone Feb 9, 2024

zachaller reviewed Feb 12, 2024

View reviewed changes

wilkermichael and others added 3 commits February 12, 2024 08:29

add exception to requireCanaryStableServices to disable validation …

7836d38

…when using the `hashicorp/consul` plugin Co-authored-by: Ashwin Venkatesh <[email protected]> Signed-off-by: Michael Wilkerson <[email protected]>

added HashiCorp to USERS.md

bb6d86e

Co-authored-by: Ashwin Venkatesh <[email protected]> Signed-off-by: Michael Wilkerson <[email protected]>

wilkermichael force-pushed the master branch from 647ca89 to 06a85d1 Compare February 12, 2024 16:29

addressed comments and added more test cases

36c169b

Co-authored-by: Ashwin Venkatesh <[email protected]> Signed-off-by: Michael Wilkerson <[email protected]>

wilkermichael force-pushed the master branch from 59571fb to 36c169b Compare February 12, 2024 16:41

wilkermichael requested a review from zachaller February 12, 2024 17:18

zachaller approved these changes Feb 12, 2024

View reviewed changes

zachaller enabled auto-merge (squash) February 12, 2024 17:22

zachaller disabled auto-merge February 12, 2024 17:23

zachaller merged commit 724d53d into argoproj:master Feb 12, 2024
22 of 23 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: Add exception to `requireCanaryStableServices` to disable validation when using the `hashicorp/consul` plugin #3339

chore: Add exception to `requireCanaryStableServices` to disable validation when using the `hashicorp/consul` plugin #3339

wilkermichael commented Jan 30, 2024 •

edited

Loading

github-actions bot commented Jan 30, 2024 •

edited

Loading

codecov bot commented Jan 30, 2024 •

edited

Loading

github-actions bot commented Jan 30, 2024 •

edited

Loading

david-yu commented Feb 9, 2024

zachaller Feb 12, 2024

thisisnotashwin Feb 12, 2024

sonarcloud bot commented Feb 12, 2024

thisisnotashwin commented Feb 12, 2024

chore: Add exception to requireCanaryStableServices to disable validation when using the hashicorp/consul plugin #3339

chore: Add exception to requireCanaryStableServices to disable validation when using the hashicorp/consul plugin #3339

Conversation

wilkermichael commented Jan 30, 2024 • edited Loading

github-actions bot commented Jan 30, 2024 • edited Loading

Go Published Test Results

codecov bot commented Jan 30, 2024 • edited Loading

Codecov Report

github-actions bot commented Jan 30, 2024 • edited Loading

E2E Tests Published Test Results

david-yu commented Feb 9, 2024

zachaller Feb 12, 2024

Choose a reason for hiding this comment

thisisnotashwin Feb 12, 2024

Choose a reason for hiding this comment

sonarcloud bot commented Feb 12, 2024

Quality Gate passed

thisisnotashwin commented Feb 12, 2024

chore: Add exception to `requireCanaryStableServices` to disable validation when using the `hashicorp/consul` plugin #3339

chore: Add exception to `requireCanaryStableServices` to disable validation when using the `hashicorp/consul` plugin #3339

wilkermichael commented Jan 30, 2024 •

edited

Loading

github-actions bot commented Jan 30, 2024 •

edited

Loading

codecov bot commented Jan 30, 2024 •

edited

Loading

github-actions bot commented Jan 30, 2024 •

edited

Loading