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

Bug 1576728 - Fixes multiple requests to a CR #947

Merged
merged 2 commits into from May 15, 2018

Conversation

shawn-hurley
Copy link
Contributor

@shawn-hurley shawn-hurley commented May 11, 2018

Describe what this PR does and why we need it:

This makes the dao calls happen 1 at a time for a dao. Attempts to handle the case when two service instances are gotten with two bindings. both are deleted, but because we only remove each of one, and when we update we just set the value, we were adding back a binding.

Depends on automationbroker/bundle-lib#80

@openshift-ci-robot openshift-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label May 11, 2018
Copy link
Contributor

@jmrodri jmrodri left a comment

Choose a reason for hiding this comment

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

Other than the version in the Gopkg, looks good.

Gopkg.lock Outdated
@@ -44,6 +44,7 @@
version = "0.1.2"

[[projects]]
branch = "bug-1576728"
Copy link
Contributor

Choose a reason for hiding this comment

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

will this remain when we merge?

Gopkg.toml Outdated
version = "~0.1.0"
#version = "~0.1.0"
branch = "bug-1576728"
source = "github.com/shawn-hurley/bundle-lib"
Copy link
Contributor

Choose a reason for hiding this comment

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

are we doing this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, this depends on this automationbroker/bundle-lib#80

bundleLock: sync.Mutex{},
bindingLock: sync.Mutex{},
instanceLock: sync.Mutex{},
}
Copy link
Contributor

Choose a reason for hiding this comment

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

CRDs suck as a database :)

@shawn-hurley shawn-hurley added the 3.10 | release-1.2 Kubernetes 1.10 | Openshift 3.10 | Broker release-1.2 label May 15, 2018
Copy link
Contributor

@eriknelson eriknelson left a comment

Choose a reason for hiding this comment

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

ACK, discussed with @shawn-hurley and @jmrodri. Determined best course of action for 3.10 until a longer term fix can be employed in 3.11.

@shawn-hurley can we file an issue that describes why this isn't the desired long-term fix and potential avenues to explore for a longer term fix?

Also, these Gopkg refs need to be updated after we cut a bundle-lib release with the related fix, correct?

namespace string
client automationbrokerv1.AutomationbrokerV1alpha1Interface
namespace string
bundleLock sync.Mutex
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@rthallisey
Copy link
Contributor

automationbroker/bundle-lib#80 just merged. Should be able to remove branch = "bug-1576728" everywhere

@eriknelson
Copy link
Contributor

@rthallisey I think we probably want to update the Gopkg stuff to reference a release tag of bundle-lib.

@openshift-ci-robot openshift-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels May 15, 2018
@shawn-hurley shawn-hurley changed the title Bug 1576728 - this is an attempt to fix the bug Bug 1576728 - Fixes multiple requests to a CR May 15, 2018
@eriknelson
Copy link
Contributor

This is ready to merge with the bundle lib release.

@eriknelson eriknelson merged commit 7e70688 into openshift:master May 15, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.10 | release-1.2 Kubernetes 1.10 | Openshift 3.10 | Broker release-1.2 size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants