e2e cleanup

[nirrozenbaum]

this PR removes the existing hack/test-e2e.sh script that isn't used or referenced anywhere, and renames the new e2e script to hack/test-e2e.sh (instead of hack/run-e2es).

[netlify]

✅ Deploy Preview for gateway-api-inference-extension ready!

Name	Link
🔨 Latest commit	85aefa2
🔍 Latest deploy log	https://app.netlify.com/projects/gateway-api-inference-extension/deploys/68507df6912ad800080b247f
😎 Deploy Preview	https://deploy-preview-988--gateway-api-inference-extension.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify project configuration.

[ahg-g]

/assign @danehans

[danehans]

This var seems to be unused

[danehans]

I don't see the Gateway API CRDs being installed anywhere and they're not needed to run the e2e.

[nirrozenbaum]

@danehans both comments are correct. it was a leftover from the previous code that I just renamed.
fixed both.

[kfswain]

/approve
looks good on my end & looks like @danehans comments are addressed. Will let him leave the final lgtm stamp tho

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: kfswain, nirrozenbaum

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [kfswain]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[danehans]

Shouldn't we build and load the EPP image if an existing kubectl context is found here:

gateway-api-inference-extension/hack/test-e2e.sh

Line 33 in 85aefa2

echo "Active kubecontext found. Running Go e2e tests in ./epp..."

Something like:

if kubectl config current-context >/dev/null 2>&1; then
  KIND_CLUSTER=$(kubectl config current-context) make image-kind
  echo "Active kind cluster $KIND_CLUSTER found in kubecontext. Running Go e2e tests in ./epp..."
else
  install_kind
  kind create cluster --name inference-e2e
  KIND_CLUSTER=inference-e2e make image-kind
  echo "Kind cluster $KIND_CLUSTER created. Running Go e2e tests in ./epp..."
fi

[nirrozenbaum]

Shouldn't we build and load the EPP image if an existing kubectl context is found here:

gateway-api-inference-extension/hack/test-e2e.sh

Line 33 in 85aefa2

echo "Active kubecontext found. Running Go e2e tests in ./epp..."

Something like:

if kubectl config current-context >/dev/null 2>&1; then
  KIND_CLUSTER=$(kubectl config current-context) make image-kind
  echo "Active kind cluster $KIND_CLUSTER found in kubecontext. Running Go e2e tests in ./epp..."
else
  install_kind
  kind create cluster --name inference-e2e
  KIND_CLUSTER=inference-e2e make image-kind
  echo "Kind cluster $KIND_CLUSTER created. Running Go e2e tests in ./epp..."
fi

@danehans good question.
when kubectl context is found we don't know if the cluster that is used is kind, and I don't think we can make that assumption.

having said that, I think you just caught an important bug - e2e tests are always using main image and not the testing the PR code.
https://github.com/kubernetes-sigs/gateway-api-inference-extension/blob/main/test/testdata/inferencepool-e2e.yaml#L50

[nirrozenbaum]

@danehans @kfswain am I missing something or is this setup always tests main image in PRs (meaning we're not e2e testing the PR changes)?
my original intention was just a file rename but this seems to be a bug that should be addressed immediately.
what are your thoughts about having a dedicated target for running e2e tests in ci? then we can always setup kind, load the image and run the tests.

[danehans]

@danehans @kfswain am I missing something or is this setup always tests main image in PRs (meaning we're not e2e testing the PR changes)?

You are correct. You can follow a pattern similar to $E2E_NS to set the image for the EPP manifest. For example:

Replace the image value in https://github.com/kubernetes-sigs/gateway-api-inference-extension/blob/main/test/testdata/inferencepool-e2e.yaml#L50 with a variable:

image: $E2E_IMAGE

Update the e2e script to use the existing EXTRA_TAG env var in the Makefile:

# build & load the image into kind
EXTRA_TAG=e2e \
KIND_CLUSTER=inference-e2e \
make image-kind

# Tell the test suite the image to look for
export E2E_IMAGE="${IMAGE_REPO}:${EXTRA_TAG}"

Do the string replacement in the helper that already handles $E2E_NS:

func createInferExt(k8sClient client.Client, filePath string) {
    image := os.Getenv("E2E_IMAGE")
    gomega.Expect(image).NotTo(gomega.BeEmpty())

   ....
        m = strings.ReplaceAll(m, "$E2E_IMAGE", image)
    }
    …
}

[nirrozenbaum]

You are correct. You can follow a pattern similar to $E2E_NS to set the image for the EPP manifest. For example:

@danehans yes, I know how to solve it :).
only wanted to verify I'm not missing anything.
I will take care of this.

[nirrozenbaum]

/hold