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

Fix crd data loss e2e test #3173

Conversation

perdasilva
Copy link
Collaborator

@perdasilva perdasilva commented Feb 9, 2024

What

Fixes CRD e2e flake "blocks a CRD upgrade that could cause data loss".

Cause

The test steps involve installing a bundle, then upgrading it to a bundle with a bad crd. The install plan will retry for 60 seconds before reaching a failed state.

Fix

Applying the operatorframework.io/bundle-unpack-timeout to the OG in the test namespace to reduce the retry timeout to 5 seconds (instead of 60) leading to faster failed states

Fixes #2638

@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 9, 2024
@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 9, 2024
@perdasilva perdasilva force-pushed the perdasilva/crd-e2e-dataloss-flake-fix branch from cc5d1d1 to f6d5239 Compare February 9, 2024 15:13
@perdasilva perdasilva changed the title [WIP] fix crd data loss e2e test Fix crd data loss e2e test Feb 9, 2024
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 9, 2024
ankitathomas
ankitathomas previously approved these changes Feb 9, 2024
Copy link
Contributor

@ankitathomas ankitathomas 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 Indicates that a PR is ready to be merged. label Feb 9, 2024
@grokspawn grokspawn added this pull request to the merge queue Feb 9, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Feb 9, 2024
tmshort
tmshort previously approved these changes Feb 9, 2024
Copy link
Contributor

@tmshort tmshort left a comment

Choose a reason for hiding this comment

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

/lgtm

@perdasilva perdasilva changed the title Fix crd data loss e2e test [WIP]Fix crd data loss e2e test Feb 9, 2024
@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 9, 2024
@perdasilva
Copy link
Collaborator Author

blocking merge as there might be a better way to solve this

Signed-off-by: Per Goncalves da Silva <pegoncal@redhat.com>
@perdasilva perdasilva dismissed stale reviews from tmshort and ankitathomas via c679200 February 12, 2024 09:06
@perdasilva perdasilva force-pushed the perdasilva/crd-e2e-dataloss-flake-fix branch from f6d5239 to c679200 Compare February 12, 2024 09:06
@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Feb 12, 2024
@perdasilva perdasilva changed the title [WIP]Fix crd data loss e2e test Fix crd data loss e2e test Feb 12, 2024
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 12, 2024
Copy link
Contributor

@tmshort tmshort left a comment

Choose a reason for hiding this comment

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

Different approach, changing the bundle unpack timeout - which is really what this is testing now.
/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Feb 12, 2024
Copy link

openshift-ci bot commented Feb 12, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ankitathomas, perdasilva, tmshort

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

@tmshort tmshort added this pull request to the merge queue Feb 12, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Feb 12, 2024
@tmshort tmshort added this pull request to the merge queue Feb 12, 2024
Merged via the queue into operator-framework:master with commit 85d9de2 Feb 12, 2024
15 of 16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FLAKE] blocks a CRD upgrade that could cause data loss
3 participants