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

refactor pytest e2e on top of k8s manifests #380

Closed
tarilabs opened this issue Sep 12, 2024 · 1 comment · Fixed by #447
Closed

refactor pytest e2e on top of k8s manifests #380

tarilabs opened this issue Sep 12, 2024 · 1 comment · Fixed by #447

Comments

@tarilabs
Copy link
Member

Following the great work and refactor from @isinyaaa where the pytest are now being leveraged for e2e-testing,

I would propose to move away from the use of docker-compose.yaml,
and rebase the e2e on top of the KF manifests,
with the following proposal:

Before all e2e tests

This is where we typically launch the docker compose up.

  1. kind create cluster
  2. kubectl create namespace kubeflow
  3. from the manifests directory, kubectl apply -k overlays/db
  4. wait for Deployments like we do here
  5. launch kubectl port-forward svc/model-registry-service -n kubeflow 8080:8080 in the background, maybe noting down pid
  6. maybe await a couple seconds for the port fwd like we do here

After each test

This is where we typically delete the sqlite file, and restart the mlmd service in docker compose.

Seems to me this:

kubectl exec -n kubeflow -it $(kubectl get pods -l component=db -o jsonpath="{.items[0].metadata.name}" -n kubeflow) -- mysql -u root -ptest -D metadb -e "START TRANSACTION; DELETE FROM Artifact; DELETE FROM ArtifactProperty; DELETE FROM Association; DELETE FROM Attribution; DELETE FROM Context; DELETE FROM ContextProperty; DELETE FROM Event; DELETE FROM EventPath; DELETE FROM Execution; DELETE FROM ExecutionProperty; DELETE FROM ParentContext; COMMIT;"

should do equivalently (yes it's a 1-liner).

I have run a couple scripts locally, for the same model name and version, which is passing (wouldn't otherwise) but not the full e2e test suite. I prefer to mention this in full transparency, because I don't have full confirmation beforehand this will be enough.

After all e2e tests

This is where we typically do the docker compose down.

Should be enough to:

  1. stop ps of port fwd from the previous step
  2. kubectl delete cluster

Additional considerations

Currently we re-generate the image and we let docker compose pick-it-up, but for kind (or minikube, etc) we actually need the image as part of the "cluster"'s registry: maybe we could repurpose the make target to do these operations, such as regenerating the image, starting kind, kind load docker-image ..., etc and preparing the cluster for the e2e testing from pytest.

@Al-Pragliola
Copy link
Contributor

I fully agree with this proposal and vouch for this change. It will bring us closer to real-world scenarios and help identify problems that could arise in an actual cluster.

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 a pull request may close this issue.

2 participants