Skip to content
This repository has been archived by the owner on May 6, 2022. It is now read-only.

Add feature flag for making instance/binding locks optional #1917

Merged
merged 1 commit into from
Apr 13, 2018
Merged

Add feature flag for making instance/binding locks optional #1917

merged 1 commit into from
Apr 13, 2018

Conversation

nilebox
Copy link
Contributor

@nilebox nilebox commented Apr 5, 2018

Extracting introduction of the new feature flag from #1872 to make reviews easier through small iterative PRs.

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Apr 5, 2018
Copy link
Contributor

@pmorie pmorie left a comment

Choose a reason for hiding this comment

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

LGTM - i believe this was hashed through in #1872

Copy link
Contributor

@MHBauer MHBauer left a comment

Choose a reason for hiding this comment

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

LGTM, but would prefer for @duglin to validate.

AsyncBindingOperations: {Default: false, PreRelease: utilfeature.Alpha},
NamespacedServiceBroker: {Default: false, PreRelease: utilfeature.Alpha},
ResponseSchema: {Default: false, PreRelease: utilfeature.Alpha},
OriginatingIdentityLocking: {Default: true, PreRelease: utilfeature.Alpha},
Copy link
Contributor

Choose a reason for hiding this comment

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

@duglin I think this is set up the way we want.

// If the OriginatingIdentityLocking feature is set then don't allow spec updates
// if processing of the current generation hasn't finished yet
if utilfeature.DefaultFeatureGate.Enabled(scfeatures.OriginatingIdentityLocking) {
if old.Generation != new.Generation && old.Status.ReconciledGeneration != old.Generation {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we get this changed so that it allow the request go go thru when its the same user? And the other check below too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't want to change the behavior in this PR, your change should be a follow-up to this one.

@duglin
Copy link
Contributor

duglin commented Apr 13, 2018

rebase needed

@MHBauer
Copy link
Contributor

MHBauer commented Apr 13, 2018

rebased, conflict was on feature gates between adding url update and adding this flag.

@duglin
Copy link
Contributor

duglin commented Apr 13, 2018

LGTM once tests pass

@duglin duglin added the LGTM2 label Apr 13, 2018
@nilebox nilebox merged commit 92dda3d into kubernetes-retired:master Apr 13, 2018
@nilebox nilebox deleted the lock-optional branch April 25, 2018 23:00
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. LGTM1 LGTM2 size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants