Skip to content

⚠️ Remove HasValidBundle condition - #990

Merged
m1kola merged 1 commit into
operator-framework:mainfrom
m1kola:remove_HasValidBundle_condition
Jun 28, 2024
Merged

⚠️ Remove HasValidBundle condition#990
m1kola merged 1 commit into
operator-framework:mainfrom
m1kola:remove_HasValidBundle_condition

Conversation

@m1kola

Copy link
Copy Markdown
Member

Description

Closes#989 by removing the condition and solves an E2E flake.

Note: in this brief were are revisiting ClusterExtension conditions and their reasons. One of the points is about removal of HasValidBundle condition type.

The brief is still in review, but I would like to move forward with this change as it resolves a flake I hit regularly (especially locally).

Reviewer Checklist

  • API Go Documentation
  • Tests: Unit Tests (and E2E Tests, if appropriate)
  • Comprehensive Commit Messages
  • Links to related GitHub Issue(s)

@openshift-ciopenshift-ciBot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 28, 2024
@netlify

netlifyBot commented Jun 28, 2024

Copy link
Copy Markdown

Deploy Preview for olmv1 ready!

NameLink
🔨 Latest commitd1bda18
🔍 Latest deploy loghttps://app.netlify.com/sites/olmv1/deploys/667eb2afa90d6a00086bfb8e
😎 Deploy Previewhttps://deploy-preview-990--olmv1.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@codecov

codecovBot commented Jun 28, 2024

Copy link
Copy Markdown

Codecov Report

Attention: Patch coverage is 0% with 1 line in your changes missing coverage. Please review.

Project coverage is 77.19%. Comparing base (cab41aa) to head (d1bda18).

FilesPatch %Lines
...nternal/controllers/clusterextension_controller.go0.00%1 Missing ⚠️
Additional details and impacted files
@@ Coverage Diff @@## main #990 +/- ##
==========================================
+ Coverage 76.89% 77.19% +0.29% 
==========================================
Files 17 17 Lines 1225 1206 -19 ==========================================
- Hits 942 931 -11 + Misses 201 193 -8 
Partials 82 82 
FlagCoverage Δ
e2e56.96% <0.00%> (+0.72%)⬆️
unit51.90% <0.00%> (-0.10%)⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

@m1kola
m1kola marked this pull request as ready for review June 28, 2024 12:33
@m1kola
m1kola requested a review from a team as a code ownerJune 28, 2024 12:34
@openshift-ciopenshift-ciBot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 28, 2024
@m1kola

Copy link
Copy Markdown
MemberAuthor

go-apidiff is expected to fail because I'm chasing the API here.

@m1kola
m1kola requested a review from bentitoJune 28, 2024 12:42
Comment threadtest/e2e/cluster_extension_install_test.go Outdated
Signed-off-by: Mikalai Radchuk <mradchuk@redhat.com>
@m1kola
m1kolaforce-pushed the remove_HasValidBundle_condition branch from 32fde13 to d1bda18CompareJune 28, 2024 12:55
@bentito

Copy link
Copy Markdown
Contributor

/lgtm

@openshift-ciopenshift-ciBot added the lgtm Indicates that a PR is ready to be merged. label Jun 28, 2024
@m1kolam1kola mentioned this pull request Jun 28, 2024
4 tasks

@ankitathomasankitathomas left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

/lgtm

@m1kola
m1kola added this pull request to the merge queue Jun 28, 2024
Merged via the queue into operator-framework:main with commit 97fb589Jun 28, 2024
@m1kola
m1kola deleted the remove_HasValidBundle_condition branch June 28, 2024 14:16
perdasilva pushed a commit to LalatenduMohanty/operator-controller that referenced this pull request Aug 13, 2024
Signed-off-by: Mikalai Radchuk <mradchuk@redhat.com>
@skattojuskattoju mentioned this pull request Sep 25, 2024
4 tasks
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lgtmIndicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ClusterExtension's condition HasValidBundle is False when Resolved, Installed and Unpacked are all True

3 participants

@m1kola@bentito@ankitathomas