-
Notifications
You must be signed in to change notification settings - Fork 367
TNZ-69318 Persist and present broker_provided_metadata #4699
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
TNZ-69318 Persist and present broker_provided_metadata #4699
Conversation
|
|
4a66b6c to
e6a2725
Compare
|
@sethboyles these failing Backward Compatibility Unit Tests do not seem related to my changes? I even start thinking we might merge this as this change :
|
|
@atanas-attodorov-wq I'm currently digging into the backward compatibility tests to see if I can understand what's going wrong there. @beyhan do you have concerns about implementing an OSBAPI 2.17 feature before we've implemented 2.16? Or do you think it's OK to add some of the features like this one as they are requested? |
TNZ-69318 Persist and present broker_provided_metadata TNZ-69318 Persist and present broker_provided_metadata TNZ-69318 Persist and present broker_provided_metadata TNZ-69318 Persist and present broker_provided_metadata TNZ-69318 Persist and present broker_provided_metadata Add unit tests Add unit tests Remove Trailing spaces TNZ-70252 Apply code review comments TNZ-70252 Revert back column type to text TNZ-70252 add text column size constraint TNZ-70252 add text column size constraint
cadea3d to
9ac03d7
Compare
|
@atanas-attodorov-wq I believe your branch was failing the Backwards Compatibility tests because it was behind main and was missing some migrations. I've rebased the branch off of main so hopefully they will pass now. |
Thanks Seth, |
I think it's OK to add features as they are requested if they provide value, as long as the proposed solution aligns with how we want to adopt 2.16 and 2.17 overall. If the feature is self-contained and doesn't create architectural conflicts or technical debt that will need significant refactoring later, then implementing it out of order shouldn't be a problem. During my presentation at CF Day Europe, I mentioned that I'm looking for an approach to "register" and "deregister" existing service instances to a CF foundation, which may also require implementing OSBAPI features that don't even exist yet in the current OSBAPI version. Of course, this would depend on whether the community sees value in these new features. |
|
Isn't this note in 2.16 OSBAPI spec defining part of this feature? https://github.com/cloudfoundry/servicebroker/blob/v2.16/spec.md#service-instance-metadata It seems to me 2.16 introduced I'm OK with merging this. I'll approve and wait for others to take a look at implementation before actually merging. |
Changes in cloud_controller_ng:
- TNZ-69318 Persist and present broker_provided_metadata
PR: cloudfoundry/cloud_controller_ng#4699
Author: atanas-attodorov-wq <atanas-at.todorov@broadcom.com>
This implementation adds support for persisting and displaying broker-provided metadata from OSBAPI responses for managed service instances. The metadata is stored separately from user-specified metadata (labels/annotations) and extracted from both synchronous and asynchronous provision/update operations.
This implementation adds support for persisting and displaying broker-provided metadata from OSBAPI responses for managed service instances.
This change:
It is clear that Cloud Foundry do not fully adopt OSBAPI 2.17 and is still on OSBAPI 2.15 but this is one step forward.
Here is a link to discussion:
#4633
Links to any other associated PRs
Extend GenerateManifest output model to all retrun SB specific metadata pivotal-cf/on-demand-services-sdk#264
Propagate tags from generated by service adapter Bosh Manifest into Metadata part of ProvisionedServiceSpec pivotal-cf/on-demand-service-broker#439
[Yes ] I have reviewed the contributing guide
[ Yes] I have viewed, signed, and submitted the Contributor License Agreement
[ Yes] I have made this pull request to the
mainbranch[ Yes] I have run all the unit tests using
bundle exec rake[No] I have run CF Acceptance Tests