Leverage ObjStore Routes for Prereq Health Checks #519
Conversation
|
A new image has been built to help with testing out this PR: To use this image run the following: cd $(mktemp -d)
git clone git@github.com:opendatahub-io/data-science-pipelines-operator.git
cd data-science-pipelines-operator/
git fetch origin pull/519/head
git checkout -b pullrequest 098c2d020fa439a6defd1db5a4c39d5d7292c22d
make deploy IMG="quay.io/opendatahub/data-science-pipelines-operator:pr-519"More instructions here on how to deploy and test a Data Science Pipelines Application. |
|
Change to PR detected. A new PR build was completed. |
VaniHaripriya
force-pushed
the
issue-221
branch
from
9c5eb01
to
3a51a0d
Compare
|
Change to PR detected. A new PR build was completed. |
|
Change to PR detected. A new PR build was completed. |
1 similar comment
|
Change to PR detected. A new PR build was completed. |
VaniHaripriya
force-pushed
the
issue-221
branch
from
d97abc4
to
6ec90bf
Compare
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
Change to PR detected. A new PR build was completed. |
1 similar comment
|
Change to PR detected. A new PR build was completed. |
@amadhusu It is an expected issue. Updated the testing instructions how to avoid this issue, please take a look at it. |
|
Change to PR detected. A new PR build was completed. |
1 similar comment
|
Change to PR detected. A new PR build was completed. |
VaniHaripriya
force-pushed
the
issue-221
branch
from
01c2ef3
to
2574c0d
Compare
|
Change to PR detected. A new PR build was completed. |
|
Change to PR detected. A new PR build was completed. |
VaniHaripriya
force-pushed
the
issue-221
branch
from
43e5a15
to
1b323ee
Compare
|
Change to PR detected. A new PR build was completed. |
VaniHaripriya
force-pushed
the
issue-221
branch
from
341ec43
to
38e34f4
Compare
|
Change to PR detected. A new PR build was completed. |
|
can we squash these into a single commit |
VaniHaripriya
force-pushed
the
issue-221
branch
from
ffc6825
to
43d9035
Compare
|
Change to PR detected. A new PR build was completed. |
2 similar comments
|
Change to PR detected. A new PR build was completed. |
|
Change to PR detected. A new PR build was completed. |
VaniHaripriya
force-pushed
the
issue-221
branch
from
2307057
to
929396e
Compare
|
Change to PR detected. A new PR build was completed. |
|
Change to PR detected. A new PR build was completed. |
VaniHaripriya
force-pushed
the
issue-221
branch
from
06b5233
to
6fccd1c
Compare
|
Change to PR detected. A new PR build was completed. |
1 similar comment
|
Change to PR detected. A new PR build was completed. |
Resolved pre-commit issues Removed loggers Added TLS capabilities to minio route and handled 503 error Added new example to test minio healthcheck with enableExternalRoute as true Fix Pre-commit errors Updated error message Updated Error message Updated as per comments Fixed test case issue Fix error statement Removed excess spaces
VaniHaripriya
force-pushed
the
issue-221
branch
from
956bb97
to
c4ace98
Compare
|
Change to PR detected. A new PR build was completed. |
| @@ -0,0 +1,26 @@ | |||
| apiVersion: datasciencepipelinesapplications.opendatahub.io/v1alpha1 | |||
There was a problem hiding this comment.
please put this in the v2 directory, you will need to rebase:
https://github.com/opendatahub-io/data-science-pipelines-operator/tree/main/config/samples/v2
| p.ObjectStorageConnection.ExternalRouteURL = route.Spec.Host | ||
| p.ObjectStorageConnection.Endpoint = route.Spec.Host | ||
| p.ObjectStorageConnection.Secure = util.BoolPointer(true) | ||
| p.ObjectStorageConnection.Host = route.Spec.Host |
There was a problem hiding this comment.
accessing route. here looks like it's prone to error because it hasn't been created yet, have you looked into just constructing the route host? it's always the same format:
https://${route_name}-${namespace}.${api-server-hostname}
route_name/namespace you already know, just check if we can easily figure out the api-server hostname, then you don't need to do a get for a route here
|
the comment above is not a big issue as this is mostly intended for development purposes (minio usecase) it's questionable to have the |
The issue resolved by this Pull Request:
Resolves RHOAIENG-1643
Description of your changes:
Leverage the Object store external route to perform the Health check.
Testing instructions
Deploy DSPO
Deploy DSPA instance using dspa_healthcheck.yaml file and check the logs to see if the Minio object store health check is successful.
Note: Please create a CA bundle to avoid custom cert issues while testing and update the same under apiServer section as show in the dspa_healthcheck.yaml file. Created configmap with ca cert as shown below:
kind: ConfigMap
apiVersion: v1
metadata:
name: my-ca
data:
ca.crt: |