Breaking api changes - Refactor ServiceBindingRequest api #608

akashshinde · 2020-08-13T07:43:49Z

Motivation

Ref: https://issues.redhat.com/browse/APPSVC-574

Primarily

Align the ServiceBinding API with the Proposal which also takes into account the new specification.
Open up the API for review by the OLM team. ( There's no point doing an API review for the API which is in master since it was going to change )

Also ..

Standardize use of Golang Kubernetes APIs. Example, used corev1.LocalObjectReference instead of name wherever application.

Changes

This is how ServiceBinding CR would look like

---
apiVersion: operators.coreos.com/v1alpha1
kind: ServiceBinding
metadata:
  name: example-servicebinding
  namespace: sbose
spec:
  services:
    - name: demo-database
      group: postgresql.baiju.dev
      kind: Database
      version: v1alpha1

  application:
    name: nodejs-rest-http-crud
    group: apps
    version: v1
    resource: deployments

Set a Group and Kind which is Kubernetes / OperatorFramework friendly.

apiVersion: operators.coreos.com/v1alpha1
kind: ServiceBinding

Following changes have been made in this PR

Updated apps.openshift.io group to operators.coreos.com
Update ServiceBindingRequest kind to ServiceBinding
Removed spec.backingServiceSelector
Replaced spec.applicationSelector with spec.application
Using Application *Application instead of Application Application ( feedback by OLM team )
Using DetectBindingResources *bool instead of DetectBindingResources bool ( feedback by OLM team )
Replaced spec.backingServiceSelectors with spec.services
Replaced resourceRef in application and services to name

Testing

No extra tests added. Ensured existing ones work!
Validated manually by creating a binding between a nodejs app & Database. (connecting single service)
Validated manually by creating a binding between a nodejs app & Database and etcd (connecting multiple services)

openshift-ci-robot · 2020-08-13T07:43:51Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

akashshinde · 2020-08-17T10:30:42Z

/assign @pedjak
/assign @isutton

akashshinde · 2020-08-17T10:31:00Z

/assign @DhritiShikhar @Avni-Sharma

akashshinde · 2020-08-18T10:23:39Z

/test e2e

...og/service-binding-operator/0.1.1/service-binding-operator.v0.1.1.clusterserviceversion.yaml

sbose78 · 2020-08-18T11:57:34Z

/hold Let the others continue the review, I'm going to bikeshed on the API group name before this gets merged :)

...talog/service-binding-operator/manifests/service-binding-operator.clusterserviceversion.yaml

...og/service-binding-operator/0.1.1/service-binding-operator.v0.1.1.clusterserviceversion.yaml

pedjak · 2020-08-18T18:04:51Z

deploy/role.yaml

      - "list"
      - "watch"
      - "update"
+  - apiGroups:


you should revisit the existing roles from above and remove those pointing to the old apigroups.

...talog/service-binding-operator/manifests/service-binding-operator.clusterserviceversion.yaml

akashshinde · 2020-08-20T05:11:48Z

/test e2e

akashshinde · 2020-08-20T07:02:50Z

/test 4.5-e2e

akashshinde · 2020-08-21T04:58:41Z

/test e2e

Makefile

pkg/apis/apps/v1alpha1/servicebinding_types.go

pkg/controller/servicebinding/annotations/bindinginfo.go

pedjak · 2020-08-24T17:11:31Z

could we rename customEnvVar to env, to match the specification better?

pmacik · 2020-09-11T05:58:14Z

/retest

akashshinde · 2020-09-11T06:39:42Z

/retest

akashshinde · 2020-09-11T08:40:13Z

/retest

akashshinde · 2020-09-11T11:16:26Z

/retest

akashshinde · 2020-09-14T05:33:52Z

/retest

akashshinde · 2020-09-14T08:26:57Z

/retest

openshift-ci-robot · 2020-09-14T13:01:02Z

@akashshinde: The following tests failed, say /retest to rerun all failed tests:

Test name	Commit	Details	Rerun command
ci/prow/unit	`7441228`	link	`/test unit`
ci/prow/lint	`7441228`	link	`/test lint`
ci/prow/e2e	`7441228`	link	`/test e2e`

Full PR test history. Your PR dashboard.

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.

pedjak · 2020-09-14T21:38:47Z

/retest

Updated apps.openshift.io group to operators.coreos.com Updated ServiceBindingRequest kind to ServiceBinding Removed spec.backingServiceSelector Replaced spec.applicationSelector with spec.application Using Application *Application instead of Application Application ( feedback by OLM team ) Using DetectBindingResources *bool instead of DetectBindingResources bool ( feedback by OLM team ) Replaced spec.backingServiceSelectors with spec.services Replaced resourceRef in application and services to name

pedjak · 2020-09-15T07:17:57Z

/lgtm
/approve

pedjak · 2020-09-15T08:08:11Z

/hold cancel

pedjak · 2020-09-15T08:08:46Z

/lgtm
/approve

openshift-ci-robot · 2020-09-15T08:08:51Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: pedjak

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:

~~OWNERS~~ [pedjak]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

openshift-ci-robot added the do-not-merge/work-in-progress label Aug 13, 2020

openshift-ci-robot requested review from baijum and otaviof August 13, 2020 07:43

akashshinde force-pushed the breaking-api-changes branch from 350037b to 6e25a4e Compare August 17, 2020 07:03

akashshinde marked this pull request as ready for review August 17, 2020 10:29

openshift-ci-robot removed the do-not-merge/work-in-progress label Aug 17, 2020

openshift-ci-robot assigned isutton and pedjak Aug 17, 2020

openshift-ci-robot assigned Avni-Sharma and DhritiShikhar Aug 17, 2020

akashshinde force-pushed the breaking-api-changes branch from 5aace07 to 7cab24f Compare August 18, 2020 05:30

Avni-Sharma reviewed Aug 18, 2020

View reviewed changes

...og/service-binding-operator/0.1.1/service-binding-operator.v0.1.1.clusterserviceversion.yaml Outdated Show resolved Hide resolved

sbose78 requested a review from isutton August 18, 2020 11:55

openshift-ci-robot added the do-not-merge/hold label Aug 18, 2020

Avni-Sharma reviewed Aug 18, 2020

View reviewed changes

...talog/service-binding-operator/manifests/service-binding-operator.clusterserviceversion.yaml Outdated Show resolved Hide resolved

pedjak suggested changes Aug 18, 2020

View reviewed changes

Avni-Sharma reviewed Aug 19, 2020

View reviewed changes

...talog/service-binding-operator/manifests/service-binding-operator.clusterserviceversion.yaml Outdated Show resolved Hide resolved

akashshinde requested a review from pedjak August 20, 2020 07:05

DhritiShikhar mentioned this pull request Aug 20, 2020

Community Meeting - August 20, 2020 #609

Closed

openshift-ci-robot added the needs-rebase label Aug 24, 2020

pedjak suggested changes Aug 24, 2020

View reviewed changes

akashshinde force-pushed the breaking-api-changes branch 2 times, most recently from 567224e to 3ae312c Compare September 11, 2020 04:47

akashshinde changed the title ~~Breaking api changes - Refactor ServiceBindingRequest api~~ Bug 1878016: Breaking api changes - Refactor ServiceBindingRequest api Sep 11, 2020

akashshinde changed the title ~~Bug 1878016: Breaking api changes - Refactor ServiceBindingRequest api~~ Breaking api changes - Refactor ServiceBindingRequest api Sep 11, 2020

akashshinde force-pushed the breaking-api-changes branch from 37ab2ff to 6e1274d Compare September 15, 2020 04:12

openshift-ci-robot added lgtm approved labels Sep 15, 2020

openshift-ci-robot removed the do-not-merge/hold label Sep 15, 2020

openshift-merge-robot merged commit 6d7a3aa into redhat-developer:master Sep 15, 2020

pedjak mentioned this pull request Sep 15, 2020

Remove backingServiceSelector from the CRD spec #455

Closed

arthurdm mentioned this pull request Sep 15, 2020

Proposal to add an alternate service binding implementation #674

Closed

dharmit mentioned this pull request Sep 17, 2020

Make "odo link" compatible with the breaking changes introduced by Service Binding Operator redhat-developer/odo#3973

Closed

4 tasks

Avni-Sharma mentioned this pull request Sep 21, 2020

add a java mp jpa scenario using crunchy psql operator #631

Closed

divyanshiGupta mentioned this pull request Sep 30, 2020

Bug 1883591: Support ServiceBinding breaking api changes openshift/console#6792

Merged

Avni-Sharma mentioned this pull request Oct 15, 2020

Update README.md #719

Closed

divyanshiGupta mentioned this pull request Oct 20, 2020

Bug 1889783: Support ServiceBinding breaking api changes openshift/console#6962

Merged

pedjak mentioned this pull request Jan 25, 2021

Upstream-friendly API Group of the ServiceBindingRequest CRD #364

Closed

Breaking api changes - Refactor ServiceBindingRequest api #608

Breaking api changes - Refactor ServiceBindingRequest api #608

Uh oh!

Conversation

akashshinde commented Aug 13, 2020 • edited by baijum Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Changes

Testing

Uh oh!

openshift-ci-robot commented Aug 13, 2020

Uh oh!

akashshinde commented Aug 17, 2020

Uh oh!

akashshinde commented Aug 17, 2020

Uh oh!

akashshinde commented Aug 18, 2020

Uh oh!

Uh oh!

sbose78 commented Aug 18, 2020

Uh oh!

Uh oh!

Uh oh!

pedjak Aug 18, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

akashshinde commented Aug 20, 2020

Uh oh!

akashshinde commented Aug 20, 2020

Uh oh!

akashshinde commented Aug 21, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pedjak commented Aug 24, 2020

Uh oh!

pmacik commented Sep 11, 2020

Uh oh!

akashshinde commented Sep 11, 2020

Uh oh!

akashshinde commented Sep 11, 2020

Uh oh!

akashshinde commented Sep 11, 2020

Uh oh!

akashshinde commented Sep 14, 2020

Uh oh!

akashshinde commented Sep 14, 2020

Uh oh!

openshift-ci-robot commented Sep 14, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pedjak commented Sep 14, 2020

Uh oh!

pedjak commented Sep 15, 2020

Uh oh!

pedjak commented Sep 15, 2020

Uh oh!

pedjak commented Sep 15, 2020

Uh oh!

openshift-ci-robot commented Sep 15, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

14 participants

akashshinde commented Aug 13, 2020 •

edited by baijum

Loading

openshift-ci-robot commented Sep 14, 2020 •

edited

Loading