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

docs, fixes and scripts to run e2e tests in minikube #1254

Merged
merged 7 commits into from
Jan 8, 2021

Conversation

oavdeev
Copy link
Collaborator

@oavdeev oavdeev commented Jan 6, 2021

What this PR does / why we need it:
This contains a doc describing how to run e2e tests in minikube, and some fixes and scripts to make that work.

The fixes are to make sure custom S3 endpoints are used throughout by Feast and tests, so it can run against minio.

Does this PR introduce a user-facing change?:

NONE

tests/README.md Outdated
```bash
#
# NB!! make sure to use bash 5.0+ for the next step. You'd need to brew install it on OSX
DOCKER_REPOSITORY=gcr.io/kf-feast GIT_TAG=develop bash ./infra/scripts/setuplocal.sh
Copy link
Member

Choose a reason for hiding this comment

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

Should this be setup-e2e-local.sh?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yep good catch, that was a last minute rename

Signed-off-by: Oleg Avdeev <[email protected]>
Signed-off-by: Oleg Avdeev <[email protected]>
@oavdeev
Copy link
Collaborator Author

oavdeev commented Jan 6, 2021

/kind housekeeping

@@ -0,0 +1,109 @@
#!/bin/bash
Copy link
Collaborator

Choose a reason for hiding this comment

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

can we combine this with infra/scripts/runner-helper.sh

Copy link
Contributor

Choose a reason for hiding this comment

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

good point, done

Signed-off-by: Oleg Avdeev <[email protected]>
Signed-off-by: Oleg Avdeev <[email protected]>
@feast-ci-bot
Copy link
Collaborator

feast-ci-bot commented Jan 8, 2021

@oavdeev: The following test failed, say /retest to rerun all failed tests:

Test name Commit Details Rerun command
test-end-to-end-azure c76c602 link /test test-end-to-end-azure

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@oavdeev
Copy link
Collaborator Author

oavdeev commented Jan 8, 2021

/test test-end-to-end-aws

@feast-ci-bot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: oavdeev, woop

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:

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

@woop
Copy link
Member

woop commented Jan 8, 2021

/lgtm

if ! time helm install --wait "$RELEASE" infra/charts/feast \
--timeout 15m \
if ! time helm install --wait "$RELEASE" ./infra/charts/feast \
--timeout 5m \
Copy link
Collaborator

Choose a reason for hiding this comment

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

the azure test requires >5min (15 to be safe), I believe because of some services requiring others to be up + crashloopbackoff. Probably could be fixed but I had just increased the timeout

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ok rolled this back, but this is peculiar since on EKS it usually takes maybe a minute, and you'd think this is pure k8s stuff that shouldn't depend on the cloud provider much.

Signed-off-by: Oleg Avdeev <[email protected]>
@feast-ci-bot
Copy link
Collaborator

New changes are detected. LGTM label has been removed.

@feast-ci-bot feast-ci-bot removed the lgtm label Jan 8, 2021
def _get_staging_client(self):
uri = urlparse(self._staging_location)
return get_staging_client(uri.scheme, self._config)

def _get_azure_credentials(self):
Copy link
Collaborator

Choose a reason for hiding this comment

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

I had changed it recently to just pass in the whole config but if we don't want to do that, then this function needs the Azure blob account name and access key passed in

@oavdeev oavdeev merged commit a99005c into feast-dev:master Jan 8, 2021
@oavdeev oavdeev deleted the minikube-tests branch January 8, 2021 04:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants