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

test: add install for Authroino operator #1519

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

zdtsw
Copy link
Member

@zdtsw zdtsw commented Jan 18, 2025

  • without authorino installed, we have functions for dependency are not tested

Description

How Has This Been Tested?

Screenshot or short clip

Merge criteria

  • You have read the contributors guide.
  • Commit messages are meaningful - have a clear and concise summary and detailed explanation of what was changed and why.
  • Pull Request contains a description of the solution, a link to the JIRA issue, and to any dependent or related Pull Request.
  • Testing instructions have been added in the PR body (for PRs involving changes that are not immediately obvious).
  • The developer has manually tested the changes and verified that the changes work

@zdtsw
Copy link
Member Author

zdtsw commented Jan 19, 2025

/test opendatahub-operator-e2e

@@ -436,6 +437,7 @@ func ensureServicemeshOperators(t *testing.T, tc *testContext) error { //nolint:
ops := []string{
serverlessOpName,
servicemeshOpName,
authorinoOpName,
Copy link
Contributor

Choose a reason for hiding this comment

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

I created that time bomb, but probably makes sense to rename ensureServicemeshOperators to ensureDependencyOperators or something (did @VaishnaviHire already do it somewhere?)

Copy link
Member Author

Choose a reason for hiding this comment

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

updated the fuction name to ensureDependencyOperators

@biswassri
Copy link
Contributor

/test opendatahub-operator-e2e

@biswassri
Copy link
Contributor

biswassri commented Jan 21, 2025

Look good to me. Not adding lgtm since it's do-not-merge

@openshift-ci openshift-ci bot added the lgtm label Jan 27, 2025
Copy link

openshift-ci bot commented Jan 27, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ykaliuta

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

@zdtsw
Copy link
Member Author

zdtsw commented Jan 27, 2025

i probably need to get openshift/release#60470 out first,
i am not 100% sure but looks like authorino stable channel might not be visible on old ocp version.

- without authorino installed, we have functions for dependency are not tested

Signed-off-by: Wen Zhou <[email protected]>
Copy link

openshift-ci bot commented Feb 13, 2025

New changes are detected. LGTM label has been removed.

@zdtsw
Copy link
Member Author

zdtsw commented Feb 13, 2025

with authorino publish their statble release on 4.14-4.15 ocp, this should work without openshift/release#60470

@@ -36,6 +36,7 @@ const (
servicemeshNamespace = "openshift-operators"
Copy link
Contributor

Choose a reason for hiding this comment

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

not for this PR, but we should probably have a namespace per operator, since the default namespace suggested by specific operators may be different, i.e. for serverless it is openshift-serverless

Copy link
Contributor

Choose a reason for hiding this comment

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

Not related as well, but I am wondering why one of them toggles "Installing" (I looked the console) when if it's installed it should not change the configuration.

Copy link
Member Author

Choose a reason for hiding this comment

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

Not related as well, but I am wondering why one of them toggles "Installing" (I looked the console) when if it's installed it should not change the configuration.
so you already have operatro installed, then run e2e and operator flipped from installed->installing->installed ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, Succeed -> Installing -> Succeed. Not for you?

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, rechecked now, did not observe. Some glitch in old cluster may be.

Copy link

openshift-ci bot commented Feb 13, 2025

@zdtsw: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/opendatahub-operator-e2e 106fec0 link true /test opendatahub-operator-e2e

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-sigs/prow repository. I understand the commands that are listed here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Todo
Development

Successfully merging this pull request may close these issues.

4 participants