Skip to content
This repository has been archived by the owner on Mar 19, 2024. It is now read-only.

Add conformance testing workflow for GitHub Actions #139

Merged
merged 22 commits into from
Apr 4, 2022

Conversation

nathancoleman
Copy link
Member

Changes proposed in this PR:

Stand up a kind cluster, install MetalLB, and run conformance tests against our implementation of the Gateway API.

How I've tested this PR:

🤖 tests pass

How I expect reviewers to test this PR:

Checklist:

  • Tests added
  • CHANGELOG entry added

    HashiCorp engineers only, community PRs should not add a changelog entry.
    Entries should use imperative present tense (e.g. Add support for...)

nathancoleman and others added 4 commits March 31, 2022 17:16
Co-Authored-By: sarahalsmiller <100602640+sarahalsmiller@users.noreply.github.com>
Co-Authored-By: sarahalsmiller <100602640+sarahalsmiller@users.noreply.github.com>
Co-Authored-By: sarahalsmiller <100602640+sarahalsmiller@users.noreply.github.com>
@nathancoleman nathancoleman marked this pull request as ready for review April 1, 2022 17:52
Copy link
Member Author

@nathancoleman nathancoleman left a comment

Choose a reason for hiding this comment

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

Personal Review

Overall, I really love the workflow here and how we're installing from a Helm chart like our customers do. Would probably love to port the e2e tests over to a similar workflow in the future.

- "conformance-*"

schedule:
- cron: '0 0 * * *'
Copy link
Member Author

Choose a reason for hiding this comment

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

This will run at midnight UTC every day

schedule:
- cron: '0 0 * * *'

workflow_dispatch:
Copy link
Member Author

Choose a reason for hiding this comment

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

This allows us to kick off runs manually in the repo Actions tab if we choose

Copy link
Contributor

Choose a reason for hiding this comment

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

Can this be triggered manually on specific branches/PRs, or only on main?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep yep. Here's the blogpost I initially read about it

Comment on lines 79 to 80
repository: "nathancoleman/gateway-api"
ref: "eventually-consistent-conformance"
Copy link
Member Author

@nathancoleman nathancoleman Apr 1, 2022

Choose a reason for hiding this comment

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

This uses my fork + branch of the gateway-api repo from kubernetes-sigs/gateway-api#1080.
Hoping that merges in the next week or so and we can update this to use kubernetes-sigs/gateway-api@master.

- name: Clone consul-k8s
uses: actions/checkout@v2
with:
repository: "hashicorp/consul-k8s"
Copy link
Member Author

Choose a reason for hiding this comment

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

We need to run using the main branch of consul-k8s for now because of some fixes that haven't been released yet. Once those fixes are released, we can drop this installation from a clone.

@@ -1,5 +1,5 @@
FROM alpine:3.13
FROM alpine:latest
Copy link
Member Author

Choose a reason for hiding this comment

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

Updated to match the default step in Dockerfile. I don't think there's a reason to be on an older version, but can pin this if we generally prefer keeping it pinned.

@@ -29,3 +29,10 @@ patches:
value: {'consul.hashicorp.com/connect-inject': 'true', 'consul.hashicorp.com/connect-service-port': '3000'}
target:
kind: Deployment
# We don't have enough resources in the GitHub-hosted Actions runner to support 2 replicas
Copy link
Member Author

Choose a reason for hiding this comment

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

Turns out kind runs out of CPU before all of the conformance testing resources can spin up. There's no easy option for increasing the size of the runner today. It sounds like the consul team is running into similar issues that we may need to figure out in the future if/when upstream adds more deployments.

internal/testing/conformance/run.sh Outdated Show resolved Hide resolved
Copy link
Contributor

@mikemorris mikemorris left a comment

Choose a reason for hiding this comment

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

Followed the steps, got everything running in a local kind cluster (some tests seem flaky locally but I think that's just a resource/slowness issue) and see the passing CI tests triggered by the branch push, this looks great!

.github/workflows/conformance.yml Outdated Show resolved Hide resolved
schedule:
- cron: '0 0 * * *'

workflow_dispatch:
Copy link
Contributor

Choose a reason for hiding this comment

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

Can this be triggered manually on specific branches/PRs, or only on main?

.github/workflows/conformance.yml Show resolved Hide resolved
.github/workflows/conformance.yml Outdated Show resolved Hide resolved
.gitignore Outdated Show resolved Hide resolved
internal/testing/conformance/consul-config.yaml Outdated Show resolved Hide resolved
.github/workflows/conformance.yml Show resolved Hide resolved
nathancoleman and others added 2 commits April 4, 2022 10:13
Co-authored-by: Mike Morris <mikemorris@users.noreply.github.com>
Co-authored-by: Mike Morris <mikemorris@users.noreply.github.com>
@nathancoleman nathancoleman requested a review from mikemorris April 4, 2022 14:21
@nathancoleman nathancoleman merged commit a2c82f9 into main Apr 4, 2022
@nathancoleman nathancoleman deleted the conformance-ci branch April 4, 2022 18:04
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants