Adds "odo service create --context" feature #1997

dharmit · 2019-08-07T11:46:16Z

What is the purpose of this change? What does it change?

This PR enables the users to create a service with odo service create --context /path/to/component/dir command. This allows service creation from anywhere in the system.

Was the change discussed in an issue?

fixes #1872

How to test changes?

Build the binary

Using nodejs-ex as an example, perform following:

$ git clone https://github.com/sclorg/nodejs-ex /tmp/nodejs-ex
$ cd /tmp/nodejs-ex
$ odo create nodejs
$ cd # this takes you to home dir
$ odo service create dh-postgresql-apb --plan dev -p postgresql_user=luke -p postgresql_password=secret -p postgresql_database=my_data -p postgresql_version=9.6 --context /tmp/nodejs-ex

# Use the --context from /tmp/nodejs-ex/ to list the service
$ odo service list --context /tmp/nodejs-ex

# Use the --context from /tmp/nodejs-ex/ to delete the service
$ odo service delete --context /tmp/nodejs-ex

# Bonus points for testing with --app and --project flag along with --context
# I think the project should exist on the cluster. If app doesn't exist, odo will create it.
$ odo service create dh-prometheus-apb --plan ephemeral --context /tmp/nodejs-ex --app myapp --project myproject

dharmit · 2019-08-08T05:11:39Z

I'll add tests for failure scenario. Missed adding that in the initial PR.

dharmit · 2019-08-09T07:43:24Z

I'll add tests for failure scenario. Missed adding that in the initial PR.

Added this as well as added --context flag to odo service list and odo service delete commands as that seemed like a logical thing to do since we're adding the flag to odo service create command.

surajnarwade · 2019-08-09T09:20:17Z

code wise LGTM

I will wait for CI and then approve it 😄

amitkrout · 2019-08-20T11:08:23Z

tests/integration/servicecatalog/service_test.go

@@ -107,6 +108,7 @@ var _ = Describe("odoServiceE2e", func() {
 			project = helper.CreateRandProject()


To avoid preference conflict in parallel run, use format of context and project create

context = helper.CreateNewContext() os.Setenv("GLOBALODOCONFIG", filepath.Join(context, "config.yaml")) project = helper.CreateRandProject()

Also in JsutAfterEach unset the env var

os.Unsetenv("GLOBALODOCONFIG")

amitkrout · 2019-08-20T11:08:28Z

tests/integration/servicecatalog/service_test.go

+		It("should be able to create, list and delete a service using a given value for --context", func() {
+			// create a component by copying the example
+			helper.CopyExample(filepath.Join("source", "nodejs"), context)
+			helper.CmdShouldPass("odo", "create", "nodejs", "--app", app, "--project", project)


We have already one nodejs component create in this test file. Can you try with some other component

$ odo catalog list components NAME PROJECT TAGS dotnet openshift 2.0,latest httpd openshift 2.4,latest nginx openshift 1.10,1.12,1.8,latest nodejs openshift 10,6,8,8-RHOAR,latest perl openshift 5.24,5.26,latest php openshift 7.0,7.1,latest python openshift 2.7,3.5,3.6,latest ruby openshift 2.3,2.4,2.5,latest wildfly openshift 10.0,10.1,11.0,12.0,13.0,8.1,9.0,latest

amitkrout · 2019-08-20T11:08:33Z

tests/integration/servicecatalog/service_test.go

+		It("should fail to create a service when given directory doesn't contain a context", func() {
+			// cd to the originalDir to create service using --context
+			helper.Chdir(originalDir)
+			stdOut := helper.CmdShouldFail("odo", "service", "create", "dh-postgresql-apb", "--plan", "dev",


Pleas add SetDefaultConsistentlyDuration(30 * time.Second) in BeforeEach(func(){} block because CmdShouldFail has been implemented in a different way, otherwise later it will be flaky

See http://onsi.github.io/gomega/#making-asynchronous-assertions for more details.

amitkrout · 2019-08-20T12:23:38Z

tests/integration/servicecatalog/service_test.go

+			Expect(stdOut).To(ContainSubstring(serviceName))
+
+			// now check if deleting the service using --context works
+			stdOut = helper.CmdShouldPass("odo", "service", "delete", "-f", "serviceName", "--context", context)


"serviceName" -> serviceName

Thanks for the catch!

cdrage · 2019-08-21T20:23:45Z

/retest
(travis failed due to a flake)

dharmit · 2019-08-23T12:15:17Z

Made changes as per @amitkrout's review feedback.

@kadel @girishramnani @mohammedzee1000 @cdrage can I please have a review on this? I have already demo'd this and have made numerous requests in the past on slack for reviewing this!

amitkrout · 2019-08-23T19:02:25Z

Travis is failing
/hold

tests/integration/servicecatalog/service_test.go

This was not a part of any issue or bug or feature request but just made sense to add it since we're adding `--context` flag to `odo service create` command.

…d delete

… with --context flag

#1997 (review)

amitkrout · 2019-08-29T11:59:15Z

/test integration

amitkrout · 2019-08-29T11:59:34Z

/hold cancel

amitkrout · 2019-08-29T12:01:13Z

/approve

openshift-ci-robot · 2019-08-29T12:01:22Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: amitkrout

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:

~~pkg/odo/OWNERS~~ [amitkrout]
~~tests/OWNERS~~ [amitkrout]

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

cdrage

Thanks for adding these tests. All of this works 👍

Great job @dharmit !

/lgtm

openshift-bot · 2019-08-29T17:17:29Z

/retest

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

openshift-bot · 2019-08-29T19:15:01Z

/retest

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

- Lists all URLs even if they are undeployed or not (redhat-developer#2034) - Added json output for `odo project delete -o json` (redhat-developer#2037) - Fixed service integration test that was previously failing (redhat-developer#2022) - `odo push` will now only push changed files (redhat-developer#2030) - We now use relative paths within the file indexer (redhat-developer#2003) - You can now create list and edit services using --app and --project (redhat-developer#2001) - Deleted files will now propagate to the OpenShift container (redhat-developer#1999) - Added `odo service create --context` functionality (redhat-developer#1997) - Making cross-compile independant of gox vendor package (redhat-developer#2047) - `odo-supervisord-image` has been renamed to `odo-init-image` (redhat-developer#2027) - Releases now use .tar.gz (redhat-developer#2009) - If there is an error creating a service, it will fail quicker (redhat-developer#2008) - We now have a Google Group! (redhat-developer#2007) - Added documentation on how to manage environment variables(redhat-developer#2026) - Badges added to the README (redhat-developer#2060) - Updated documentation on uninstallation (redhat-developer#2053) - Added documentation for default parameters (redhat-developer#2038) - Minor update to help output (redhat-developer#2006) - Updated documentation regarding bootstrapper image (redhat-developer#1991)

- Lists all URLs even if they are undeployed or not (#2034) - Added json output for `odo project delete -o json` (#2037) - Fixed service integration test that was previously failing (#2022) - `odo push` will now only push changed files (#2030) - We now use relative paths within the file indexer (#2003) - You can now create list and edit services using --app and --project (#2001) - Deleted files will now propagate to the OpenShift container (#1999) - Added `odo service create --context` functionality (#1997) - Making cross-compile independant of gox vendor package (#2047) - `odo-supervisord-image` has been renamed to `odo-init-image` (#2027) - Releases now use .tar.gz (#2009) - If there is an error creating a service, it will fail quicker (#2008) - We now have a Google Group! (#2007) - Added documentation on how to manage environment variables(#2026) - Badges added to the README (#2060) - Updated documentation on uninstallation (#2053) - Added documentation for default parameters (#2038) - Minor update to help output (#2006) - Updated documentation regarding bootstrapper image (#1991)

openshift-ci-robot added the size/M label Aug 7, 2019

openshift-ci-robot requested review from mohammedzee1000 and surajnarwade August 7, 2019 11:47

dharmit mentioned this pull request Aug 9, 2019

Create, list and edit services using "--app" and "--project" flags (without a context) #2001

Merged

openshift-ci-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. Required by Prow. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. Required by Prow. labels Aug 17, 2019

amitkrout suggested changes Aug 20, 2019

View reviewed changes

openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. Required by Prow. label Aug 23, 2019

amitkrout suggested changes Aug 26, 2019

View reviewed changes

tests/integration/servicecatalog/service_test.go Outdated Show resolved Hide resolved

dharmit added 8 commits August 28, 2019 14:44

Adds --context flag to odo service create command

28d20df

Adds tests for 'odo service --context'

2b124ea

Adds --context flag to service list and delete commands

03f5cd7

This was not a part of any issue or bug or feature request but just made sense to add it since we're adding `--context` flag to `odo service create` command.

Modified tests for service create and added those for service list an…

6531748

…d delete

Adds test for failure to create service using non-component directory…

a69cf78

… with --context flag

Changes as per Amit's feedback

879d83c

#1997 (review)

Remove the context flag from test for it to fail as expected

f796f7b

Remove a test in favour of opening the issue #2051

9115000

openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. Required by Prow. label Aug 29, 2019

openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. Required by Prow. label Aug 29, 2019

dharmit requested review from kadel, cdrage and mik-dass and removed request for surajnarwade August 29, 2019 12:21

cdrage approved these changes Aug 29, 2019

View reviewed changes

openshift-ci-robot assigned cdrage Aug 29, 2019

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. Required by Prow. label Aug 29, 2019

openshift-merge-robot merged commit ff6f5de into redhat-developer:master Aug 29, 2019

cdrage mentioned this pull request Sep 3, 2019

Release 1.0.0-beta5 #2075

Merged

dharmit deleted the fix-1872 branch September 10, 2019 05:57

rm3l added the estimated-size/M (10-20) Rough sizing for Epics. About 1 sprint of work for one person label Jun 18, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adds "odo service create --context" feature #1997

Adds "odo service create --context" feature #1997

dharmit commented Aug 7, 2019 •

edited

dharmit commented Aug 8, 2019

dharmit commented Aug 9, 2019

surajnarwade commented Aug 9, 2019

amitkrout Aug 20, 2019

amitkrout Aug 20, 2019

amitkrout Aug 20, 2019

amitkrout Aug 20, 2019

dharmit Aug 23, 2019

cdrage commented Aug 21, 2019

dharmit commented Aug 23, 2019

amitkrout commented Aug 23, 2019

amitkrout commented Aug 29, 2019

amitkrout commented Aug 29, 2019

amitkrout commented Aug 29, 2019

openshift-ci-robot commented Aug 29, 2019

cdrage left a comment

openshift-bot commented Aug 29, 2019

openshift-bot commented Aug 29, 2019

		@@ -107,6 +108,7 @@ var _ = Describe("odoServiceE2e", func() {
		project = helper.CreateRandProject()

Adds "odo service create --context" feature #1997

Adds "odo service create --context" feature #1997

Conversation

dharmit commented Aug 7, 2019 • edited

What is the purpose of this change? What does it change?

Was the change discussed in an issue?

How to test changes?

dharmit commented Aug 8, 2019

dharmit commented Aug 9, 2019

surajnarwade commented Aug 9, 2019

amitkrout Aug 20, 2019

Choose a reason for hiding this comment

amitkrout Aug 20, 2019

Choose a reason for hiding this comment

amitkrout Aug 20, 2019

Choose a reason for hiding this comment

amitkrout Aug 20, 2019

Choose a reason for hiding this comment

dharmit Aug 23, 2019

Choose a reason for hiding this comment

cdrage commented Aug 21, 2019

dharmit commented Aug 23, 2019

amitkrout commented Aug 23, 2019

amitkrout commented Aug 29, 2019

amitkrout commented Aug 29, 2019

amitkrout commented Aug 29, 2019

openshift-ci-robot commented Aug 29, 2019

cdrage left a comment

Choose a reason for hiding this comment

openshift-bot commented Aug 29, 2019

openshift-bot commented Aug 29, 2019

dharmit commented Aug 7, 2019 •

edited