Skip to content
This repository has been archived by the owner on Jan 31, 2024. It is now read-only.

Fix tests to support changes introduced by 1.5 Operator #787

Merged
merged 3 commits into from
Apr 20, 2023

Conversation

VaishnaviHire
Copy link
Member

@VaishnaviHire VaishnaviHire commented Apr 13, 2023

Description

  • The deployment name for the operator is updated to opendatahub-operator-controller-manager. Update the tests to use the new name.
  • DSP Operator tests are failing because, OpenShift Pipelines Operator is being installed in the namespace other than the KfDef namespace. This is not supported, since we have added ownerreferences to KfDef

How Has This Been Tested?

Merge criteria:

  • The commits are squashed in a cohesive manner and have meaningful messages.
  • 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

Copy link
Member

@harshad16 harshad16 left a comment

Choose a reason for hiding this comment

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

/lgtm
/approve
thanks 👍

@VaishnaviHire
Copy link
Member Author

@LaVLaS Can you review this. This will unblock the failing odh-manifests tests

@anishasthana
Copy link
Member

/retest

1 similar comment
@anishasthana
Copy link
Member

/retest

@harshad16
Copy link
Member

/test odh-manifests-e2e

@VaishnaviHire
Copy link
Member Author

DSP Operator tests are failing because, OpenShift Pipelines Operator is being installed in the namespace other than the KfDef namespace. I had opened #774 to fix it. Since these PRs are blocking one another, I am including both commits in this PR.

@VaishnaviHire VaishnaviHire changed the title Fix tests to use updated operator name Fix tests to support changes introduced by 1.5 Operator Apr 18, 2023
@LaVLaS
Copy link
Contributor

LaVLaS commented Apr 19, 2023

@VaishnaviHire Since the openshift-pipelines installation is just an OLM subscription, we can simplify the workflow by apllying the openshift-pipelines subscription as part of the tests/scripts/install.sh script and delete the openshift-pipelines test since that was from the older ODH releases where we bundled openshift-pipelines as a component

If you this PR will pass right now then we can keep the current working code and I will submit a follow-up PR to update that workflow

@VaishnaviHire
Copy link
Member Author

@VaishnaviHire Since the openshift-pipelines installation is just an OLM subscription, we can simplify the workflow by apllying the openshift-pipelines subscription as part of the tests/scripts/install.sh script and delete the openshift-pipelines test since that was from the older ODH releases where we bundled openshift-pipelines as a component

If you this PR will pass right now then we can keep the current working code and I will submit a follow-up PR to update that workflow

Yes, agreed. I will update the PR to directly use the subscription.

@VaishnaviHire VaishnaviHire force-pushed the fix_operator_tests branch 3 times, most recently from c9507e1 to 8a9ec2b Compare April 19, 2023 20:56
@VaishnaviHire
Copy link
Member Author

Failed due to issues creating test clusters.

/test odh-manifests-e2e

/test 411-odh-manifests-e2e

@anishasthana
Copy link
Member

/retest-required

@openshift-ci
Copy link

openshift-ci bot commented Apr 20, 2023

@VaishnaviHire: 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/49-images 0b54bd9 link true /test 49-images

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.

@VaishnaviHire
Copy link
Member Author

Expected error in ci/prow/odh-manifests-e2e for 4.12 cluster.

oc get-token.. is depricated. DS Pipelines tests need to be updated to fix this. I think we just need to remove the if condition in the linked test. I will can add additional commit in this PR.

Copy link
Member

@anishasthana anishasthana left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci openshift-ci bot added the lgtm label Apr 20, 2023
@anishasthana
Copy link
Member

/merge-status

@LaVLaS
Copy link
Contributor

LaVLaS commented Apr 20, 2023

/approve

@openshift-ci
Copy link

openshift-ci bot commented Apr 20, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: anishasthana, harshad16, LaVLaS

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:
  • OWNERS [LaVLaS,anishasthana]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@VaishnaviHire
Copy link
Member Author

Merging this since the tests passed

@VaishnaviHire VaishnaviHire merged commit 34bb613 into opendatahub-io:master Apr 20, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants