- 
                Notifications
    You must be signed in to change notification settings 
- Fork 717
CI: test Kubernetes port forwarder #4118
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | 
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| # Extra tests | ||
|  | ||
| The extra tests located in this directory are not automatically executed via `make bats`. | ||
|  | ||
| Some tests are executed on the CI, some ones are not. | ||
| Refer to the configuration of the GitHub Actions to see what tests are executed. | 
| Original file line number | Diff line number | Diff line change | 
|---|---|---|
| @@ -0,0 +1,64 @@ | ||
| # SPDX-FileCopyrightText: Copyright The Lima Authors | ||
| # SPDX-License-Identifier: Apache-2.0 | ||
|  | ||
| # This test verifies that a Kubernetes cluster can be started and that the single node is ready. | ||
|  | ||
| load "../helpers/load" | ||
|  | ||
| : "${TEMPLATE:=k8s}" | ||
|  | ||
| # Instance names are "${NAME}-0", "${NAME}-1", ... | ||
| NAME="k8s" | ||
|  | ||
| get_num_nodes() { | ||
| local nodes=0 | ||
| for tag in "${BATS_TEST_TAGS[@]}"; do | ||
| if [[ $tag =~ ^nodes:([0-9]+)$ ]]; then | ||
| nodes="${BASH_REMATCH[1]}" | ||
| fi | ||
| done | ||
| 
      Comment on lines
    
      +15
     to 
      +19
    
   There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not convinced that this is a good way to control setup for individual tests. I think it would be clearer not to rely on  @test 'Single-node configuration' {
   setup_nodes 1
   # ...
}
@test 'Multi-node configuration' {
   setup_nodes "${LIMA_BATS_K8S_NODES:-3}"
   # ...
}And  This also allows you to control the number of nodes from the outside without modifying the test source code to adjust the tag. (Ab)using the tags to pass arguments to the setup/teardown functions is a bit confusing, and should be simpler. But we can always change this later, e.g. when there actually is a multi-node test. | ||
| if [[ $nodes -eq 0 ]]; then | ||
| echo >&2 "nodes:N tag is required" | ||
| exit 1 | ||
| fi | ||
| echo "$nodes" | ||
| } | ||
|  | ||
| local_setup() { | ||
| local nodes=$(get_num_nodes) | ||
| for ((i=0; i<nodes; i++)); do | ||
| limactl delete --force "${NAME}-$i" || : | ||
| limactl start --tty=false --name "${NAME}-$i" "template:${TEMPLATE}" 3>&- 4>&- | ||
| # NOTE: No support for multi-node clusters yet. | ||
| done | ||
| for node in $(k get node -o name); do | ||
| k wait --timeout=5m --for=condition=ready "${node}" | ||
| done | ||
| } | ||
|  | ||
| local_teardown() { | ||
| local nodes=$(get_num_nodes) | ||
| for ((i=0; i<nodes; i++)); do | ||
| limactl delete --force "${NAME}-$i" || : | ||
| done | ||
| } | ||
|  | ||
| k() { | ||
| There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would call it  | ||
| # The host home directory is not mounted in the case of k8s. | ||
| limactl shell --workdir=/ "${NAME}-0" -- kubectl "$@" | ||
| } | ||
|  | ||
| # bats test_tags=nodes:1 | ||
| @test 'Single-node' { | ||
| k create deployment nginx --image="${TEST_CONTAINER_IMAGES["nginx"]}" | ||
| k rollout status deployment nginx --timeout 60s | ||
| k create service nodeport nginx --node-port=31080 --tcp=80:80 | ||
| run curl --fail --silent --show-error --retry 30 --retry-all-errors http://localhost:31080 | ||
| assert_success | ||
| assert_output --partial "Welcome to nginx" | ||
| # TODO: support UDP | ||
| k delete service nginx | ||
| k delete deployment nginx | ||
| } | ||
|  | ||
| # TODO: add a test for multi-node | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You have to set the variable inside the test that is flaky; the harness sets it to 0, so you can't set it from the outside:
Without the logging you cannot tell if the test needed retries to pass (and
BATS_TEST_RETRY_NUMBERis not defined insetuporteardown).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can set it at the file level, so it applies to every test in the same file:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel GHA YAML is the right place to set
BATS_TEST_RETRIES. Maybe we should submit a proposal to the upstream.Can we just merge this PR?