console icon indicating copy to clipboard operation
console copied to clipboard

Bug 2079944: Fix crash and allow users to link a Pod with bindable services

Open christoph-jerolimov opened this issue 3 years ago • 12 comments
trafficstars

Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=2079944

Analysis / Root cause: When a developer creates a simple Pod (without a surrounding Deployment), the topology could not create a ServiceBinding for it.

The action "Create Service Binding" is not shown when right-click a Pod.

When dragging the Arrow from the Pod to the PC "example" the hint "Create Service Binding" is shown! When releasing the drop over it a modal "Create Service Binding" is shown. But the create process fails with this error:

Error "Required value" for field "spec.application.version".

ServiceBinding submitted for a Deployment (works fine):

apiVersion: binding.operators.coreos.com/v1alpha1
kind: ServiceBinding
metadata:
  name: nodeinfo-d-example-pc
  namespace: binding
spec:
  application:
    name: nodeinfo
    group: apps
    version: v1
    resource: deployments
  services:
    - group: postgres-operator.crunchydata.com
      version: v1beta1
      kind: PostgresCluster
      name: example
  detectBindingResources: true

ServiceBinding submitted for a Pod (doesn't work):

apiVersion: binding.operators.coreos.com/v1alpha1
kind: ServiceBinding
metadata: 
  name: example-p-example-pc
  namespace: binding2
spec:
  application:
    name: example
    group: v1
    resource: pods
  services:
    - group: postgres-operator.crunchydata.com
      version: v1beta1
      kind: PostgresCluster
      name: example
  detectBindingResources: true

But it should be:

apiVersion: binding.operators.coreos.com/v1alpha1
kind: ServiceBinding
metadata: 
  name: example-p-example-podZ
  namespace: binding2
spec:
  application:
    name: example
    group: core
    version: v1
    resource: pods
  services:
    - group: postgres-operator.crunchydata.com
      version: v1beta1
      kind: PostgresCluster
      name: example
  detectBindingResources: true

Solution Description: Fixing the createServiceBinding method that could not handle the Pod apiVersion.

Screen shots / Gifs for design review: Before:

before

After:

after

Validation TODO

Unit test coverage report: Small new test added:

 PASS  packages/topology/src/operators/actions/__tests__/serviceBindHey, I don't want keep you here, but do you know what a ServiceBinding does? It provides ings.spec.ts
  createServiceBindingResource
    ✓ creates the right ServiceBinding for a Deployment (6ms)
    ✓ creates the right ServiceBinding for a Pod

Test setup:

  1. Navigate to Operator Hub

  2. Install Service Binding Operator

  3. Install Crunchy Postgres for Kubernetes (verified) operator

  4. Switch to 'Developer' perspective

  5. Add > Operator Backed > Postgres Cluster > Create You need to define

    • Backup > repo > name: "repo1"
    • Backup > restore > repoName: "repo1"
    • Standby > repoName: "repo1"
  6. Create a Pod (search for Pod, Create Pod and create the default Pod)

  7. Link both via drag and drop or by right-clicking the Pod.

Browser conformance:

  • [x] Chrome
  • [ ] Firefox
  • [ ] Safari
  • [ ] Edge

christoph-jerolimov avatar May 03 '22 20:05 christoph-jerolimov

@jerolimov: This pull request references Bugzilla bug 2079944, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker.

3 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target release (4.11.0) matches configured target release for branch (4.11.0)
  • bug is in the state ASSIGNED, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

Requesting review from QA contact: /cc @sanketpathak

In response to this:

Bug 2079944: Fix crash and allow users to link a Pod with bindable services

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.

openshift-ci[bot] avatar May 03 '22 20:05 openshift-ci[bot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jerolimov

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

openshift-ci[bot] avatar May 03 '22 20:05 openshift-ci[bot]

Serena, who can validate this from the SBO perspective if it makes sense ("it works for me") to link a Pod to a bindable component.

/cc @serenamarie125 /hold

christoph-jerolimov avatar May 03 '22 20:05 christoph-jerolimov

@jerolimov: This pull request references Bugzilla bug 2079944, which is valid.

3 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target release (4.11.0) matches configured target release for branch (4.11.0)
  • bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

Requesting review from QA contact: /cc @sanketpathak

In response to this:

Bug 2079944: Fix crash and allow users to link a Pod with bindable services

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.

openshift-ci[bot] avatar May 06 '22 15:05 openshift-ci[bot]

@jerolimov: all tests passed!

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.

openshift-ci[bot] avatar May 17 '22 22:05 openshift-ci[bot]

/cc @divyanshiGupta

invincibleJai avatar May 31 '22 15:05 invincibleJai

Code looks good to me but will run the PR and verify if everything works as expected.

divyanshiGupta avatar Jun 06 '22 11:06 divyanshiGupta

@jerolimov On binding a workload to a service, a secret is created with all the binding data the service exposes. But I dont see that happening when I bind a pod to the service. We should confirm this the app-services team if this is expected.

https://user-images.githubusercontent.com/20724543/172394022-0dd86036-8237-4b0c-88c2-94815dc70e2c.mov

divyanshiGupta avatar Jun 07 '22 13:06 divyanshiGupta

@jerolimov I am not able to delete SB in case of pod. It works fine for deployments.

https://user-images.githubusercontent.com/20724543/172394675-5831cb41-90da-42f4-becd-b11b396c9bf8.mov

divyanshiGupta avatar Jun 07 '22 13:06 divyanshiGupta

Issues go stale after 90d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle stale. Stale issues rot after an additional 30d of inactivity and eventually close. Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle stale

openshift-bot avatar Sep 06 '22 01:09 openshift-bot

@jerolimov: 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.

openshift-merge-robot avatar Sep 06 '22 01:09 openshift-merge-robot

Stale issues rot after 30d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle rotten. Rotten issues close after an additional 30d of inactivity. Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle rotten /remove-lifecycle stale

openshift-bot avatar Oct 06 '22 08:10 openshift-bot

Rotten issues close after 30d of inactivity.

Reopen the issue by commenting /reopen. Mark the issue as fresh by commenting /remove-lifecycle rotten. Exclude this issue from closing again by commenting /lifecycle frozen.

/close

openshift-bot avatar Nov 06 '22 00:11 openshift-bot

@openshift-bot: Closed this PR.

In response to this:

Rotten issues close after 30d of inactivity.

Reopen the issue by commenting /reopen. Mark the issue as fresh by commenting /remove-lifecycle rotten. Exclude this issue from closing again by commenting /lifecycle frozen.

/close

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.

openshift-ci[bot] avatar Nov 06 '22 00:11 openshift-ci[bot]

@jerolimov: This pull request references Bugzilla bug 2079944. The bug has been updated to no longer refer to the pull request using the external bug tracker. All external bug links have been closed. The bug has been moved to the NEW state. Warning: Failed to comment on Bugzilla bug with reason for changed state.

In response to this:

Bug 2079944: Fix crash and allow users to link a Pod with bindable services

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.

openshift-ci[bot] avatar Nov 06 '22 00:11 openshift-ci[bot]

The better fix is available here: 12320

Keep this closed

christoph-jerolimov avatar Dec 09 '22 14:12 christoph-jerolimov