Revert and re-adapt test/e2e/common/utils.py #147
Conversation
israel-hdez
force-pushed
the
50-revert-e2e-utils-temp-fix
branch
from
dfeef38 to
2fb2dcd
Compare
|
/test ci/prow/e2e-fast ci/prow/e2e-slow |
|
@israel-hdez: The specified target(s) for
Use In response to this:
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. |
|
/test e2e-fast e2e-slow |
| cluster_ip = host | ||
| else: | ||
| cluster_ip = get_cluster_ip() | ||
| return cluster_ip, host, path |
There was a problem hiding this comment.
Sorry, I reverted.
The python linter didn't like it. See: https://github.com/opendatahub-io/kserve/actions/runs/7269665250/job/19807632226#step:4:38
There was a problem hiding this comment.
hmm, not sure why.
PEP8 suggests a line terminator while this custom linter asks to remove.
| @@ -77,6 +77,9 @@ kustomize build $PROJECT_ROOT/config/overlays/test | \ | |||
| oc apply -f - | |||
| oc wait --for=condition=ready pod -l control-plane=kserve-controller-manager -n kserve --timeout=300s | |||
|
|
|||
| echo "Installing odh-model-controller" | |||
| kustomize build $PROJECT_ROOT/test/scripts/openshift-ci | oc apply -f - | |||
There was a problem hiding this comment.
This can be simplified to oc apply -k $PROJECT_ROOT/test/scripts/openshift-ci
israel-hdez
force-pushed
the
50-revert-e2e-utils-temp-fix
branch
2 times, most recently
from
5ab45ae to
c0b7dd6
Compare
This reverts changes done in commit ecff079 to test/e2e/common/utils.py. It is possible to revert since opendatahub-io/odh-model-controller#59 is solved. After the revert, another adaptation was needed. CI in upstream assumes that the URL in the InferenceService cannot be resolved, so it uses the IP of the istio-ingressgateway together with the HTTP Host header to workaround this limitation. However, in ODH testing environment the URL of InferenceServices can be resolved. The adaptation adds a flag to use the URL of the isvc, rather than using the IP of the istio-ingressgateway. This adaptation can be contributed to upstream, because the community can benefit from it. The CI setup is updated, so that the revert works correctly: * odh-model-controller is now being installed * TLS is removed from the istio-ingressgateway Additionally, the following other changes were done to the CI setup: * Turn off Mesh-related components that are not needed for CI * Prefer usage of the ServiceMeshMember CRD to enroll namespaces to the Mesh Fixes opendatahub-io#50 Signed-off-by: Edgar Hernández <[email protected]>
israel-hdez
force-pushed
the
50-revert-e2e-utils-temp-fix
branch
from
c0b7dd6 to
f5fdcf4
Compare
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: israel-hdez, spolti 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 |
openshift-merge-bot
bot
merged commit 4825c74
into
opendatahub-io:master
…p/component-updates/kserve-router-28 Update kserve-router-28 to 21d74d5
What this PR does / why we need it:
This reverts changes done in commit ecff079 to test/e2e/common/utils.py. It is possible to revert since opendatahub-io/odh-model-controller#59 is solved.
After the revert, another adaptation was needed. CI in upstream assumes that the URL in the InferenceService cannot be resolved, so it uses the IP of the istio-ingressgateway together with the HTTP Host header to workaround this limitation. However, in ODH testing environment the URL of InferenceServices can be resolved. The adaptation adds a flag to use the URL of the isvc, rather than using the IP of the istio-ingressgateway.
This adaptation can be contributed to upstream, because the community can benefit from it.
The CI setup is updated, so that the revert works correctly:
Additionally, the following other changes were done to the CI setup:
Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close the issue(s) when PR gets merged):Fixes #50
Checklist:
Release note: