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

reenable the controller default relist interval #2125

Merged
merged 2 commits into from
Jun 25, 2018
Merged

reenable the controller default relist interval #2125

merged 2 commits into from
Jun 25, 2018

Conversation

MHBauer
Copy link
Contributor

@MHBauer MHBauer commented Jun 15, 2018

  • dump apiserver defaults + validation
  • dump checks in controller for a set value

resolves #2119

/cc @duglin
/assign @duglin

@k8s-ci-robot k8s-ci-robot requested a review from duglin June 15, 2018 19:49
@k8s-ci-robot k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Jun 15, 2018
 - dump apiserver defaults + validation
 - dump checks in controller for a set value
)
}

if spec.RelistBehavior == sc.ServiceBrokerRelistBehaviorManual && spec.RelistDuration != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the defined behavior for a manual relist? I don't quite understand why your change makes it okay to say manual and set a duration.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Manual is "only respond to an increase in the relist-requests, no automated pinging of the broker"

My opinion is that this causes unecessary friction if switching from duration to manual, and that while it doesn't make much sense to explicitly specify manual and duration, it shouldn't be a hard error. Manual should override everything else.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, that makes sense thanks! 👍

Copy link
Contributor

@carolynvs carolynvs left a comment

Choose a reason for hiding this comment

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

LGTM

@duglin
Copy link
Contributor

duglin commented Jun 25, 2018

LGTM

@duglin duglin merged commit ea84fa5 into kubernetes-retired:master Jun 25, 2018
kikisdeliveryservice pushed a commit to kikisdeliveryservice/service-catalog that referenced this pull request Jun 29, 2018
)

* reenable the controller default relist interval

 - dump apiserver defaults + validation
 - dump checks in controller for a set value

* fixup unit tests due to changed behavior
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 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.

Change default relist and add a config flag
4 participants