Skip to content

Conversation

@jgehrcke
Copy link
Collaborator

@jgehrcke jgehrcke commented Sep 19, 2025

For #613.

Introduces TEST_CHART_LOCAL.

You may want to review the rendered readme update.

Output:

$ TEST_CHART_LOCAL=1 make bats
[...]
tests.bats
[...]
 ✓ helm-install 25.8.0-dev from deployments/helm/nvidia-dra-driver-gpu/ [5294]
[...]
 ✓ upgrade: wipe-state, install-last-stable, upgrade-to-current-dev [41290]

14 tests, 0 failures in 122 seconds

Still works:

$ unset TEST_CHART_LOCAL 
$ export TEST_CHART_VERSION="25.8.0-dev-124734f2-chart"
$ make bats
[...]
	--env TEST_CHART_REPO="oci://ghcr.io/nvidia/k8s-dra-driver-gpu" \
	--env TEST_CHART_VERSION=25.8.0-dev-124734f2-chart \
[...]
tests.bats
[...]
 ✓ helm-install 25.8.0-dev-124734f2-chart from oci://ghcr.io/nvidia/k8s-dra-driver-gpu [5549]
...

@copy-pr-bot
Copy link

copy-pr-bot bot commented Sep 19, 2025

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@jgehrcke jgehrcke force-pushed the jp/bats-for-local-dev branch from 152e93a to 2c8eafa Compare September 19, 2025 10:44
# Consumed in upgrade test via kubectl apply -f <URL>
# (can be a branch, tag, or commit). TODO: parse default
# from `TEST_CHART_VERSION`.
TEST_CRD_UPGRADE_TARGET_GIT_REF ?= "main"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

todo: override this, too, for local dev (consume from local checkout).
will do in a follow-up.

@klueska
Copy link
Collaborator

klueska commented Sep 22, 2025

It feels a bit awkward to have to set a separate variable of TEST_CHART_LOCAL to enable local development.

I would actually expect the default to be local development, i.e.

TEST_CHART_REPO="$(PROJECT_DIR)/deployments/helm/k8s-dra-driver-gpu"
TEST_CHART_VERSION="devel"

When you then optionally override if you want to test something externally.

@jgehrcke
Copy link
Collaborator Author

jgehrcke commented Sep 22, 2025

Thanks for having had a quick look!

It feels a bit awkward to have to set a separate variable of TEST_CHART_LOCAL to enable local development.

Right... because it is awkward :).

I would actually expect the default to be local development

I agree that we want to have a proper 'local first' procedure.

Screenshot from my planning doc:
image

That is: tested system based on local state, including container image, CRD, Helm chart (and whatever I forgot to enumerate here).

Today, this doesn't work auto-magically because images need to be built and get to the nodes.

I don't want TEST_CHART_LOCAL to stay. Think of it as a rather temporary and maybe even pointless (feature) flag as we sort some things out before we can nicely first-class this use case. With, eventually, a polished interface and useful semantics.

I explicitly want to iterate rather ruthlessly on the configuration interface for now.

It's important to get the feedback and have the discussion. For today, I'd like to move forward with this patch because I'm stacking PRs (#598) and I'd like to minimize overhead.

@jgehrcke
Copy link
Collaborator Author

jgehrcke commented Sep 23, 2025

Will move forward with this. The test and README adjustments here are useful, and used in #598, #610.

Also discussed out-of-band: I will pay attention to not breaking established workflows, and I appreciate that we bear with me for the moment. It's important for me to move fast and keep my momentum up as I try to converge sub efforts quickly here. Again, the ultimate goal is to get us to start iterating against CI (before that, everything is rather premature, especially interface considerations and technical priorities will become clearest in view of CI).

@jgehrcke jgehrcke self-assigned this Sep 23, 2025
@jgehrcke jgehrcke added the ci/testing issue/PR related to CI and/or testing label Sep 23, 2025
@jgehrcke jgehrcke added this to the v25.8.0 milestone Sep 23, 2025
@jgehrcke jgehrcke merged commit adf0b8e into NVIDIA:main Sep 23, 2025
7 checks passed
@klueska klueska moved this from Backlog to Closed in Planning Board: k8s-dra-driver-gpu Sep 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci/testing issue/PR related to CI and/or testing

Projects

Development

Successfully merging this pull request may close these issues.

2 participants