-
Notifications
You must be signed in to change notification settings - Fork 107
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
tests: e2e: install servicemesh operators and check DSC(I) for it #833
tests: e2e: install servicemesh operators and check DSC(I) for it #833
Conversation
66063eb
to
2d837c3
Compare
/test opendatahub-operator-e2e |
b6a3ec2
to
bc187e4
Compare
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: asanzgom 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 |
tests/e2e/helper_test.go
Outdated
@@ -137,6 +154,22 @@ func setupDSCInstance() *dsc.DataScienceCluster { | |||
return dscTest | |||
} | |||
|
|||
func setupServicemeshSubscription(name string, ns string) *ofapi.Subscription { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we generalize these functions? Moving forward we might have multiple operators as dependencies. Redefining the functions should not be required.
Currently we have 3 dependent operators - Servicemesh, serverless and authorino
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bad naming :) It is actually not servicemesh specific.
bc187e4
to
48f6daa
Compare
New changes are detected. LGTM label has been removed. |
f5a5aa3
to
503f814
Compare
6eb750b
to
e2b35b1
Compare
/retest-required |
Make testDSCCreation test to wait for DSC instance to be in "Ready" phase. Otherwise if it's initialization delayed other tests may fail. The problem appeared during experiments of enabling kserve component and installing it's prerequisites (serverless/servicemesh operators). Add tc.wait() wrapper on PollUntilContextTimeout to simplify the call. Set 'immediate' to true, no need to wait 10s before first check. Signed-off-by: Yauheni Kaliuta <ykaliuta@redhat.com>
Adding servicemesh opertators installation and checks increases the overall test suite time, so increase the timeout. Signed-off-by: Yauheni Kaliuta <ykaliuta@redhat.com>
Introduce setUp() method for testContext to install dependency operators, - install servicemeshoperator and serverless-operator - enable servicemesh in DSCI - check that DSCI and DSC reflect the configuration change. Since it is supposed that user install them (and user removes) it is not implemented as additional tests but setUp. Probably would be fine to have proper uninstallation in tearDown in case the test installed them but removing of Subscriptions and CSVs is not enough (checked, got broken setup) and implementation of proper cleanup does not make sense at the moment. To work with client.Client methods add github.com/operator-framework/api/pkg/operators/v1alpha1 types to the scheme. Update Kserve check, it does not need special case (error "Please install the operator before enabling component") handling since its dependencies are installed. DSCI/DSC validation code checks only servicemesh related parts mostly for simplicity. But there is no any more special configuration there to get unexpected values. Spec values in setupSubscription() which creates an instance of Subscription structure are basically predefined, the operators are provided by redhat-operators catalog, must be installed into openshift-operators namespace and so on. InstallPlanApproval was added to workaround CI problems (see below) but did not help alone. In the CI the way it installs opendatahub-operator makes its and following Subscriptions manually approved. As the result creating Subscription objects for the operators does not cause operators installation, it hungs waiting for InstallPlan approvement. To fix it the code finds the InstallPlans and patches (server-apply patching is used) it to get approved. It requires Force option since the fields belong to 'catalog'. ClusterServiceVersionNames is copied from the original object otherwise server complains about missing field. It also required Kind and APIVersion. ensureServicemeshOperators() takes *testing.T just for logging. Channel is used instead of waitGroup to collect errors from every operator installation which are done in parallel to speed up the process. Signed-off-by: Yauheni Kaliuta <ykaliuta@redhat.com>
e2b35b1
to
e7160f5
Compare
/lgtm |
/retest-required |
02dad4a
into
opendatahub-io:incubation
…endatahub-io#833) * tests: e2e: dsc creation: wait until DSC is "Ready" Make testDSCCreation test to wait for DSC instance to be in "Ready" phase. Otherwise if it's initialization delayed other tests may fail. The problem appeared during experiments of enabling kserve component and installing it's prerequisites (serverless/servicemesh operators). Add tc.wait() wrapper on PollUntilContextTimeout to simplify the call. Set 'immediate' to true, no need to wait 10s before first check. Signed-off-by: Yauheni Kaliuta <ykaliuta@redhat.com> * tests: e2e: increase overall timeout to 20m in the Makefile Adding servicemesh opertators installation and checks increases the overall test suite time, so increase the timeout. Signed-off-by: Yauheni Kaliuta <ykaliuta@redhat.com> * tests: e2e: install servicemesh opertators before test run Introduce setUp() method for testContext to install dependency operators, - install servicemeshoperator and serverless-operator - enable servicemesh in DSCI - check that DSCI and DSC reflect the configuration change. Since it is supposed that user install them (and user removes) it is not implemented as additional tests but setUp. Probably would be fine to have proper uninstallation in tearDown in case the test installed them but removing of Subscriptions and CSVs is not enough (checked, got broken setup) and implementation of proper cleanup does not make sense at the moment. To work with client.Client methods add github.com/operator-framework/api/pkg/operators/v1alpha1 types to the scheme. Update Kserve check, it does not need special case (error "Please install the operator before enabling component") handling since its dependencies are installed. DSCI/DSC validation code checks only servicemesh related parts mostly for simplicity. But there is no any more special configuration there to get unexpected values. Spec values in setupSubscription() which creates an instance of Subscription structure are basically predefined, the operators are provided by redhat-operators catalog, must be installed into openshift-operators namespace and so on. InstallPlanApproval was added to workaround CI problems (see below) but did not help alone. In the CI the way it installs opendatahub-operator makes its and following Subscriptions manually approved. As the result creating Subscription objects for the operators does not cause operators installation, it hungs waiting for InstallPlan approvement. To fix it the code finds the InstallPlans and patches (server-apply patching is used) it to get approved. It requires Force option since the fields belong to 'catalog'. ClusterServiceVersionNames is copied from the original object otherwise server complains about missing field. It also required Kind and APIVersion. ensureServicemeshOperators() takes *testing.T just for logging. Channel is used instead of waitGroup to collect errors from every operator installation which are done in parallel to speed up the process. Signed-off-by: Yauheni Kaliuta <ykaliuta@redhat.com> --------- Signed-off-by: Yauheni Kaliuta <ykaliuta@redhat.com>
Description
https://issues.redhat.com/browse/RHOAIENG-489
Introduce setUp() method for testContext to install dependency operators,
Since it is supposed that user install them (and user removes) it is
not implemented as additional tests but setUp.
How Has This Been Tested?
Merge criteria: