keithchong commented 3 years ago

Signed-off-by: Keith Chong kykchong@redhat.com

Have to create another PR. I had an issue with rebase with the original PR: https://github.com/redhat-developer/kam/pull/148. It's easier just to create a new PR and close the other one.

What type of PR is this? Use this one instead, but see comments from the original PR. This is for the smart defaulting for kam bootstrap. This also includes a bug fix for #147 , since testing of the feature uncovered the bug.

/kind bug - Fixes 147 /kind enhancement - See GitOps story 563. Fixes points 1 to 4.

What does this PR do / why we need it: See GitOps story 563 points 1-4

147

Have you updated the necessary documentation? (Not yet)

[ ] Documentation update is required by this PR.
[ ] Documentation has been updated.

Which issue(s) this PR fixes: Fixes #147 and GitOps story 563 points 1-4

How to test changes / Special notes to the reviewer: Run kam bootstrap and see if you get prompted for the output folder. Check that the resources are written to the current folder.

openshift-ci-robot commented 3 years ago

@keithchong: The label(s) kind/-, kind/fixes, kind/147, kind/-, kind/see, kind/gitops, kind/story, kind/563., kind/, kind/fixes, kind/points, kind/1, kind/to, kind/4. cannot be applied, because the repository doesn't have them

In response to [this](https://github.com/redhat-developer/kam/pull/158): >Signed-off-by: Keith Chong > >Have to create another PR. I had an issue with rebase with the original PR: https://github.com/redhat-developer/kam/pull/148. >It's easier just to create a new PR and close the other one. > >**What type of PR is this?** >Use this one instead, but see comments from the original PR. >This is for the smart defaulting for kam bootstrap. This also includes a bug fix for #147 , since testing of the feature uncovered the bug. > >/kind bug - Fixes 147 >/kind enhancement - See GitOps story 563. Fixes points 1 to 4. > >**What does this PR do / why we need it**: >See >GitOps story 563 points 1-4 >#147 > >**Have you updated the necessary documentation?** >(Not yet) > >* [ ] Documentation update is required by this PR. >* [ ] Documentation has been updated. > >**Which issue(s) this PR fixes**: >Fixes #147 >and GitOps story 563 points 1-4 > >**How to test changes / Special notes to the reviewer**: >Run kam bootstrap and see if you get prompted for the output folder. Check that the resources are written to the current folder. > > Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.

codecov-io commented 3 years ago

Codecov Report

Merging #158 (e4726f3) into master (51d7c3b) will decrease coverage by 0.68%. The diff coverage is 14.56%.

@@            Coverage Diff             @@
##           master     #158      +/-   ##
==========================================
- Coverage   59.33%   58.64%   -0.69%     
==========================================
  Files          56       56              
  Lines        2855     2902      +47     
==========================================
+ Hits         1694     1702       +8     
- Misses       1003     1040      +37     
- Partials      158      160       +2

Flag	Coverage Δ
unittests	`58.64% <14.56%> (-0.69%)`	:arrow_down:

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
pkg/cmd/ui/ui.go	`0.00% <0.00%> (ø)`
pkg/cmd/ui/validate.go	`47.76% <0.00%> (+5.65%)`	:arrow_up:
pkg/cmd/bootstrap.go	`27.18% <3.70%> (-4.88%)`	:arrow_down:
pkg/pipelines/bootstrap.go	`67.48% <65.00%> (-0.04%)`	:arrow_down:

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 51d7c3b...e4726f3. Read the comment docs.

bigkevmcd commented 3 years ago

/approve

bigkevmcd commented 3 years ago

/lgtm

openshift-ci-robot commented 3 years ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bigkevmcd

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](https://github.com/redhat-developer/kam/blob/master/OWNERS)~~ [bigkevmcd] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment

keithchong commented 3 years ago

Test scenarios. What was tested during unit testing:

kam bootstrap with any argument EXCEPT --interactive should be in non-interactive mode, as before.

kam bootstrap launches command into interactive mode, as before

Also kam bootstrap --interactive launches command into interactive mode

kam bootstrap --interactive and WITH one or more flags launches command into interactive mode. Any values for flags specified on the command line will be used. The values can override the default values OR if the user chooses to, they can even specify the default value. Interactive mode SHOULD NOT ask for the values of these flags. (In other words, there is no point in prompting the user for a value that was already specified on the command line.) This fulfills the acceptance criteria of the enhancement.

If in interactive mode, the very first question displayed is to prompt the user to choose to use defaults or not. Should test for both yes and no responses, and then answer remaining questions until KAM finishes. Also test that flags from the command line are honoured and not prompted again during interactive mode. Keep in mind what are defaults, and what has been overridden.

Examples

kam bootstrap --interactive --> Use defaults? --> No --> This should bring you to the original behaviour as before this enhancement was added.

kam bootstrap --interactive --> Use defaults? --> Yes --> This should prompt you for required flags, and related questions.

`kam bootstrap --service-repo-url https://github.com/keithchong/taxi.git --image-repo quay.io/keithchong/taxi --gitops-repo-url https://github.com/keithchong/gitops11.git --dockercfgjson ~/Downloads/keithchong-robot-auth.json --interactive should prompt you for your git token

Do test override output folder or not

Test the first four points in the acceptance criteria.

Example: This is the sample output for testing the default image registry

Completing Bootstrap process

✓ Options used:
Service repository: https://github.com/keithchong/taxi.git
GitOps repository: https://github.com/keithchong/gitops.git
Image repository: image-registry.openshift-image-registry.svc:5000/cicd/taxi
Commit status tracker: true
Output folder: gitops
Overwrite output folder: true

redhat-developer / kam

Smart default for bootstrap options except for epic point 5; Fix 147 #158

147

Codecov Report