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

ExternalID selectors for instances and bindings #2155

Merged
merged 3 commits into from
Jun 26, 2018
Merged

ExternalID selectors for instances and bindings #2155

merged 3 commits into from
Jun 26, 2018

Conversation

nilebox
Copy link
Contributor

@nilebox nilebox commented Jun 25, 2018

Fixes #2124

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jun 25, 2018
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approver: duglin

Assign the PR to them by writing /assign @duglin in a comment when ready.

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

@k8s-ci-robot k8s-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jun 25, 2018
staebler
staebler approved these changes Jun 25, 2018
View reviewed changes
Copy link
Contributor

@staebler staebler left a comment

Choose a reason for hiding this comment

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

/lgtm

pkg/registry/servicecatalog/binding/storage.go
objectMetaFieldsSet := generic.ObjectMetaFieldsSet(&binding.ObjectMeta, true)
return generic.MergeFieldsSets(objectMetaFieldsSet, nil)

specFieldSet := make(fields.Set, 1)
Copy link
Contributor

Choose a reason for hiding this comment

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

I was about to comment on this being 3 instead of 1. Really, this should be 1 + len(objectMetaFieldSet, and the function should be merging objectMetaFieldSet into specFieldSet instead of the other way around. However, that is beyond the scope of this PR.

@k8s-ci-robot k8s-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jun 25, 2018
@k8s-ci-robot k8s-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Jun 25, 2018
@nilebox
Copy link
Contributor Author

nilebox commented Jun 25, 2018

@staebler sorry, I forgot to register the new ServiceBindingFieldLabelConversionFunc function. Can you please re-lgtm?

@staebler
Copy link
Contributor

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jun 25, 2018
@jberkhahn
Copy link
Contributor

This seems reasonable to me.

@k8s-ci-robot
Copy link
Contributor

New changes are detected. LGTM label has been removed.

@k8s-ci-robot k8s-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Jun 26, 2018
@nilebox nilebox added the LGTM1 label Jun 26, 2018
@nilebox
Copy link
Contributor Author

nilebox commented Jun 26, 2018

Has the process for lgtm/LGTM1/2 changed? Adding LGTM1 just in case, need one more approval.

@MHBauer
Copy link
Contributor

MHBauer commented Jun 26, 2018

@nilebox not yet, we're getting there. need to get prow running CI
see kubernetes/test-infra#8291
new plugins are enabled and I would like to see people getting used to them.

carolynvs
carolynvs approved these changes Jun 26, 2018
View reviewed changes
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

@carolynvs carolynvs merged commit a94b774 into kubernetes-retired:master Jun 26, 2018
@nilebox nilebox deleted the externalid-selector branch June 26, 2018 12:20
kikisdeliveryservice pushed a commit to kikisdeliveryservice/service-catalog that referenced this pull request Jun 29, 2018
@nilebox @kikisdeliveryservice
ExternalID selectors for instances and bindings (kubernetes-retired#2155
f554821 
)

* ExternalID selectors for instances and bindings

* Register ServiceBinding filter conversion function

* gofmt
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@staebler staebler staebler approved these changes

@carolynvs carolynvs carolynvs approved these changes

@MHBauer MHBauer Awaiting requested review from MHBauer

Assignees
No one assigned
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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add ability to filter instances and bindings by externalID
6 participants
@nilebox @k8s-ci-robot @staebler @jberkhahn @MHBauer @carolynvs