Skip to content

Integration test for the 'upgrade' command#2679

Merged
ihcsim merged 3 commits intomasterfrom
isim/upgrade-integration-test
Apr 12, 2019
Merged

Integration test for the 'upgrade' command#2679
ihcsim merged 3 commits intomasterfrom
isim/upgrade-integration-test

Conversation

@ihcsim
Copy link
Contributor

@ihcsim ihcsim commented Apr 10, 2019 •
edited
Loading

The extra steps to download and install an edge release increases the test duration by less than a minute, on my machine:

This branch:

=== PASS: all tests passed
bin/test-run `pwd`/bin/linkerd  33.16s user 4.85s system 9% cpu 6:33.27 total

master:

=== PASS: all tests passed
bin/test-run `pwd`/bin/linkerd  35.64s user 5.46s system 11% cpu 5:52.69 total

Fixes #2669

Signed-off-by: Ivan Sim [email protected]

@ihcsim ihcsim requested a review from siggy April 10, 2019 03:09
@l5d-bot
Copy link
Collaborator

l5d-bot commented Apr 10, 2019

Integration test results for 377073a: fail 😕
Log output: https://gist.github.com/2cb0cb76ea473f2403d33ba4c0cd472d

@ihcsim ihcsim force-pushed the isim/upgrade-integration-test branch from 377073a to 71e1c67 Compare April 10, 2019 03:46
@l5d-bot
Copy link
Collaborator

l5d-bot commented Apr 10, 2019

Integration test results for 71e1c67: fail 😕
Log output: https://gist.github.com/58dcbc939ac85760efc77201c993f1ef

@ihcsim ihcsim force-pushed the isim/upgrade-integration-test branch from 71e1c67 to 17a1a55 Compare April 10, 2019 04:07
@l5d-bot
Copy link
Collaborator

l5d-bot commented Apr 10, 2019

Integration test results for 17a1a55: fail 😕
Log output: https://gist.github.com/6861d5c6ff8a6e104079d021895b57f0

@ihcsim ihcsim force-pushed the isim/upgrade-integration-test branch from 17a1a55 to c4f4715 Compare April 10, 2019 05:41
@l5d-bot
Copy link
Collaborator

l5d-bot commented Apr 10, 2019

Integration test results for c4f4715: fail 😕
Log output: https://gist.github.com/51ad468e414a291673772bdae133d478

@ihcsim ihcsim force-pushed the isim/upgrade-integration-test branch 4 times, most recently from dc6475f to 40917f9 Compare April 10, 2019 06:47
@l5d-bot
Copy link
Collaborator

l5d-bot commented Apr 10, 2019

Integration test results for 40917f9: fail 😕
Log output: https://gist.github.com/62d0e2eeb322213b5db40601d29577d8

@ihcsim ihcsim force-pushed the isim/upgrade-integration-test branch from 40917f9 to 304d15b Compare April 10, 2019 15:45
@l5d-bot
Copy link
Collaborator

l5d-bot commented Apr 10, 2019

Integration test results for 304d15b: fail 😕
Log output: https://gist.github.com/5b0542ee4c5d114525e672f7acdac737

@ihcsim ihcsim force-pushed the isim/upgrade-integration-test branch from 304d15b to 5252609 Compare April 10, 2019 16:49
@l5d-bot
Copy link
Collaborator

l5d-bot commented Apr 10, 2019

Integration test results for 5252609: fail 😕
Log output: https://gist.github.com/af2ac25b1794b31e86f92b6a37b74523

@ihcsim ihcsim force-pushed the isim/upgrade-integration-test branch from 5252609 to b488ed8 Compare April 10, 2019 17:10
@l5d-bot
Copy link
Collaborator

l5d-bot commented Apr 10, 2019

Integration test results for b488ed8: fail 😕
Log output: https://gist.github.com/28c07c770da8c3168d9e4f46d4528170

siggy
siggy reviewed Apr 10, 2019
View reviewed changes
Copy link
Member

@siggy siggy left a comment

Choose a reason for hiding this comment

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

awesome!

had a couple comments around the --proxy-auto-inject flag, but overall this looks (and works) great!

=== PASS: all tests passed

@ihcsim ihcsim force-pushed the isim/upgrade-integration-test branch from b488ed8 to dfd72aa Compare April 11, 2019 04:35
ihcsim
ihcsim commented Apr 11, 2019
View reviewed changes
bin/test-run
printf "Running test [%s] %s\\n" "$(basename "$1")" "$2"
go test -v "$1" --linkerd="$linkerd_path" --linkerd-namespace="$linkerd_namespace" --k8s-context="$k8s_context" --integration-tests "$2"
filename="$1"
shift
Copy link
Contributor Author

@ihcsim ihcsim Apr 11, 2019

Choose a reason for hiding this comment

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

It's like args[1:].

ihcsim
ihcsim commented Apr 11, 2019
View reviewed changes
bin/test-run
shift

printf "Running test [%s] %s\\n" "$(basename "$filename")" "$@"
go test -v "$filename" --linkerd="$linkerd_path" --k8s-context="$k8s_context" --integration-tests "$@"
Copy link
Contributor Author

@ihcsim ihcsim Apr 11, 2019

Choose a reason for hiding this comment

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

Replaced the "$2" with "$@", because the former converts multiple arguments into one long, space-delimited argument. For example, when running run_test filename --linkerd-namespace=linkerd --proxy-auto-inject, the testutil.TestHelper ends up treating the linkerd namespace as linkerd --proxy-auto-inject.

Copy link
Contributor Author

@ihcsim ihcsim Apr 11, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Also, removed the --linkerd-namespace="$linkerd_namespace" argument. This should be part of $@, instead of relying on the global $linkerd_namespace variable. This was problematic for upgrade and auto-inject test cases which depend on the control plane in a different namespace.

@l5d-bot
Copy link
Collaborator

l5d-bot commented Apr 11, 2019

Integration test results for dfd72aa: fail 😕
Log output: https://gist.github.com/f2db297136528c496f7851f87143d3ae

@ihcsim ihcsim force-pushed the isim/upgrade-integration-test branch from dfd72aa to cd843e1 Compare April 11, 2019 18:30
@l5d-bot
Copy link
Collaborator

l5d-bot commented Apr 11, 2019

Integration test results for cd843e1: success 🎉
Log output: https://gist.github.com/3c5e8f34c4b8a060ecd2833a81335d18

siggy
siggy approved these changes Apr 11, 2019
View reviewed changes
Copy link
Member

@siggy siggy left a comment

Choose a reason for hiding this comment

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

looks good! one more nit then 🚢 👍

Ivan Sim added 2 commits April 11, 2019 14:24
@ihcsim ihcsim force-pushed the isim/upgrade-integration-test branch from cd843e1 to bcf9ff4 Compare April 11, 2019 21:54
@l5d-bot
Copy link
Collaborator

l5d-bot commented Apr 11, 2019

Integration test results for bcf9ff4: success 🎉
Log output: https://gist.github.com/0cd572fae372f28709682532272cc51d

@ihcsim ihcsim merged commit 1c0f147 into master Apr 12, 2019
@ihcsim ihcsim deleted the isim/upgrade-integration-test branch April 12, 2019 02:37
@alpeb alpeb mentioned this pull request Apr 16, 2019
Closed
siggy added a commit that referenced this pull request May 20, 2019
@siggy
Modify upgrade integration test to upgrade stable
007eba6 
In #2679 we introduced an upgrade integration test. At the time we only
supported upgrading from a recent edge. Since that PR, a stable build
was released supporting upgrade.

Modify the upgrade integration test to upgrade from the latest stable
rather than latest edge. This fulfills the original intent of #2669.

Also add some known k8s event warnings.

Signed-off-by: Andrew Seigner <[email protected]>
@siggy siggy mentioned this pull request May 20, 2019
Merged
siggy added a commit that referenced this pull request May 20, 2019
@siggy
Modify upgrade integration test to upgrade stable
0e19d9e 
In #2679 we introduced an upgrade integration test. At the time we only
supported upgrading from a recent edge. Since that PR, a stable build
was released supporting upgrade.

Modify the upgrade integration test to upgrade from the latest stable
rather than latest edge. This fulfills the original intent of #2669.

Also add some known k8s event warnings.

Signed-off-by: Andrew Seigner <[email protected]>
ihcsim pushed a commit that referenced this pull request May 20, 2019
@siggy @ihcsim
Modify upgrade integration test to upgrade stable (#2835)
bd4c278 
In #2679 we introduced an upgrade integration test. At the time we only
supported upgrading from a recent edge. Since that PR, a stable build
was released supporting upgrade.

Modify the upgrade integration test to upgrade from the latest stable
rather than latest edge. This fulfills the original intent of #2669.

Also add some known k8s event warnings.

Signed-off-by: Andrew Seigner <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@siggy siggy siggy approved these changes

Assignees

@ihcsim ihcsim

Labels

area/test

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

upgrade integration tests

3 participants

@ihcsim @l5d-bot @siggy