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.

Sign up for GitHub

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

Jump to bottom

[v2] update to SBO library to 1.0 #5446

Merged
merged 7 commits into from
Mar 1, 2022
Merged

[v2] update to SBO library to 1.0 #5446

merged 7 commits into from
Mar 1, 2022

Conversation

feloy
Copy link
Contributor

@feloy feloy commented Feb 9, 2022

Fixes #5221

If you delete the Service Binding Operator from your OpenShift cluster, IsResourceSupported continues to detect it => in this circumstance, odo will create sbr for binding, and will timeout waiting for deployment to be restarted after the sbr is created. SOLUTION: delete servicebindings CRDs manually

@feloy feloy mentioned this pull request Feb 9, 2022
Merged
3 tasks
@feloy feloy changed the title [v2] update to SBO library to 1.0 [wip] [v2] update to SBO library to 1.0 Feb 9, 2022
@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. Required by Prow. label Feb 9, 2022
@kadel
Copy link
Member

kadel commented Feb 11, 2022

Talked to SBO team about redhat-developer/service-binding-operator#1088 they won't have the bandwidth to look into redhat-developer/service-binding-operator#1088 in the next 2 sprints (probably even more). It looks like we will have to implement one of the workarounds on our side.

The easiest workaround on our side could be simply updating our tests to include creation of a Secret and defining kubernetesConfig.redisSecret.name in Redis CR

@kadel kadel closed this Feb 11, 2022
@kadel kadel reopened this Feb 11, 2022
@kadel
Copy link
Member

kadel commented Feb 11, 2022

@kadel kadel closed this 15 seconds ago
@kadel kadel reopened this 11 seconds ago

😇 wrong button, sorry 😊

@sonarcloud
Copy link

sonarcloud bot commented Feb 11, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@feloy
Copy link
Contributor Author

feloy commented Feb 11, 2022

Talked to SBO team about redhat-developer/service-binding-operator#1088 they won't have the bandwidth to look into redhat-developer/service-binding-operator#1088 in the next 2 sprints (probably even more). It looks like we will have to implement one of the workarounds on our side.

The easiest workaround on our side could be simply updating our tests to include creation of a Secret and defining kubernetesConfig.redisSecret.name in Redis CR

OK, this is actually what is done in this PR. I'll mark the PR as ready for review

Thanks @kadel

@feloy feloy changed the title [wip] [v2] update to SBO library to 1.0 [v2] update to SBO library to 1.0 Feb 11, 2022
@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. Required by Prow. label Feb 11, 2022
@kadel
Copy link
Member

kadel commented Feb 11, 2022

OK, this is actually what is done in this PR. I'll mark the PR as ready for review

oh, my bad. I should have checked the code of this PR

kadel
kadel reviewed Feb 11, 2022
View reviewed changes
Copy link
Member

@kadel kadel left a comment

Choose a reason for hiding this comment

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

/approve

tests/integration/operatorhub/cmd_service_test.go
@@ -169,6 +169,8 @@ var _ = Describe("odo service command tests for OperatorHub", func() {
"-p", "kubernetesConfig.serviceType=ClusterIP",
"-p", "kubernetesConfig.resources.requests.cpu=100m",
"-p", "kubernetesConfig.resources.requests.memory=128Mi",
"-p", "kubernetesConfig.redisSecret.name=redis-secret",
Copy link
Member

Choose a reason for hiding this comment

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

👍

@openshift-ci
Copy link

openshift-ci bot commented Feb 11, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: kadel

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 Indicates a PR has been approved by an approver from all required OWNERS files. Required by Prow. label Feb 11, 2022
@feloy feloy added the v2 Issue or PR that applies to the v2 of odo label Feb 23, 2022
@feloy
Copy link
Contributor Author

feloy commented Feb 24, 2022

/retest

@dharmit
Copy link
Member

dharmit commented Mar 1, 2022

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. Required by Prow. label Mar 1, 2022
@openshift-merge-robot openshift-merge-robot merged commit 4922010 into redhat-developer:v2 Mar 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kadel kadel kadel left review comments

@dharmit dharmit Awaiting requested review from dharmit

@rnapoles-rh rnapoles-rh Awaiting requested review from rnapoles-rh

Assignees

@dharmit dharmit

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. Required by Prow. lgtm Indicates that a PR is ready to be merged. Required by Prow. v2 Issue or PR that applies to the v2 of odo
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@feloy @kadel @dharmit @openshift-merge-robot