Print yaml definition of CR when asked for dry-run #2810

dharmit · 2020-04-03T09:42:55Z

What type of PR is this?

/kind feature

What does does this PR do / why we need it:
When user does odo service create <operator-name> --crd <crd-name> --dry-run,
it will print only the CR's yaml definition that will be used to bring up the
service.

This is needed because yaml definitions provided in the alm-examples are not
always complete. Sometimes they contain template data which needs to be
modified to be able to start the service. After this PR, we will have another
PR which will let the user specify file containing a yaml definition to bring
up the service. That's where this will yaml output in this PR will be really
helpful.

Which issue(s) this PR fixes:

Addresses first acceptance criteria of #2723.

How to test changes / Special notes to the reviewer:

Make sure you have an operator installed on your cluster.
Enable experimental mode export ODO_EXPERIMENTAL=true

Execute following command:

$ odo service create etcdoperator.v0.9.4 --crd EtcdCluster --dry-run
Following definition will be used to create EtcdCluster:

apiVersion: etcd.database.coreos.com/v1beta2
kind: EtcdCluster
metadata:
  name: example
spec:
  size: 3
  version: 3.2.13

NOTE to reviewer & QE: The output generated by above command is likely to
change when the operator is upgrade by its developers. Hence, the integration
test for this PR only checks for the presence of the line Following definition will be used to create.

NOTE to approver: This PR is rebased on top of #2739 and hence that should
go in first.

tests/integration/operatorhub/cmd_service_test.go

amitkrout

@dharmit No UTs will be added because of #2739 (comment) including the the changes you made for cli packages pkg/odo/cli/service/create.go too, right ?

amitkrout · 2020-04-04T22:07:47Z

/test v4.3-integration-e2e-benchmark

codecov · 2020-04-05T00:18:54Z

Codecov Report

Merging #2810 into master will decrease coverage by 0.26%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           master    #2810      +/-   ##
==========================================
- Coverage   43.26%   43.00%   -0.27%     
==========================================
  Files          97       97              
  Lines        8878     8930      +52     
==========================================
- Hits         3841     3840       -1     
- Misses       4673     4726      +53     
  Partials      364      364

Impacted Files	Coverage Δ
pkg/odo/cli/service/create.go	`13.26% <0.00%> (-0.95%)`	⬇️
pkg/kclient/secrets.go	`85.71% <0.00%> (-3.76%)`	⬇️
pkg/url/url.go	`26.66% <0.00%> (-1.41%)`	⬇️
pkg/occlient/occlient.go	`51.81% <0.00%> (-0.93%)`	⬇️
...g/devfile/adapters/kubernetes/component/adapter.go	`27.61% <0.00%> (-0.27%)`	⬇️
pkg/watch/watch.go	`69.41% <0.00%> (+2.35%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 749739a...65dfbcd. Read the comment docs.

pkg/odo/cli/service/create.go

dharmit · 2020-04-13T09:25:03Z

@dharmit No UTs will be added because of #2739 (comment) including the the changes you made for cli packages pkg/odo/cli/service/create.go too, right ?

Yes, that's right. The data is so dynamic that it's not possible to add much in terms of unit tests. But I'm really willing to discuss adding more integration tests if you or anyone thinks that we can add some scenario.

girishramnani · 2020-04-14T12:23:38Z

/approve

openshift-ci-robot · 2020-04-14T12:24:03Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: girishramnani

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

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

adisky · 2020-04-15T07:43:00Z

Looks good to me, just one suggestion, we should have some difference in formatting of message and yaml

$ odo service create nfd.4.3.10-202003311428 --crd NodeFeatureDiscovery --dry-run
Following definition will be used to create NodeFeatureDiscovery:

apiVersion: nfd.openshift.io/v1alpha1
kind: NodeFeatureDiscovery
metadata:
  name: nfd-master-server
spec:
  namespace: openshift-nfd

adisky · 2020-04-15T08:47:33Z

tests/integration/operatorhub/cmd_service_test.go

+			operators := helper.CmdShouldPass("odo", "catalog", "list", "services")
+			etcdOperator := regexp.MustCompile(`etcdoperator\.*[a-z][0-9]\.[0-9]\.[0-9]`).FindString(operators)
+
+			helper.CmdShouldPass("odo", "service", "create", etcdOperator, "--crd", "EtcdCluster", "--dry-run")


can we verify that output contains apiVersion Kind strings? as now this Following definition will be used to create. is removed

Makes sense to me. Done.

adisky · 2020-04-15T10:27:34Z

/lgtm

openshift-bot · 2020-04-15T11:43:18Z

/retest

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

openshift-ci-robot added the kind/feature Categorizes issue as a feature request. For PRs, that means that the PR is the implementation label Apr 3, 2020

openshift-ci-robot requested review from prietyc123 and rajivnathan April 3, 2020 09:43

dharmit mentioned this pull request Apr 3, 2020

Instantiate an operator backed service using yaml file #2723

Closed

2 tasks

amitkrout suggested changes Apr 4, 2020

View reviewed changes

tests/integration/operatorhub/cmd_service_test.go Outdated Show resolved Hide resolved

openshift-ci-robot assigned amitkrout Apr 4, 2020

amitkrout reviewed Apr 4, 2020

View reviewed changes

girishramnani mentioned this pull request Apr 7, 2020

Add ability to create new service backed by an operator #2739

Merged

girishramnani reviewed Apr 7, 2020

View reviewed changes

pkg/odo/cli/service/create.go Outdated Show resolved Hide resolved

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 Apr 10, 2020

Only print yaml definition when asked for dry-run

17f0234

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

adisky reviewed Apr 15, 2020

View reviewed changes

Remove informative message so that output can be redirected to file

65dfbcd

openshift-ci-robot assigned adisky Apr 15, 2020

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. Required by Prow. label Apr 15, 2020

openshift-merge-robot merged commit a8d0a17 into redhat-developer:master Apr 15, 2020

dharmit deleted the fix-2723 branch May 15, 2020 04:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Print yaml definition of CR when asked for dry-run #2810

Print yaml definition of CR when asked for dry-run #2810

dharmit commented Apr 3, 2020

amitkrout left a comment

amitkrout commented Apr 4, 2020

codecov bot commented Apr 5, 2020 •

edited

dharmit commented Apr 13, 2020

girishramnani commented Apr 14, 2020

openshift-ci-robot commented Apr 14, 2020

adisky commented Apr 15, 2020

adisky Apr 15, 2020 •

edited

dharmit Apr 15, 2020

adisky commented Apr 15, 2020

openshift-bot commented Apr 15, 2020

Print yaml definition of CR when asked for dry-run #2810

Print yaml definition of CR when asked for dry-run #2810

Conversation

dharmit commented Apr 3, 2020

amitkrout left a comment

Choose a reason for hiding this comment

amitkrout commented Apr 4, 2020

codecov bot commented Apr 5, 2020 • edited

Codecov Report

dharmit commented Apr 13, 2020

girishramnani commented Apr 14, 2020

openshift-ci-robot commented Apr 14, 2020

adisky commented Apr 15, 2020

adisky Apr 15, 2020 • edited

Choose a reason for hiding this comment

dharmit Apr 15, 2020

Choose a reason for hiding this comment

adisky commented Apr 15, 2020

openshift-bot commented Apr 15, 2020

codecov bot commented Apr 5, 2020 •

edited

adisky Apr 15, 2020 •

edited