fix: Enable default triggers for argo rollouts #1689

RaviHari · 2021-12-08T14:58:56Z

Fixes: #1668

Signed-off-by: Ravi Hari [email protected]

Checklist:

[ X ] 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.
[ X ] 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".
[ X ] I've signed my commits with DCO
[ X ] I have written unit and/or e2e tests for my change. PRs without these are unlikely to be merged.
[ X ] My builds are green. Try syncing with master if they are not.
[ X ] My organization is added to USERS.md.

Signed-off-by: Ravi Hari <[email protected]>

RaviHari · 2021-12-08T17:19:03Z

Testing Done:

Logs:

INFO[2021-12-08T22:33:14+05:30] Start processing                              resource=rollout-demo/rollout-bluegreen
INFO[2021-12-08T22:33:14+05:30] Trigger on-rollout-completed result: [{[0].xxxxx  [rollout-completed] false}]  resource=rollout-demo/rollout-bluegreen
INFO[2021-12-08T22:33:14+05:30] Trigger on-scaling-replica-set result: [{[0].xxxxx  [scaling-replicaset] false}]  resource=rollout-demo/rollout-bluegreen

apiVersion: v1
kind: ConfigMap
metadata:
  name: argo-rollouts-notification-configmap
  namespace: argo-rollouts
data:
  service.slack: |
    token: $slack-token
    icon: https://avatars.githubusercontent.com/u/30269780?s=400&v=4
  template.rollout-completed: |
    message: Rollout {{.rollout.metadata.name}} has been completed.
    slack:
      attachments: |
          [{
            "title": "{{ .rollout.metadata.name}}",
            "color": "#18be52",
            "fields": [
            {
              "title": "Strategy",
              "value": "{{if .rollout.spec.strategy.blueGreen}}BlueGreen{{end}}{{if .rollout.spec.strategy.canary}}Canary{{end}}",
              "short": true
            }
            {{range $index, $c := .rollout.spec.template.spec.containers}}
              {{if not $index}},{{end}}
              {{if $index}},{{end}}
              {
                "title": "{{$c.name}}",
                "value": "{{$c.image}}",
                "short": true
              }
            {{end}}
            ]
          }]
  template.rollout-step-completed: |
    message: Rollout {{.rollout.metadata.name}} step number {{ add .rollout.status.currentStepIndex 1}}/{{len .rollout.spec.strategy.canary.steps}} has been completed.
    email:
      subject: Rollout {{.rollout.metadata.name}} step number {{ add .rollout.status.currentStepIndex 1}}/{{len .rollout.spec.strategy.canary.steps}} has been completed.
    slack:
      attachments: |
          [{
            "title": "{{ .rollout.metadata.name}}",
            "color": "#18be52",
            "fields": [
            {
              "title": "Strategy",
              "value": "{{if .rollout.spec.strategy.blueGreen}}BlueGreen{{end}}{{if .rollout.spec.strategy.canary}}Canary{{end}}",
              "short": true
            },
            {
              "title": "Step completed",
              "value": "{{add .rollout.status.currentStepIndex 1}}/{{len .rollout.spec.strategy.canary.steps}}",
              "short": true
            }
            {{range $index, $c := .rollout.spec.template.spec.containers}}
              {{if not $index}},{{end}}
              {{if $index}},{{end}}
              {
                "title": "{{$c.name}}",
                "value": "{{$c.image}}",
                "short": true
              }
            {{end}}
            ]
          }]
  template.rollout-updated: |
    message: Rollout {{.rollout.metadata.name}} has been updated.
    email:
      subject: Rollout {{.rollout.metadata.name}} has been updated.
    slack:
      attachments: |
          [{
            "title": "{{ .rollout.metadata.name}}",
            "color": "#18be52",
            "fields": [
            {
              "title": "Strategy",
              "value": "{{if .rollout.spec.strategy.blueGreen}}BlueGreen{{end}}{{if .rollout.spec.strategy.canary}}Canary{{end}}",
              "short": true
            }
            {{range $index, $c := .rollout.spec.template.spec.containers}}
              {{if not $index}},{{end}}
              {{if $index}},{{end}}
              {
                "title": "{{$c.name}}",
                "value": "{{$c.image}}",
                "short": true
              }
            {{end}}
            ]
          }]
  template.scaling-replicaset: |
    message: Scaling Rollout {{.rollout.metadata.name}}'s replicaset to {{.rollout.spec.replicas}}.
    email:
      subject: Scaling Rollout {{.rollout.metadata.name}}'s replcaset to {{.rollout.spec.replicas}}.
    slack:
      attachments: |
          [{
            "title": "{{ .rollout.metadata.name}}",
            "color": "#18be52",
            "fields": [
            {
              "title": "Strategy",
              "value": "{{if .rollout.spec.strategy.blueGreen}}BlueGreen{{end}}{{if .rollout.spec.strategy.canary}}Canary{{end}}",
              "short": true
            },
            {
              "title": "Desired replica",
              "value": "{{.rollout.spec.replicas}}",
              "short": true
            },
            {
              "title": "Updated replicas",
              "value": "{{.rollout.status.updatedReplicas}}",
              "short": true
            }
            {{range $index, $c := .rollout.spec.template.spec.containers}}
              {{if not $index}},{{end}}
              {{if $index}},{{end}}
              {
                "title": "{{$c.name}}",
                "value": "{{$c.image}}",
                "short": true
              }
            {{end}}
            ]
          }]
  trigger.on-rollout-completed: |
    - send: [rollout-completed]
  trigger.on-rollout-step-completed: |
    - send: [rollout-step-completed]
  trigger.on-rollout-updated: |
    - send: [rollout-updated]
  trigger.on-scaling-replica-set: |
    - send: [scaling-replicaset]
  defaultTriggers: |
    - on-scaling-replica-set

slack messages:


testAPP  10:40 PM
Scaling Rollout rollout-bluegreen's replicaset to 1.
rollout-bluegreen
Strategy
BlueGreen
Desired replica
1
Updated replicas
2
rollouts-demo
argoproj/rollouts-demo:blue


New
10:40
Scaling Rollout rollout-bluegreen's replicaset to 2.
rollout-bluegreen
Strategy
BlueGreen
Desired replica
2
Updated replicas
1
rollouts-demo
argoproj/rollouts-demo:blue


10:41
Scaling Rollout rollout-bluegreen's replicaset to 3.
rollout-bluegreen
Strategy
BlueGreen
Desired replica
3
Updated replicas
2
rollouts-demo
argoproj/rollouts-demo:blue

Signed-off-by: Ravi Hari <[email protected]>

utils/record/record.go

Signed-off-by: Ravi Hari <[email protected]>

utils/record/record.go

Signed-off-by: Ravi Hari <[email protected]>

jessesuen · 2021-12-09T17:42:55Z

@khhirani do you mind taking a look at this?

Signed-off-by: Ravi Hari <[email protected]>

codecov · 2021-12-11T18:49:00Z

Codecov Report

Merging #1689 (0457797) into master (4d3b992) will decrease coverage by 0.04%.
The diff coverage is 78.04%.

@@            Coverage Diff             @@
##           master    #1689      +/-   ##
==========================================
- Coverage   82.07%   82.02%   -0.05%     
==========================================
  Files         116      116              
  Lines       16064    16096      +32     
==========================================
+ Hits        13184    13203      +19     
- Misses       2208     2218      +10     
- Partials      672      675       +3

Impacted Files	Coverage Δ
utils/record/record.go	`86.75% <78.04%> (-4.08%)`	⬇️
rollout/trafficrouting/alb/alb.go	`82.26% <0.00%> (-2.69%)`	⬇️
rollout/trafficrouting/istio/controller.go	`50.81% <0.00%> (-1.63%)`	⬇️
utils/analysis/helpers.go	`77.93% <0.00%> (-0.25%)`	⬇️
rollout/pause.go	`95.33% <0.00%> (ø)`
rollout/restart.go	`98.64% <0.00%> (ø)`
rollout/replicaset.go	`67.59% <0.00%> (ø)`
analysis/controller.go	`52.17% <0.00%> (ø)`
utils/metric/metric.go	`100.00% <0.00%> (ø)`
utils/ingress/ingress.go	`100.00% <0.00%> (ø)`
... and 18 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 4d3b992...0457797. Read the comment docs.

Signed-off-by: Ravi Hari <[email protected]>

RaviHari · 2021-12-13T07:52:06Z

I noticed that some times this test-e2e fails on test case: TestIstioSuite/TestIstioAbortUpdateDeleteAllCanaryPods as given here: https://github.com/argoproj/argo-rollouts/runs/4494296112?check_suite_focus=true I think this is primarily due to a timeout not a functional defect.

time="2021-12-11T19:40:32Z" level=info msg="Waiting for condition: status=Degraded" rollout=istio-host-split
    when.go:440: timeout after 1m0s waiting for condition status=Degraded

When I run this test locally it works fine:

 make test-e2e E2E_TEST_OPTIONS="-testify.m TestIstioAbortUpdateDeleteAllCanaryPods"   
go test -timeout 30m -v -count 1 --tags e2e -p 4 --short ./test/e2e -testify.m TestIstioAbortUpdateDeleteAllCanaryPods
=== RUN   TestAnalysisSuite
    suite.go:190: warning: no tests to run
--- PASS: TestAnalysisSuite (0.00s)
=== RUN   TestAWSSuite
    suite.go:190: warning: no tests to run
--- PASS: TestAWSSuite (0.00s)
=== RUN   TestBlueGreenSuite
    suite.go:190: warning: no tests to run
--- PASS: TestBlueGreenSuite (0.00s)
=== RUN   TestCanarySuite
    suite.go:190: warning: no tests to run
--- PASS: TestCanarySuite (0.00s)
=== RUN   TestExperimentSuite
    suite.go:190: warning: no tests to run
--- PASS: TestExperimentSuite (0.00s)
=== RUN   TestFunctionalSuite
    suite.go:190: warning: no tests to run
--- PASS: TestFunctionalSuite (0.00s)
=== RUN   TestIstioSuite
=== RUN   TestIstioSuite/TestIstioAbortUpdateDeleteAllCanaryPods
time="2021-12-13T13:13:40+05:30" level=info msg="Deleting e2e-test-name=TestIstioAbortUpdateDeleteAllCanaryPods"
time="2021-12-13T13:13:43+05:30" level=info msg="service/istio-host-split-canary created\n" rollout=istio-host-split
time="2021-12-13T13:13:43+05:30" level=info msg="service/istio-host-split-stable created\n" rollout=istio-host-split
time="2021-12-13T13:13:43+05:30" level=info msg="virtualservice.networking.istio.io/istio-host-split-vsvc created\n" rollout=istio-host-split
time="2021-12-13T13:13:43+05:30" level=info msg="rollout.argoproj.io/istio-host-split created\n" rollout=istio-host-split
time="2021-12-13T13:13:43+05:30" level=info msg="Waiting for condition: status=Healthy" rollout=istio-host-split
time="2021-12-13T13:13:54+05:30" level=info msg="Condition 'status=Healthy' met after 10s" rollout=istio-host-split
time="2021-12-13T13:13:54+05:30" level=info msg="Updated rollout pod spec: 2021-12-13T13:13:54.128467+05:30" rollout=istio-host-split
time="2021-12-13T13:13:54+05:30" level=info msg="Waiting for condition: status=Paused" rollout=istio-host-split
time="2021-12-13T13:13:59+05:30" level=info msg="Condition 'status=Paused' met after 5s" rollout=istio-host-split
time="2021-12-13T13:13:59+05:30" level=info msg="Pod expectation 'revision:2 pod count == 2' met" rollout=istio-host-split
time="2021-12-13T13:13:59+05:30" level=info msg="Promoted rollout" rollout=istio-host-split
time="2021-12-13T13:13:59+05:30" level=info msg="Waiting for condition: status=Paused" rollout=istio-host-split
time="2021-12-13T13:13:59+05:30" level=info msg="Condition 'status=Paused' met after 0s" rollout=istio-host-split
time="2021-12-13T13:13:59+05:30" level=info msg="Promoted rollout" rollout=istio-host-split
time="2021-12-13T13:13:59+05:30" level=info msg="Waiting for condition: status=Paused" rollout=istio-host-split
time="2021-12-13T13:14:04+05:30" level=info msg="Condition 'status=Paused' met after 4s" rollout=istio-host-split
time="2021-12-13T13:14:04+05:30" level=info msg="Pod expectation 'revision:2 pod count == 4' met" rollout=istio-host-split
time="2021-12-13T13:14:04+05:30" level=info msg="Aborted rollout" rollout=istio-host-split
time="2021-12-13T13:14:04+05:30" level=info msg="Waiting for condition: status=Degraded" rollout=istio-host-split
time="2021-12-13T13:14:04+05:30" level=info msg="Condition 'status=Degraded' met after 0s" rollout=istio-host-split
time="2021-12-13T13:14:04+05:30" level=info msg="Pod expectation 'revision:1 pod count == 5' met" rollout=istio-host-split
time="2021-12-13T13:14:04+05:30" level=info msg="Pod expectation 'revision:2 pod count == 4' met" rollout=istio-host-split
time="2021-12-13T13:14:04+05:30" level=info msg="rs expectation 'RS istio-host-split-b8d98f5fd scale down' met" rollout=istio-host-split
time="2021-12-13T13:14:04+05:30" level=info msg="Deleting e2e-test-name=TestIstioAbortUpdateDeleteAllCanaryPods"
time="2021-12-13T13:14:04+05:30" level=info msg="Deleting e2e-test-name"
--- PASS: TestIstioSuite (24.76s)
    --- PASS: TestIstioSuite/TestIstioAbortUpdateDeleteAllCanaryPods (24.49s)
=== RUN   TestSMIIngressSuite
    suite.go:190: warning: no tests to run
--- PASS: TestSMIIngressSuite (0.00s)
=== RUN   TestSMISuite
    suite.go:190: warning: no tests to run
--- PASS: TestSMISuite (0.00s)
PASS
ok      github.com/argoproj/argo-rollouts/test/e2e      25.688s

jessesuen · 2021-12-13T09:33:10Z

I think this is primarily due to a timeout not a functional defect.

Yes tests are flaky right now. I'll resubmit it when I think/notice it is the case.

harikrongali · 2021-12-13T18:31:33Z

@RaviHari let me validate and come back. quick question, have you applied https://github.com/argoproj/argo-rollouts/blob/master/manifests/notifications-install.yaml ?

RaviHari · 2021-12-14T01:37:14Z

@RaviHari let me validate and come back. quick question, have you applied https://github.com/argoproj/argo-rollouts/blob/master/manifests/notifications-install.yaml ?

@harikrongali ,
Thanks for loking into it.. In my local test yes I applied.. but the E2E tests now fail for a different usecase.. FAIL: TestFunctionalSuite/TestBlueGreenUpdate which passed in earlier runs.

jessesuen

LGTM!

utils/record/record.go

…ut has no subscribers Signed-off-by: Ravi Hari <[email protected]>

Signed-off-by: Ravi Hari <[email protected]>

sonarcloud · 2021-12-15T17:57:28Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
7 Code Smells

No Coverage information
5.3% Duplication

alexmt

LGTM

enable default trigerres for argo rollouts

c7d855f

Signed-off-by: Ravi Hari <[email protected]>

RaviHari changed the title ~~fix: enable default trigerres for argo rollouts~~ fix: Enable default trigerres for argo rollouts Dec 8, 2021

RaviHari added 3 commits December 8, 2021 22:26

fix: enable default trigerres for argo rollouts

eb99e51

Signed-off-by: Ravi Hari <[email protected]>

fix: enable default trigers

56e8a53

Signed-off-by: Ravi Hari <[email protected]>

fix: enable default trigers.. use api alias for notificaiton api

656c93d

Signed-off-by: Ravi Hari <[email protected]>

fix: enable default trigers.. remove comments

c67697b

Signed-off-by: Ravi Hari <[email protected]>

huikang reviewed Dec 8, 2021

View reviewed changes

utils/record/record.go Outdated Show resolved Hide resolved

utils/record/record.go Outdated Show resolved Hide resolved

utils/record/record.go Show resolved Hide resolved

fix: enable default trigers.. remove commented import

04c004a

Signed-off-by: Ravi Hari <[email protected]>

huikang reviewed Dec 8, 2021

View reviewed changes

utils/record/record.go Outdated Show resolved Hide resolved

utils/record/record.go Show resolved Hide resolved

fix: enable default trigers.. use cfg.Triggers

6a915f9

Signed-off-by: Ravi Hari <[email protected]>

RaviHari changed the title ~~fix: Enable default trigerres for argo rollouts~~ fix: Enable default triggers for argo rollouts Dec 9, 2021

RaviHari requested a review from huikang December 9, 2021 15:11

jessesuen requested a review from khhirani December 9, 2021 17:43

RaviHari added 2 commits December 10, 2021 22:18

fix: linting

28d487b

Signed-off-by: Ravi Hari <[email protected]>

fix: Update TestRequeueAfter.. tests

9e8f22e

Signed-off-by: Ravi Hari <[email protected]>

RaviHari added 3 commits December 12, 2021 00:21

fix: revert changes to notification docs

c9ea845

Signed-off-by: Ravi Hari <[email protected]>

fix: update notification docs

05949f0

Signed-off-by: Ravi Hari <[email protected]>

fix: update mkdocs to include latest notification channels

31269fa

Signed-off-by: Ravi Hari <[email protected]>

harikrongali requested review from harikrongali and removed request for khhirani December 13, 2021 18:30

jessesuen approved these changes Dec 14, 2021

View reviewed changes

alexmt requested changes Dec 14, 2021

View reviewed changes

utils/record/record.go Show resolved Hide resolved

fix: don't return error if notifications are not configured and rollo…

e97f0d4

…ut has no subscribers Signed-off-by: Ravi Hari <[email protected]>

fix: linting

0457797

Signed-off-by: Ravi Hari <[email protected]>

alexmt approved these changes Dec 15, 2021

View reviewed changes

alexmt merged commit 5b140c9 into argoproj:master Dec 15, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Enable default triggers for argo rollouts #1689

fix: Enable default triggers for argo rollouts #1689

RaviHari commented Dec 8, 2021 •

edited

Loading

RaviHari commented Dec 8, 2021

jessesuen commented Dec 9, 2021

codecov bot commented Dec 11, 2021 •

edited

Loading

RaviHari commented Dec 13, 2021 •

edited

Loading

jessesuen commented Dec 13, 2021

harikrongali commented Dec 13, 2021

RaviHari commented Dec 14, 2021

jessesuen left a comment

sonarcloud bot commented Dec 15, 2021

alexmt left a comment

fix: Enable default triggers for argo rollouts #1689

fix: Enable default triggers for argo rollouts #1689

Conversation

RaviHari commented Dec 8, 2021 • edited Loading

RaviHari commented Dec 8, 2021

jessesuen commented Dec 9, 2021

codecov bot commented Dec 11, 2021 • edited Loading

Codecov Report

RaviHari commented Dec 13, 2021 • edited Loading

jessesuen commented Dec 13, 2021

harikrongali commented Dec 13, 2021

RaviHari commented Dec 14, 2021

jessesuen left a comment

Choose a reason for hiding this comment

sonarcloud bot commented Dec 15, 2021

alexmt left a comment

Choose a reason for hiding this comment

RaviHari commented Dec 8, 2021 •

edited

Loading

codecov bot commented Dec 11, 2021 •

edited

Loading

RaviHari commented Dec 13, 2021 •

edited

Loading