This repository has been archived by the owner on Jun 26, 2024. It is now read-only.
SBO design direction with multiple services support #286
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
It is expected the Secret to not exist if the binding hasn't been performed yet, so the API was changed to support this use case. Additionally, found is introduced to avoid importing "errors" and Kubernetes own errors package.
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
@isutton: PR needs rebase. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
Validation of the output secret is not yet being performed.
sbose78
reviewed
Jan 6, 2020
| Version string `json:"version"` | ||
| Kind string `json:"kind"` | ||
| ResourceRef string `json:"resourceRef"` | ||
| schema.GroupVersionKind `json:",inline" yaml:",inline"` |
There was a problem hiding this comment.
@isutton Could I assign https://issues.redhat.com/browse/APPSVC-324 to you
CC @baijum ?
There was a problem hiding this comment.
Can we do this change in a separate PR merge it first?
|
What is the plan with this PR ? |
added 3 commits
January 14, 2020 14:05
With the current version specified in go.mod, the following error happens: $ go mod vendor go: github.com/openshift/api@v3.9.1-0.20190424152011-77b8897ec79a+incompatible: invalid pseudo-version: preceding tag (v3.9.0) not found This patch replaces it with a valid pseudo-version encoding the same commit specified in the original.
This is in preparation to check-out those files in master before changing existing code.
isutton
pushed a commit
to isutton/service-binding-operator
that referenced
this pull request
Feb 7, 2020
isutton
pushed a commit
to isutton/service-binding-operator
that referenced
this pull request
Feb 13, 2020
|
@isutton: The following tests failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
openshift-merge-robot
pushed a commit
that referenced
this pull request
Feb 19, 2020
* WIP; bring more code from the #286 * Add missing secrets in testing stage * Use metav1.GroupVersionKind in BackingServiceSelector field schema.GroupVersionKind doesn't have any serialization information, whereas metav1.GroupVersionKind does. * Use make() according to the rules * Make govet happy by not using composite literal with unkeyed fields * Make linter happy by removing ineffassign to err * Use ServiceBinder in Reconciler business logic * Temporarily comment unused method * Add backingServiceSelectors field to CRD * Add back bindinginfo.go and bindinginfo_test.go * Bring annotation fixes from master * BackingServiceSelectors is still optional * Annotate the related secret as well * Make backingServiceSelector optional * Do not append the secret since, it should be done right before updating the resources annotations * Ensure one finalizer in the manifest * Add error message for observability * Amend the backing service selector if it has some value inside * If no error is raised, return early when one field is missing * Add multiple services example directory with README
openshift-merge-robot
pushed a commit
that referenced
this pull request
Feb 20, 2020
* Adding conditions[] to status for SBO * remove success for detection of app resources * Add unit test case for status condition * Add binding status for failing CI * WIP; bring more code from the #286 * Add missing secrets in testing stage * Use metav1.GroupVersionKind in BackingServiceSelector field schema.GroupVersionKind doesn't have any serialization information, whereas metav1.GroupVersionKind does. * Use make() according to the rules * Make govet happy by not using composite literal with unkeyed fields * Make linter happy by removing ineffassign to err * Use ServiceBinder in Reconciler business logic * Temporarily comment unused method * Add backingServiceSelectors field to CRD * Add back bindinginfo.go and bindinginfo_test.go * Bring annotation fixes from master * Append test cases * require BindingSuccess * BackingServiceSelectors is still optional * Annotate the related secret as well * Make backingServiceSelector optional * Do not append the secret since, it should be done right before updating the resources annotations * Ensure one finalizer in the manifest * Add error message for observability * Amend the backing service selector if it has some value inside * If no error is raised, return early when one field is missing * Add multiple services example directory with README * Remove magic string * Remove double declaration * Requeue only if not found * Update BindingStatus * Update the ServiceBinder's SBR after it has been updated * Add specific errors for empty application and backing service selectors * Add conditions checking to existing Bind tests * Three replicas is fine * Propagate the error * Export functions to execute the updates * Update conditions accordingly if binding couldn't be executed * Resolve commit in README.md * Update comment * Add comments for functions * Make functions as exported * make function as non exported Co-authored-by: Igor Sutton <isuttonl@redhat.com>
|
This has landed in master. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation
Some changes are required to implement arbitrary paths to embed secret in Application resources and support multiple backing services.
Disclaimer
This PR serves as reference to SBO design direction with multiple services support; it is very likely the work should be re-done from
masterand existing components be used in several PRs paving the way to reach the desired design.Changes
This PR proposes a design direction using and modifying existing components, and also add some high level components and types.
The
Reconcilefunction was re-factored, creating an extra component and a builder function; the builder function wraps the data gathering process, encoded byPlannerandRetriever, and theServiceBindercomponent which is responsible for the orchestration of objects in Kubernetes, usingBinderandSecret.Retrieverreceived the "detect bindable resources" functionality; previous bindable resources implementation was too late in the process. I believe that we can simplify it further as well, but can't pinpoint exactly which direction as of yet.RelatedResourcesfield was added toPlanto support multiple backing services, deprecating bothCRDDescriptionandCR.RelatedResourceis a CRDDescription/CR tuple.Testing
End-to-end tests have not been modified in this branch, so it is expected that everything that was working prior this work should be working.
Tests have been added for
ServiceBinder, and tests covering existing components have been modified accordingly.