odo add binding - Bind as files UI update

[valaparthvi]

Signed-off-by: Parthvi Vala pvala@redhat.com

What type of PR is this:
/kind feature

What does this PR do / why we need it:
This PR adds a more understandable question when asking how to service should be bound to the component.

Which issue(s) this PR fixes:

Fixes #5770

PR acceptance criteria:

• Unit test

• Integration test

• Documentation (https://deploy-preview-5817--odo-docusaurus-preview.netlify.app/docs/3.0.0/command-reference/add-binding)

How to test changes / Special notes to the reviewer:

1. mkdir /tmp/101; cd /tmp/101
2. Setup a bindable operator and spawn a service. For example - https://operatorhub.io/operator/cloud-native-postgresql
3. odo init --name restapi --devfile go --starter go-starter
4. odo add binding

[netlify]

✅ Deploy Preview for odo-docusaurus-preview canceled.

Name	Link
🔨 Latest commit	c9601e6
🔍 Latest deploy log	https://app.netlify.com/sites/odo-docusaurus-preview/deploys/62a8464533e56300092bb352

[odo-robot]

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

[valaparthvi]

/retest-required

[odo-robot]

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

[odo-robot]

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

[odo-robot]

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

[odo-robot]

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

[feloy]

IMO, you should avoid testing the literal value directly. You could pass an int as AskOne to get the index of the result, and test on the index.

[valaparthvi]

Right, I keep forgetting about using the index.

[valaparthvi]

I do not want to return the int value here, I want to evaluate the bindAsFiles's bool here. So, even if I use int to store the value of user's answer, I would rely on the options to maintain. Something like below -

func (o *Survey) AskBindAsFiles() (bool, error) {
	options := []string{"Bind as Files", "Bind as Environment Variables"}
	question := &survey.Select{
		Message: "How do you want to bind the service?",
		Options: options,
	}
	var answer int
	err := survey.AskOne(question, &answer)
	if err != nil {
		return true, err
	}

	var bindAsFiles bool
	if answer == 0 {
		bindAsFiles = true
	}
	return bindAsFiles, nil
}

This doesn't seem correct to me.
Is this what you were suggesting?

[feloy]

You are right, this solution now relies on the order of the options, and not anymore on the content, which can be equally error prone.

Or, in your first version, you could make a constant with "Bind as files", to be sure one is not changed without the other.

[valaparthvi]

Const sounds like a better idea, will do that.

[feloy]

/approve

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: feloy

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 [feloy]

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

[valaparthvi]

/override ci/prow/v4.10-integration-e2e

[openshift-ci]

@valaparthvi: Overrode contexts on behalf of valaparthvi: ci/prow/v4.10-integration-e2e

In response to this:

/override ci/prow/v4.10-integration-e2e

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.

[dharmit]

Only doc review. Besides the requested/suggested changes, I feel that we should document the need to either have a running odo dev in a separate terminal, or execute odo dev after adding a binding if it's not running in a separate terminal.

[dharmit]

Code review. Looks good, only minor suggestions.

[dharmit]

Really? Why is that the case? I mean, what's blocking us from setting up Operators on k8s cluster?

[valaparthvi]

I have to take a jab at it again, but the last time we tried, we had some problem related to annotations' character limit with a certain OLM version. I work with a minikube cluster locally, so I can ascertain that it works well on a vanilla K8s cluster, but I'll have to take a look at test cluster again.

[dharmit]

/lgtm

[valaparthvi]

I feel that we should document the need to either have a running odo dev in a separate terminal, or execute odo dev after adding a binding if it's not running in a separate terminal.

I don't think that has to be the case. odo add binding does not require for the component to be pushed on the cluster, it simply adds data to the devfile, you can run odo dev later on to create it on the cluster. I think the suggestion you made in #5817 (comment) should suffice.

[sonarcloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
2 Code Smells

No Coverage information
0.5% Duplication

[dharmit]

/lgtm

[valaparthvi]

/override ci/prow/v4.10-integration-e2e

[openshift-ci]

@valaparthvi: Overrode contexts on behalf of valaparthvi: ci/prow/v4.10-integration-e2e

In response to this:

/override ci/prow/v4.10-integration-e2e

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.