Openshift-ci onboarding #39
Conversation
israel-hdez
force-pushed
the
os-ci-onboarding
branch
from
f8c6939
to
ef07ebf
Compare
israel-hdez
force-pushed
the
os-ci-onboarding
branch
6 times, most recently
from
a070dc9
to
cffa750
Compare
These are the needed changes to have openshift-ci running the E2E tests successfully. There are several groups of E2E tests that can be deduced from the .github/workflows/e2e-test.yaml file: fast, slow, explainer, transformer-mms, qpext, grpc, helm, raw and kourier. For ODH, the `fast`, `slow` and `grpc` groups are the ones that cover the features that are going to be supported in the initial adoption of ODH. This commit contains the needed adaptations to the E2E tests of the `fast` and `slow` groups to successfully run them in an openshift cluster. It also adds a few scripts on test/scripts/openshift-ci to run these E2Es in the openshift-ci operator. Some of these changes should be seen as provisional and should be rolled back: * test/e2e/common/utils.py: because of the networking/DNS expectations, that are currently not covered in ODH's installation. * test/e2e/predictor/*: * In general all changes under this path should be seen as provisional. However, since ODH won't support all ServingRuntimes, it is possible that some of the tests will stay out. * There are some GRPC-related tests marked as skipped. Since this work is not enabling the `grpc` group, a subsequent commit/PR for enabling GRPC E2Es should remove/revert those skip marks. * Also, there are some tests skipped with the `Not testable in ODH at the moment` reason. The root cause of the failure should be investigated to re-enable these tests. * python/kserve/kserve/models/v1beta1_inference_service.py: This is injecting an annotation that is required given the specifics of OSSM/Maistra and OpenShift-Serverless that are used in ODH. This annotation is, currently, user responsibility and this was the cleanest way to add it in the E2Es. Being platform-specific, it's been discussed that this (and some other) annotation should be injected by some controller to relief the user from this responsibility. If this happens, this change should be reverted. Also, ideally, changes to the following files should be contributed back to upstream. Those changes are not required in upstream and should have no effect, but in openshift-ci become required because a different builder image is being used: * Dockerfile * agent.Dockerfile Signed-off-by: Edgar Hernández <23639005+israel-hdez@users.noreply.github.com>
israel-hdez
force-pushed
the
os-ci-onboarding
branch
from
cffa750
to
ecff079
Compare
| @@ -150,6 +150,8 @@ def metadata(self, metadata): | |||
| :param metadata: The metadata of this V1beta1InferenceService. # noqa: E501 | |||
| :type: V1ObjectMeta | |||
| """ | |||
| if metadata is not None: | |||
There was a problem hiding this comment.
Are these tests from upstream? If so, why are we modifying them downstream first?
There was a problem hiding this comment.
Yes, these come from upstream. In the main comment of the PR is on the list of things that we may want to revert.
It is here because upstream does not need this. It is specific to our OS prerequisites.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: israel-hdez, Xaenalt 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 |
|
/retest |
|
@israel-hdez: The following tests failed, say
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. |
|
@vaibhavjainwiz There seems to be a bad configuration for the image push. Merging, since images build OK, but cannot be pushed. |
What this PR does / why we need it:
These are the needed changes to have openshift-ci running the E2E tests successfully.
There are several groups of E2E tests that can be deduced from the .github/workflows/e2e-test.yaml file: fast, slow, explainer, transformer-mms, qpext, grpc, helm, raw and kourier. For ODH, the
fast,slowandgrpcgroups are the ones that cover the features that are going to be supported in the initial adoption of ODH.This commit contains the needed adaptations to the E2E tests of the
fastandslowgroups to successfully run them in an openshift cluster. It also adds a few scripts on test/scripts/openshift-ci to run these E2Es in the openshift-ci operator.Some of these changes should be seen as provisional and should be rolled back:
grpcgroup, a subsequent commit/PR for enabling GRPC E2Es should remove/revert those skip marks.Not testable in ODH at the momentreason. The root cause of the failure should be investigated to re-enable these tests.Also, ideally, changes to the following files should be contributed back to upstream. Those changes are not required in upstream and should have no effect, but in openshift-ci become required because a different builder image is being used:
Which issue(s) this PR fixes :
Related #5
Feature/Issue validation/testing:
This is about the tests themselves :-)
Checklist:
🙂 Have you added unit/e2e tests that prove your fix is effective or that this feature works?
✔️ Has code been commented, particularly in hard-to-understand areas?
➖ Have you made corresponding changes to the documentation?
Release note: