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

e2e: add cleanup logic to kserve tests to ensure no KnativeServing is present in the cluster #1517

Merged

Conversation

lburgazzoli
Copy link
Contributor

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

@lburgazzoli lburgazzoli requested a review from grdryn January 18, 2025 14:20
@openshift-ci openshift-ci bot requested review from biswassri and ykaliuta January 18, 2025 14:20
@@ -26,6 +28,7 @@ func kserveTestSuite(t *testing.T) {
ComponentTestCtx: ct,
}

t.Run("Validate environment", componentCtx.validateEnv)
Copy link
Contributor

Choose a reason for hiding this comment

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

May be it should be part of setUp()? I do not see a lot of sense to use assertions there.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

you mean in the kserveTestSuite ?

Copy link
Contributor

Choose a reason for hiding this comment

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

I do not have strong opinion. It may make sense to limit to the particular test if it does not matter for others. Allows to skip unnecessary work if the test is skipped. On the other hand it may be part of general preparation of the environment. Originally I was thinking about one global place. There is one https://github.com/opendatahub-io/opendatahub-operator/blob/main/tests/e2e/helper_test.go#L459 which is called from creation suite. With the changes it also can be reconsidered.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it make sense, I'll have a look

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Copy link

codecov bot commented Jan 18, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 19.69%. Comparing base (9b062f6) to head (c9e1330).
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1517   +/-   ##
=======================================
  Coverage   19.69%   19.69%           
=======================================
  Files         161      161           
  Lines       11102    11102           
=======================================
  Hits         2187     2187           
  Misses       8683     8683           
  Partials      232      232           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link

openshift-ci bot commented Jan 18, 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

@openshift-merge-bot openshift-merge-bot bot merged commit 72443c5 into opendatahub-io:main Jan 18, 2025
10 checks passed
@lburgazzoli lburgazzoli deleted the kserver-knative-serving branch January 18, 2025 19:42
zdtsw pushed a commit to zdtsw-forking/opendatahub-operator that referenced this pull request Feb 3, 2025
… present in the cluster (opendatahub-io#1517)

* e2e: add cleanup logic to kserve tests to ensure no KnativeServing is present in the cluster

* Fix findings

* lint

(cherry picked from commit 72443c5)
zdtsw added a commit that referenced this pull request Feb 4, 2025
* Fix typo in file name: pkg/utils/test/matchers/matechers.go -> pkg/utils/test/matchers/matchers.go (#1505)

(cherry picked from commit 68c6cae)

* e2e: improve testOwnedNamespacesAllExist by leveragin gomega's async assertions (#1509)

When the e2e test suite starts, it may take a while for some of the
owned namespace to become available and in such case the related test
would fail, since it does not retry. This commit replace the current
synchronous test with an asynchronous one.

(cherry picked from commit c35c6c1)

* Improve unit tests for jq's toType and byteToType (#1506)

- Refactor the toType test to handle more cases
- Add additional cases to toTypes test
- Add a specific test for byteToType

(cherry picked from commit e1457ab)

* e2e: add cleanup logic to kserve tests to ensure no KnativeServing is present in the cluster (#1517)

* e2e: add cleanup logic to kserve tests to ensure no KnativeServing is present in the cluster

* Fix findings

* lint

(cherry picked from commit 72443c5)

* Revert "e2e: add cleanup logic to kserve tests to ensure no KnativeServing is present in the cluster" (#1522)

* Revert "e2e: add cleanup logic to kserve tests to ensure no KnativeServing is…"

This reverts commit 72443c5.

* Fix after rebase

(cherry picked from commit 6a762ad)

* Add support for dynamic template data computation in template rendering action (#1527)

(cherry picked from commit bf3c108)

* chore: remove generics from Reconcier and use PlatformObject as base object type (#1514)

- remove generics from Reconciler struct as it is not needed
- set PlatformObject as the base obnject that the Reconciler handles to
  ensure the reconciler can access to common objects fields

(cherry picked from commit 94ac12c)

* chore: remove unused structs and functions from controlelrs/status package (#1535)

(cherry picked from commit 614fea1)

* chore: remove unused code (#1543)

* chore: remove unused TrimToRFC1123Name function

* chore: remove unused pkg/cluster/roles functions

* chore: remove unused pkg/cluster/resources functions

(cherry picked from commit 8c7fef2)

* chore: use constants from the status package where possible (#1541)

Co-authored-by: Wen Zhou <[email protected]>
(cherry picked from commit d538956)

* Improve Kserve's FeatureTraker handing (#1562)

* Improve Kserve's FeatureTraker handing

- Make it possible to set FT's OwnerReference as Controller reference so
  the kubernetes garbage collector can block owner deletion till the FT
  has been deleted
- Make it possible to set FT's generated resources OwnerReference as
  Controller reference so the kubernetes garbage collector can block
  FT deletion till the resources have been deleted
- Add an Kserver reconciler action to remove legacy ownership on
  DSCI/DSC is any and related e2e tests

* Update RBACs

* Update RBACs

* Fix findings

* Update pkg/cluster/meta.go

Co-authored-by: Gerard Ryan <[email protected]>

* Fix renaming

* Remove unrelated methods

---------

Co-authored-by: Gerard Ryan <[email protected]>
(cherry picked from commit bd00c9b)

---------

Co-authored-by: Luca Burgazzoli <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants