Skip to content
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

Introduce Helm Chart Test Framework Along With Basic Tests #9

Merged
merged 1 commit into from
Dec 4, 2024

Conversation

mkysel
Copy link
Collaborator

@mkysel mkysel commented Dec 2, 2024

We had no tests of the Helm charts in the past. This fixes that.

  • Fix a few issues that got revealed by the tests.
  • Introduce new CI steps
  • Introduce linters

The tests are based on gruntwork-io/terratest which allow you to assert certain facts about the Kubernetes environment. The basic set of tests installs a chart using the minimal set of variables and asserts that the pods start succesfully.

We now test MLS validation, payer service and the main sync+API service. I will make a follow-up PR that splits the sync service from the API service.

The majority of the code here is required to handle the weird Github actions environment that does not allow for easy log collection. The framework collects all relevant logs, dumps them to a file and collects all these files as a CI artefact.

Each test runs in its own namespace, so in theory, we could run them all in parallel. This also allows for easy cleanup for local testing as all you have to do is delete the namespace.

We can use this framework for much more complicated stuff if we felt like it. For now, it just ensures that the charts actually work.

Fixes #1

@mkysel mkysel changed the title Mkysel/helm tests Introduce Helm Chart Test Framework Along With Basic Tests Dec 2, 2024
@mkysel mkysel requested review from neekolas and fbac December 3, 2024 16:55
@mkysel mkysel marked this pull request as ready for review December 3, 2024 16:55
Copy link
Contributor

@neekolas neekolas left a comment

Choose a reason for hiding this comment

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

Love seeing tests for infrastructure

go-lint-fmt:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
Copy link

Choose a reason for hiding this comment

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

Suggested change
- uses: actions/checkout@v2
- uses: actions/checkout@v4

Comment on lines +27 to +30
- name: Set up Go
uses: actions/setup-go@v2
with:
go-version: 1.23
Copy link

Choose a reason for hiding this comment

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

In general, when it comes to maintaining versions in CI/CD I'd suggest using some kind of version control system.

For example this one https://github.com/smartcontractkit/tool-versions-to-env-action, which centralizes all the versions in a file .tool-versions in the root of the repository, with this format:

golang 1.22.9
golangci-lint 1.62.0
task 3.40.0
setup-go 5

Then, to call it from a workflow it would look like this:

      - uses: smartcontractkit/tool-versions-to-env-action@aabd5efbaf28005284e846c5cf3a02f2cba2f4c2 # v1.0.8
        id: tool-versions

      - name: Setup go ${{ steps.tool-versions.outputs.golang_version }}
        uses: actions/setup-go@${{ steps.tool-versions.outputs.setup-go_version }}
        with:
          go-version: ${{ steps.tool-versions.outputs.golang_version }}

)

func installDB(t *testing.T, options *helm.Options, helmChartReleaseName string) {
helm.Install(t, options, "oci://registry-1.docker.io/bitnamicharts/postgresql", helmChartReleaseName)
Copy link

Choose a reason for hiding this comment

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

nit: probably out of the scope for this PR, but this could be moved to a const and potentially be overridden with a flag provided by the user. I'm thinking here about some admin that wants to customize the bitnami postgresql for example.

func findXmtpContainer(pod *corev1.Pod) *corev1.Container {
// look for any container named "xmtp" that has logs
for _, container := range pod.Spec.Containers {
if (container.Name == "xmtpd") && doesContainerHaveLogs(&container, pod.Status.ContainerStatuses) {
Copy link

Choose a reason for hiding this comment

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

nit: I usually default to create a const for every value like "xmtpd" subject to change at some point.


AwaitNrReplicasReady(t, namespaceName, mlsDeployment, replicaCount)

return
Copy link

Choose a reason for hiding this comment

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

nit: personally I'd adhere to the golang style guide and use naked returns in really small and simple funcs, where it's guaranteed to be readable. Obviously this is not an issue!

@mkysel mkysel merged commit e3063b1 into main Dec 4, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Minikube Helm Charts CI tests
3 participants