`odo link` and `odo unlink` write to devfile without deploying to cluster by feloy · Pull Request #4819 · redhat-developer/odo

feloy · 2021-06-16T14:34:50Z

What type of PR is this?

/kind feature

What does this PR do / why we need it:

odo link stores link info in devfile
odo link does not create sbr
odo link fails if link with same name already exists
odo unlink removes link info from devfile
odo push creates sbr and makes the container run correctly
odo push deletes sbr and makes the container run correctly
set owner references on Kubernetes inlined resources
~~possibility to create link to a non deployed service~~

Which issue(s) this PR fixes:

Fixes #4208
Fixes #4750
Fixes #4768

PR acceptance criteria:

How to test changes / Special notes to the reviewer:

feloy · 2021-06-18T06:30:35Z

/test v4.7-integration-e2e

feloy · 2021-06-22T13:32:15Z

/test psi-kubernetes-integration-e2e

dharmit

/approve

@feloy thanks for this PR! An amazing result of this PR is that it takes a devfile with component, service and link information and doing odo push creates it all on the cluster. It's an experience that we had been talking about achieving since a LONG time!

openshift-ci · 2021-06-22T16:05:27Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dharmit

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [dharmit]

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

anandrkskd · 2021-06-22T16:43:50Z

+				})
+			}
+
+			list := helper.CmdShouldPass("odo", "catalog", "list", "services")


Can we use the new runner functions in tests.
Functions mentioned in this PR will remove be removed.
I would request you to use those instead of the old ones.

@feloy can you also make changes mentioned in this comment. This PR will remove the older cmd helper functions.
Thanks

feloy · 2021-06-22T18:43:28Z

/test psi-kubernetes-integration-e2e

anandrkskd · 2021-06-23T06:07:17Z

+			When("a link is created between the component and the service", func() {
+
+				BeforeEach(func() {
+					helper.CmdShouldPass("odo", "link", svcFullName)
+				})
+
+				When("odo push is executed", func() {
+					BeforeEach(func() {
+						helper.CmdShouldPass("odo", "push")
+						oc.PodsShouldBeRunning(commonVar.Project, componentName+`-app-.[a-z0-9-]*`)
+					})


@feloy why are we separating odo link and odo push in two when do we want to add a test in between these two scenario?

Can we use this instead? (asking this because I see only one test case getting tested under two layer of when)

When("a link is created between the component and the service and odo push is executed", func() { BeforeEach(func() { helper.CmdShouldPass("odo", "link", svcFullName) helper.CmdShouldPass("odo", "push") oc.PodsShouldBeRunning(commonVar.Project, componentName+`-app-.[a-z0-9-]*`) })

Yes, no problem, we can use your version, as there is no test between the two steps.

feloy · 2021-06-23T08:01:16Z

/test psi-kubernetes-integration-e2e

valaparthvi · 2021-06-30T12:27:28Z

@feloy I noticed a funny thing with odo describe.
When 2 odo services are linked to it, and one of them is linked along with --bind-as-files, it doesn't show up in odo describe.

Steps to reproduce -

odo create nodejs frontend --starter
odo push
odo service create etcdoperator.v0.9.4-clusterwide/EtcdCluster
odo push
odo link EtcdCluster/etcdcluster
odo push
odo service create postgresql-operator.v0.1.1/Backup
odo push
odo link Backup/backup --bind-as-files
odo push
odo describe

I have tried this a few times and was always able to reproduce, can you check it once?

feloy · 2021-06-30T12:29:54Z

@feloy I noticed a funny thing with odo describe.
When 2 odo services are linked to it, and one of them is linked along with --bind-as-files, it doesn't show up in odo describe.

Steps to reproduce -

odo create nodejs frontend --starter

odo push

odo service create etcdoperator.v0.9.4-clusterwide/EtcdCluster

odo push

odo link EtcdCluster/etcdcluster

odo push

odo service create postgresql-operator.v0.1.1/Backup

odo push

odo link Backup/backup --bind-as-files

odo push

odo describe

I have tried this a few times and was always able to reproduce, can you check it once?

odo describe as I demoed you this morning is not implemented in this PR but in the other one (based on this one): #4856

valaparthvi · 2021-06-30T12:33:11Z

odo describe as I demoed you this morning is not implemented in this PR but in the other one (based on this one): #4856

Ah okay. Apart from it being dependent on this PR, is it ready for testing or still a WIP?

feloy · 2021-06-30T15:49:18Z

/test psi-kubernetes-integration-e2e

feloy · 2021-07-01T06:16:42Z

/test psi-kubernetes-integration-e2e

mik-dass

/lgtm

feloy · 2021-07-01T06:35:15Z

[kubectl] The connection to the server -IP REDACTED-:8443 was refused - did you specify the right host or port?

/test psi-kubernetes-integration-e2e

feloy · 2021-07-01T07:49:00Z

Flaky test
[Fail] odo devfile push command tests verify command executions when default build and run commands are defined [It] should execute correct commands

/test v4.7-integration-e2e

openshift-bot · 2021-07-01T08:08:31Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-ci Bot added the kind/feature Categorizes issue as a feature request. For PRs, that means that the PR is the implementation label Jun 16, 2021

openshift-ci Bot requested review from girishramnani and mohammedzee1000 June 16, 2021 14:34

mik-dass reviewed Jun 17, 2021

View reviewed changes

Comment thread pkg/odo/cli/component/common_link.go Outdated

feloy changed the title ~~Feature 4208/odo link~~ [WIP] Feature 4208/odo link Jun 17, 2021

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 Jun 17, 2021

feloy force-pushed the feature-4208/odo-link branch 3 times, most recently from 2449b9e to 877186b Compare June 17, 2021 14:37

openshift-ci Bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. Required by Prow. label Jun 19, 2021

feloy force-pushed the feature-4208/odo-link branch 2 times, most recently from f8ed8dd to c68988a Compare June 22, 2021 11:36

feloy force-pushed the feature-4208/odo-link branch from c6a6f2e to 34e04ef Compare June 22, 2021 13:36

feloy removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. Required by Prow. label Jun 22, 2021

feloy changed the title ~~[WIP] Feature 4208/odo link~~ Feature 4208/odo link Jun 22, 2021

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 Jun 22, 2021

dharmit reviewed Jun 22, 2021

View reviewed changes

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 22, 2021

anandrkskd reviewed Jun 22, 2021

View reviewed changes

anandrkskd reviewed Jun 23, 2021

View reviewed changes

feloy force-pushed the feature-4208/odo-link branch from 6461a96 to ec91a51 Compare June 23, 2021 06:58

openshift-ci Bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. Required by Prow. label Jun 23, 2021

feloy force-pushed the feature-4208/odo-link branch from 5677500 to 4be820c Compare June 23, 2021 07:54

openshift-ci Bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. Required by Prow. label Jun 23, 2021

feloy force-pushed the feature-4208/odo-link branch from 4be820c to 12fed53 Compare June 23, 2021 09:35

feloy added 13 commits June 30, 2021 09:23

Changelog

0f371da

Fix remove duplicates

d55421f

Do not set owner references again

0017a78

Fix wait for pod terminating

b561203

Simplify env vars deduplication

b8ee82e

Add comment

108a91c

Dont use oc

416e3ab

link type

7898517

fix

ac6cfd6

Fix spinner

2bc451f

Fix tests

eed6739

Fic error message for link/unlink

70b4345

Review

f8592f0

feloy force-pushed the feature-4208/odo-link branch from 5cac471 to f8592f0 Compare June 30, 2021 07:23

No spinner

b320502

valaparthvi mentioned this pull request Jun 30, 2021

Test odo link extensively for 2.3 release #4801

Closed

10 tasks

mik-dass approved these changes Jul 1, 2021

View reviewed changes

openshift-ci Bot assigned mik-dass Jul 1, 2021

openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. Required by Prow. label Jul 1, 2021

openshift-merge-robot merged commit 843e520 into redhat-developer:main Jul 1, 2021

This was referenced Jul 1, 2021

Add linked service information in odo describe #4866

Merged

Pods are terminated and new pods started on initial odo link and every subsequent odo push #3803

Closed

Conversation

feloy commented Jun 16, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

feloy commented Jun 18, 2021

Uh oh!

feloy commented Jun 22, 2021

Uh oh!

dharmit left a comment

Choose a reason for hiding this comment

Uh oh!

openshift-ci Bot commented Jun 22, 2021

Uh oh!

anandrkskd Jun 22, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

anandrkskd Jun 23, 2021

Choose a reason for hiding this comment

Uh oh!

feloy commented Jun 22, 2021

Uh oh!

anandrkskd Jun 23, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

feloy Jun 23, 2021

Choose a reason for hiding this comment

Uh oh!

anandrkskd Jun 23, 2021

Choose a reason for hiding this comment

Uh oh!

feloy commented Jun 23, 2021

Uh oh!

valaparthvi commented Jun 30, 2021

Uh oh!

feloy commented Jun 30, 2021

Uh oh!

valaparthvi commented Jun 30, 2021

Uh oh!

feloy commented Jun 30, 2021

Uh oh!

feloy commented Jul 1, 2021

Uh oh!

mik-dass left a comment

Choose a reason for hiding this comment

Uh oh!

feloy commented Jul 1, 2021

Uh oh!

feloy commented Jul 1, 2021

Uh oh!

openshift-bot commented Jul 1, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

feloy commented Jun 16, 2021 •

edited

Loading

anandrkskd Jun 22, 2021 •

edited

Loading

anandrkskd Jun 23, 2021 •

edited

Loading