Skip to content

Tests reproing 503s (was 404s) during routerule apply - with t.Skip() until we have fix#1041

Merged
ldemailly merged 57 commits intomasterfrom
issue_1038_aka_404s
Apr 20, 2018
Merged

Tests reproing 503s (was 404s) during routerule apply - with t.Skip() until we have fix#1041
ldemailly merged 57 commits intomasterfrom
issue_1038_aka_404s

Conversation

@ldemailly
Copy link
Copy Markdown
Member

@ldemailly ldemailly commented Oct 6, 2017 •
edited
Loading

Test for issue #1038, errors during routingrule application

Test for issue #1038, errors during routingrule application

@ldemailly
wip
3437a9e 
Somehow stepped on a routerules.config.istio.io
Issue - will make a PR to see if it shows up elsewhere than my local
cluster


Former-commit-id: 98a33aa
@ldemailly
Copy link
Copy Markdown
Member Author

ldemailly commented Oct 6, 2017 •
edited
Loading

I had a left over old crd so I was getting errors

 the server could not find the requested resource (post routerules.config.istio.io)
, err: exit status 1
# and pilot in crash loop with
W1006 01:30:58.604835       1 client.go:194] name conflict: ListKindConflict
I1006 01:30:58.604867       1 client.go:198] missing status condition for "routerules.config.istio.io"

fixed by deleting all crds (had route-rules.config.istio.io)

@ldemailly ldemailly changed the title [wip] new 404 test [wip] Test reproing 404s (and 503s) during routerule apply Oct 6, 2017
@ldemailly ldemailly changed the title [wip] Test reproing 404s (and 503s) during routerule apply Test reproing 404s (and 503s) during routerule apply [need a fix to be mergeable] Oct 6, 2017
@ldemailly ldemailly changed the title Test reproing 404s (and 503s) during routerule apply [need a fix to be mergeable] Test reproing 404s (and 503s) during routerule apply Oct 6, 2017
@ldemailly
Copy link
Copy Markdown
Member Author

ldemailly commented Oct 6, 2017 •
edited
Loading

This fails, finding 404s and 503s, the 404s need fixing probably using this and the 503s probably some other check for "no match" ?

19:47:20 W http.go:540> Parsed non ok code 404 (HTTP/1.1 404)
19:47:20 W http.go:540> Parsed non ok code 404 (HTTP/1.1 404)
19:47:20 W http.go:540> Parsed non ok code 404 (HTTP/1.1 404)
19:47:24 W http.go:540> Parsed non ok code 503 (HTTP/1.1 503)
19:47:24 W http.go:540> Parsed non ok code 404 (HTTP/1.1 404)
19:47:24 W http.go:540> Parsed non ok code 404 (HTTP/1.1 404)
19:47:24 W http.go:540> Parsed non ok code 503 (HTTP/1.1 503)
19:47:24 W http.go:540> Parsed non ok code 503 (HTTP/1.1 503)
...
Code 200 : 135
Code 404 : 6
Code 503 : 19

@istio istio deleted a comment from istio-testing Oct 6, 2017
@ldemailly ldemailly mentioned this pull request Oct 6, 2017
Closed
@ldemailly
Copy link
Copy Markdown
Member Author

ldemailly commented Oct 6, 2017

somehow on prow it only got 503s and no 404, probably need to make changes more than once to trigger it reliably, on my personal cluster it happens more often

/retest

@ldemailly ldemailly mentioned this pull request Oct 6, 2017
Closed
@ldemailly
Copy link
Copy Markdown
Member Author

ldemailly commented Oct 6, 2017

to repro/see:

git checkout issue_1038_aka_404s
bazel build //tests/e2e/tests/simple:go_default_test
# first time:
./bazel-bin/tests/e2e/tests/simple/go_default_test -alsologtostderr -test.v -v 2  --skip_cleanup --namespace=e2e -istioctl  ~/tmp/istio-0.2.7/bin/istioctl --auth_enable --test.run  Test404sDuringChanges # will create the e2e namespace and install istio init, may get unrelated failue
# Subsequent:
./bazel-bin/tests/e2e/tests/simple/go_default_test -alsologtostderr -test.v -v 2  --skip_cleanup --skip_setup --namespace=e2e -istioctl  ~/tmp/istio-0.2.7/bin/istioctl --auth_enable --test.run  Test404sDuringChanges
# output:
14:23:24 W http.go:540> Parsed non ok code 404 (HTTP/1.1 404)
14:23:24 W http.go:540> Parsed non ok code 404 (HTTP/1.1 404)
14:23:24 W http.go:540> Parsed non ok code 404 (HTTP/1.1 404)
14:23:24 W http.go:540> Parsed non ok code 404 (HTTP/1.1 404)
14:23:24 W http.go:540> Parsed non ok code 404 (HTTP/1.1 404)
[...]
	simple1_test.go:191: Not all 146 requests were successful (map[200:136 404:10])

@ldemailly
Copy link
Copy Markdown
Member Author

ldemailly commented Oct 7, 2017 •
edited
Loading

to test:

# first install
./bazel-bin/tests/e2e/tests/simple/go_default_test -alsologtostderr -test.v -v 2  --skip_cleanup  --namespace=e2e -istioctl  ~/tmp/istio-0.2.7/bin/istioctl --auth_enable --test.run  Test404sDuringChanges --pilot_hub=$PILOT_HUB --pilot_tag=$PILOT_TAG
# then add --skip_setup
./bazel-bin/tests/e2e/tests/simple/go_default_test -alsologtostderr -test.v -v 2  --skip_setup --skip_cleanup  --namespace=e2e -istioctl  ~/tmp/istio-0.2.7/bin/istioctl --auth_enable --test.run  Test404sDuringChanges --pilot_hub=$PILOT_HUB --pilot_tag=$PILOT_TAG

@ldemailly
Copy link
Copy Markdown
Member Author

ldemailly commented Oct 12, 2017 •
edited
Loading

the 404s still reproes locally for me but somehow they didn't in the last prow run (only 503s, no 404s) since picking up new deps; they did before though, retrying:

/retest

@ldemailly
Copy link
Copy Markdown
Member Author

ldemailly commented Mar 22, 2018

@andraxylia let me get those in shape, then maybe merge them with "t.Skip()" and you can pick them up

@ldemailly
Copy link
Copy Markdown
Member Author

ldemailly commented Mar 22, 2018

maybe you can make a new test with alpha3 rules but I think it's important to resolve the 503s with alpha1

@andraxylia
Copy link
Copy Markdown
Contributor

andraxylia commented Mar 22, 2018

We will drop alpha1 is 0.8, so I will not even try.

@ldemailly
Copy link
Copy Markdown
Member Author

ldemailly commented Mar 22, 2018

we are not dropping alpha1 in 0.8; 0.8 is the last version with alpha1 and alpha1 is the only stable one

@ldemailly
Copy link
Copy Markdown
Member Author

ldemailly commented Mar 22, 2018

btw it's pretty good right now / better - at least on circle; it only got 8 errors during rule deletion, vs a lot more errors in 0.2

/test e2e-simple

@istio-merge-robot
Copy link
Copy Markdown

istio-merge-robot commented Mar 31, 2018

@ldemailly PR needs rebase

ldemailly
ldemailly commented Apr 19, 2018
View reviewed changes
tests/e2e/framework/kubernetes.go
if err = k.deployAddons(); err != nil {
log.Error("Failed to deploy istio addons")
return err
if k.InstallAddons {
Copy link
Copy Markdown
Member Author

@ldemailly ldemailly Apr 19, 2018

Choose a reason for hiding this comment

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

@douglas-reid would you know how/where I need to set it to true for test that require zipkin ?

rshriram
rshriram reviewed Apr 20, 2018
View reviewed changes
tests/e2e/tests/simple/a_simple_1_test.go
nonInjectedYaml = "tests/e2e/tests/simple/testdata/servicesNotInjected.yaml"
routingR1Yaml = "tests/e2e/tests/simple/testdata/routingrule1.yaml"
routingR2Yaml = "tests/e2e/tests/simple/testdata/routingrule2.yaml"
routingRNPYaml = "tests/e2e/tests/simple/testdata/routingruleNoPods.yaml"
Copy link
Copy Markdown
Member

@rshriram rshriram Apr 20, 2018

Choose a reason for hiding this comment

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

Nit: use the new v1alpha3. Fixing in v1alpha1 is out of scope given all the mechanics we have in new v2.

Copy link
Copy Markdown
Member Author

@ldemailly ldemailly Apr 20, 2018

Choose a reason for hiding this comment

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

it'd be a good test of our automated conversion/translation -> v1alpha3 (#3621 (comment)), but I want to get this in as is before we branch, it's been > 6 months and will allow andra and others test their fixes

tests/e2e/tests/simple/testdata/routingruleNoPods.yaml
weight: 33
- labels:
version: v2
weight: 34
Copy link
Copy Markdown
Member

@rshriram rshriram Apr 20, 2018

Choose a reason for hiding this comment

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

In new v1alpha3 you have to ensure new subsets are added before old ones are deleted. Ask Andra about it.

@istio-testing
Copy link
Copy Markdown
Collaborator

istio-testing commented Apr 20, 2018 •
edited
Loading

@ldemailly: The following tests failed, say /retest to rerun them all:

Test name Commit Details Rerun command
prow/e2e-cluster_wide-auth.sh 0c5f5d9 link /test e2e-cluster_wide-auth
prow/e2e-suite-rbac-no_auth.sh 0c5f5d9 link /test e2e-suite-rbac-no_auth
prow/e2e-suite-rbac-auth.sh bda1693 link /test e2e-suite-rbac-auth
prow/e2e-bookInfoTests-v1alpha3.sh 32f5512 link /test e2e-bookInfo-envoyv2-v1alpha3
Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@ldemailly
Copy link
Copy Markdown
Member Author

ldemailly commented Apr 20, 2018

All required tests passed
screen shot 2018-04-20 at 11 31 24 am

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ZackButcher ZackButcher ZackButcher approved these changes

+1 more reviewer

@rshriram rshriram rshriram left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@andraxylia andraxylia

Labels

area/networking

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.