odo describe binding #5773

feloy · 2022-05-30T06:48:26Z

What type of PR is this:

/kind feature

What does this PR do / why we need it:

Which issue(s) this PR fixes:

Fixes #5659

PR acceptance criteria:

Unit test
Integration test
Documentation

How to test changes / Special notes to the reviewer:

netlify · 2022-05-30T06:48:30Z

✅ Deploy Preview for odo-docusaurus-preview canceled.

Name	Link
🔨 Latest commit	`ec92333`
🔍 Latest deploy log	https://app.netlify.com/sites/odo-docusaurus-preview/deploys/6299efe72dd4fb0008d4578b

odo-robot · 2022-05-30T06:55:17Z

Unit Tests on commit 92603c9 finished successfully.
View logs: TXT HTML

odo-robot · 2022-05-30T07:01:27Z

Kubernetes Tests on commit 92603c9 finished successfully.
View logs: TXT HTML

odo-robot · 2022-05-30T07:01:38Z

Windows Tests (OCP) on commit 92603c9 finished successfully.
View logs: TXT HTML

odo-robot · 2022-05-30T07:01:46Z

OpenShift Tests on commit 92603c9 finished successfully.
View logs: TXT HTML

odo-robot · 2022-05-30T07:06:05Z

Validate Tests on commit 92603c9 finished successfully.
View logs: TXT HTML

pkg/odo/cli/describe/binding.go

pkg/binding/binding.go

kadel · 2022-06-01T09:03:39Z

did some basic testing and this looks great.

👍 ❤️ for adding documentation.

I would maybe add just one small thing.

When a user runs odo describe binding in a directory with devfile for which odo dev is not running odo should probably provide some hint why is that.
Currently, it shows only

...
...
...
Available binding information: unknown

We could add a note at the end of the output to explain what is happening.

...
...
...
Available binding information: unknown

Binding information for one or more ServiceBinding is not available because they don't exist on the cluster yet.
Start `odo dev` first to see binding information.

pkg/api/binding.go

pkg/binding/binding.go

feloy · 2022-06-01T11:24:06Z

With hindsight, shouldn't we check that the secret does exist as well (since we are getting the binding information from the secret)? I just manually deleted the secret in my local cluster (thinking it would maybe get recreated, but it was not)

I'm not sure. If the InjectionReady condition of the SB is true but the secret is not present, that seems an operational problem to me, and the user should be aware of it. Also, if the user has deleted the secret, odo should not behave normally IMO, but return an error.

rm3l · 2022-06-01T11:30:51Z

With hindsight, shouldn't we check that the secret does exist as well (since we are getting the binding information from the secret)? I just manually deleted the secret in my local cluster (thinking it would maybe get recreated, but it was not)

I'm not sure. If the InjectionReady condition of the SB is true but the secret is not present, that seems an operational problem to me, and the user should be aware of it. Also, if the user has deleted the secret, odo should not behave normally IMO, but return an error.

Okay - makes sense. Thanks.

cdrage · 2022-06-02T13:04:54Z

docs/website/versioned_docs/version-3.0.0/command-reference/describe-binding.md

+- [Describe with access to Devfile](#describe-with-access-to-devfile)
+- [Describe without access to Devfile](#describe-without-access-to-devfile)
+
+## Describe with access to Devfile


For command reference, we are trying to have a consistent format between all documentation. Can you split these sections up similar to: https://odo.dev/docs/3.0.0/command-reference/dev ?

Ex. Section for Description, Running the Command and Devfile usage.

I made the changes, thanks

valaparthvi · 2022-06-02T14:51:33Z

/approve

openshift-ci · 2022-06-02T14:51:41Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: valaparthvi

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~~ [valaparthvi]

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

dharmit

Few comments/questions, few change requests.

pkg/api/binding.go

pkg/binding/binding.go

dharmit · 2022-06-03T10:40:49Z

pkg/binding/binding.go

+	}
+
+	if bindingSB.Spec.BindAsFiles {
+		bindings := make([]string, 0, len(secret.Data))


This declaration could be made outside the if block so that it doesn't need to be done twice.

pkg/kclient/binding.go

dharmit · 2022-06-03T10:47:34Z

pkg/kclient/binding.go

+		return specApi.ServiceBinding{}, err
+	}
+
+	var result specApi.ServiceBinding


Similar question as in GetBindingServiceBinding method for var result declaration.

pkg/odo/cli/describe/binding.go

sonarqubecloud · 2022-06-03T11:27:09Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
1 Code Smell

No Coverage information
0.0% Duplication

feloy · 2022-06-03T11:27:35Z

Thanks @dharmit for your review. I made changes based on your feedback, or replied to some comments

dharmit · 2022-06-03T11:31:16Z

Since the request made by @cdrage is addressed already, and my change requests have also been addressed, LGTM.

/lgtm

* List Servicebindings from SBO only * Support both implementations * Integration tests (TBC) * Human readable output * Support --name flag * Reference doc * fix * self review * Check for InjectionReady condition before to get bindings * Fix comments * Remove unnecessary error returned value * Display info when status is unknown * Sections in doc * Review

openshift-ci bot added the kind/feature Categorizes issue as a feature request. For PRs, that means that the PR is the implementation label May 30, 2022

feloy changed the title ~~odo describe binding~~ [wip] odo describe binding May 30, 2022

openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. Required by Prow. label May 30, 2022

openshift-ci bot requested review from dharmit and rm3l May 30, 2022 06:48

feloy force-pushed the feature-5659/describe-binding branch 5 times, most recently from 07ee170 to 465ab4d Compare May 30, 2022 15:09

feloy changed the title ~~[wip] odo describe binding~~ odo describe binding May 30, 2022

openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. Required by Prow. label May 30, 2022

feloy force-pushed the feature-5659/describe-binding branch from 465ab4d to 7ad8b3a Compare May 30, 2022 15:22

rm3l requested changes May 31, 2022

View reviewed changes

openshift-ci bot assigned rm3l May 31, 2022

rm3l requested changes Jun 1, 2022

View reviewed changes

pkg/api/binding.go Outdated Show resolved Hide resolved

pkg/api/binding.go Outdated Show resolved Hide resolved

pkg/binding/binding.go Show resolved Hide resolved

feloy requested a review from rm3l June 1, 2022 11:47

rm3l approved these changes Jun 1, 2022

View reviewed changes

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. Required by Prow. label Jun 1, 2022

cdrage suggested changes Jun 2, 2022

View reviewed changes

openshift-ci bot assigned cdrage Jun 2, 2022

openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. Required by Prow. label Jun 2, 2022

feloy added 13 commits June 2, 2022 15:21

List Servicebindings from SBO only

7b77b6b

Support both implementations

d3deeb5

Integration tests (TBC)

8a30253

Human readable output

6b6cf8c

Support --name flag

ea8b9eb

Reference doc

df6c922

fix

f71c52d

self review

49425ea

Check for InjectionReady condition before to get bindings

eb1654c

Fix comments

ed26b17

Remove unnecessary error returned value

055d628

Display info when status is unknown

2c5a310

Sections in doc

489855e

feloy force-pushed the feature-5659/describe-binding branch from 231082b to 489855e Compare June 2, 2022 13:21

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. Required by Prow. label Jun 2, 2022

feloy requested review from cdrage and rm3l June 2, 2022 15:11

dharmit suggested changes Jun 3, 2022

View reviewed changes

openshift-ci bot assigned dharmit Jun 3, 2022

Review

ec92333

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. Required by Prow. label Jun 3, 2022

openshift-merge-robot merged commit ba7f1a1 into redhat-developer:main Jun 3, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

odo describe binding #5773

odo describe binding #5773

feloy commented May 30, 2022 •

edited

Loading

netlify bot commented May 30, 2022 •

edited

Loading

odo-robot bot commented May 30, 2022 •

edited

Loading

odo-robot bot commented May 30, 2022 •

edited

Loading

odo-robot bot commented May 30, 2022 •

edited

Loading

odo-robot bot commented May 30, 2022 •

edited

Loading

odo-robot bot commented May 30, 2022 •

edited

Loading

kadel commented Jun 1, 2022

feloy commented Jun 1, 2022

rm3l commented Jun 1, 2022

cdrage Jun 2, 2022

feloy Jun 2, 2022 •

edited

Loading

valaparthvi commented Jun 2, 2022

openshift-ci bot commented Jun 2, 2022

dharmit left a comment

dharmit Jun 3, 2022

dharmit Jun 3, 2022

sonarqubecloud bot commented Jun 3, 2022

feloy commented Jun 3, 2022

dharmit commented Jun 3, 2022

odo describe binding #5773

odo describe binding #5773

Conversation

feloy commented May 30, 2022 • edited Loading

netlify bot commented May 30, 2022 • edited Loading

✅ Deploy Preview for odo-docusaurus-preview canceled.

odo-robot bot commented May 30, 2022 • edited Loading

odo-robot bot commented May 30, 2022 • edited Loading

odo-robot bot commented May 30, 2022 • edited Loading

odo-robot bot commented May 30, 2022 • edited Loading

odo-robot bot commented May 30, 2022 • edited Loading

kadel commented Jun 1, 2022

feloy commented Jun 1, 2022

rm3l commented Jun 1, 2022

cdrage Jun 2, 2022

Choose a reason for hiding this comment

feloy Jun 2, 2022 • edited Loading

Choose a reason for hiding this comment

valaparthvi commented Jun 2, 2022

openshift-ci bot commented Jun 2, 2022

dharmit left a comment

Choose a reason for hiding this comment

dharmit Jun 3, 2022

Choose a reason for hiding this comment

dharmit Jun 3, 2022

Choose a reason for hiding this comment

sonarqubecloud bot commented Jun 3, 2022

feloy commented Jun 3, 2022

dharmit commented Jun 3, 2022

feloy commented May 30, 2022 •

edited

Loading

netlify bot commented May 30, 2022 •

edited

Loading

odo-robot bot commented May 30, 2022 •

edited

Loading

odo-robot bot commented May 30, 2022 •

edited

Loading

odo-robot bot commented May 30, 2022 •

edited

Loading

odo-robot bot commented May 30, 2022 •

edited

Loading

odo-robot bot commented May 30, 2022 •

edited

Loading

feloy Jun 2, 2022 •

edited

Loading