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

disable olm descriptor support #1251

Merged
merged 1 commit into from
Oct 7, 2022
Merged

disable olm descriptor support #1251

merged 1 commit into from
Oct 7, 2022

Conversation

sadlerap
Copy link
Contributor

@sadlerap sadlerap commented Oct 5, 2022

This was causing performance issues under OCP 4.11.

Signed-off-by: Andy Sadler ansadler@redhat.com

Changes

/kind enhancement

Submitter Checklist

As the author of this PR, please check off the items in this checklist:

  • Docs
    included if any changes are user facing
  • Tests
    included if any functionality added or changed. For bugfixes please include tests that can catch regressions
  • All acceptance test scenarios included in the PR which verifies a bugfix or a requested feature reported by a non-member are tagged with @external-feedback tag.
  • Follows the commit message standard

@openshift-ci openshift-ci bot added the kind/enhancement New feature or request label Oct 5, 2022
@sadlerap
Copy link
Contributor Author

sadlerap commented Oct 5, 2022

/test performance

@pmacik pmacik added release/v1.2.x Used to mark PRs to be cherry-picked in release-v1.2.x branch release/v1.3.x Used to mark PRs to be cherry-picked in release-v1.3.x branch labels Oct 5, 2022
@sadlerap sadlerap mentioned this pull request Oct 5, 2022
Closed
4 tasks
@pmacik
Copy link
Contributor

pmacik commented Oct 5, 2022

Recently, Crunchy Postgres Operator test started to fail on OCP 4.12.. working on fixing that in #1250

dperaza4dustbit
dperaza4dustbit reviewed Oct 5, 2022
View reviewed changes
Copy link
Contributor

@dperaza4dustbit dperaza4dustbit left a comment

Choose a reason for hiding this comment

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

This looks OK to me but was not expecting all the code to be removed just disabled. What changed?

test/acceptance/features/bindAppToServiceUsingSecret.feature
Then Service Binding is ready
And The application env var "BACKEND_USERNAME" has value "AzureDiamond"
And The application env var "BACKEND_PASSWORD" has value "hunter2"

Copy link
Contributor

Choose a reason for hiding this comment

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

Was not expecting to remove all these test, just disable them, in case we need to bring OLM Descriptor feature back. @pmacik and @sadlerap how easy would be to bring this back if we need to?

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's just a git revert away.

@pmacik pmacik added this to the 1.3.0 milestone Oct 6, 2022
@sadlerap @pmacik
disable olm descriptor support
ecca6f0 
This was causing performance issues under OCP 4.11.

Signed-off-by: Andy Sadler <ansadler@redhat.com>
@pmacik pmacik force-pushed the remove-olm-descriptors branch from 5a7ca05 to ecca6f0 Compare October 6, 2022 13:27
@pmacik pmacik removed the release/v1.2.x Used to mark PRs to be cherry-picked in release-v1.2.x branch label Oct 6, 2022
@pmacik
Copy link
Contributor

pmacik commented Oct 6, 2022

/test performance

@pmacik
Copy link
Contributor

pmacik commented Oct 6, 2022

/retest

@pmacik
Copy link
Contributor

pmacik commented Oct 6, 2022

/lgtm
/approve

kpi:
- name: usage
  metrics:
  - average: 12.03225806451613
    first: 1.0
    last: 34.0
    maximum: 86.0
    median: 5.0
    minimum: 0.0
    name: CPU_millicores
  
  - average: 108.09925558312655
    first: 49.0
    last: 148.0
    maximum: 149.0
    median: 112.0
    minimum: 49.0
    name: Memory_MiB

@openshift-ci openshift-ci bot added the lgtm label Oct 6, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Oct 6, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: pmacik

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-ci openshift-ci bot added the approved label Oct 6, 2022
@pmacik
Copy link
Contributor

pmacik commented Oct 7, 2022

/retest

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Oct 7, 2022

@sadlerap: 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/4.12-acceptance ecca6f0 link true /test 4.12-acceptance

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.

@pmacik
Copy link
Contributor

pmacik commented Oct 7, 2022

Manually tested with 4.12 and the tests passed. Merging.

@pmacik pmacik merged commit 8cbabbb into redhat-developer:master Oct 7, 2022
pmacik pushed a commit that referenced this pull request Oct 7, 2022
@sadlerap @pmacik
disable olm descriptor support (#1251)
c23a4ae 
This was causing performance issues under OCP 4.11.

Signed-off-by: Andy Sadler <ansadler@redhat.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@dperaza4dustbit dperaza4dustbit dperaza4dustbit left review comments

@shruthihub shruthihub Awaiting requested review from shruthihub

Assignees

@pmacik pmacik

Labels
approved kind/enhancement New feature or request lgtm release/v1.3.x Used to mark PRs to be cherry-picked in release-v1.3.x branch
Projects
None yet
Milestone
1.3.0
Development

Successfully merging this pull request may close these issues.
3 participants
@sadlerap @pmacik @dperaza4dustbit